1355179 - Remove B2G special cases in layout/base/tests

Status: RESOLVED FIXED

(Core :: Layout, enhancement)

Description

[Daniel Holbert [:dholbert]]

I just ran across a B2G special case in the test layout/base/tests/test_reftests_with_caret.html

Filing this bug on removing that special case, per bug 1306391.

Comment 1

[Daniel Holbert [:dholbert]]

Actually, I ran across one other test that needs similar fixup, so I'm generifying this bug a bit to cover the whole layout/base/tests directory.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: Bug 1355179 part 1: Remove always-true-now checks for !=B2G in test_reftests_with_caret.html.

Review commit: https://reviewboard.mozilla.org/r/128614/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/128614/

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: Bug 1355179 part 2: Add comment about porting B2G checks to be Android checks in test_event_target_radius.html.

This partially backs out changeset 7f0e4f31589e -- specifically, it backs out
all of the changes to test_event_target_radius.html in that changeset (which
all seem to be B2G-specific).

Review commit: https://reviewboard.mozilla.org/r/128616/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/128616/

Comment 4

[Daniel Holbert [:dholbert]]

The second change is slightly more complex -- I saw a few mentions of B2G in that file, so I'm just backing out all of the changes that landed to that file in that cset (since they all seem related to making it B2G-friendly), as the commit message notes.

For reference, here's the changeset whose layout/base/tests changes I'm reverting here:
  https://hg.mozilla.org/mozilla-central/rev/7f0e4f31589e
...and that was in response to bug 1190541 comment 23 (2nd half).

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8856677 [details]
Bug 1355179 part 1: Remove always-true-now checks for !=B2G in test_reftests_with_caret.html.

https://reviewboard.mozilla.org/r/128614/#review131018

You got me all excited to r+ code deletion. What a letdown! :p

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8856678 [details]
Bug 1355179 part 2: Add comment about porting B2G checks to be Android checks in test_event_target_radius.html.

https://reviewboard.mozilla.org/r/128616/#review131020

Hm, I'll give this an r+ but I'd actually rather just the "B2G" check to a general android check, and keep the code. Although the code appears to be B2G-specific, it's actually valid for any platform that supports zooming, which includes Android. It's just that this entire directory of tests is skipped on Android, so this test is never exercised in that configuration. I hope that some day we will exercise it on that configuration, and in that case it would be useful to keep the code rather than have to reinvent it to fix the failure that would result.

Comment 7

[Daniel Holbert [:dholbert]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #6)
> Comment on attachment 8856678 [details]
> Bug 1355179 part 2: Revert B2G-specific changes to
> test_event_target_radius.html.
> 
> https://reviewboard.mozilla.org/r/128616/#review131020
> 
> It's just that this entire directory of tests is
> skipped on Android

Really? Yikes, you're right! mochitest.ini has "skip-if = toolkit == 'android'"

It looks like this directory has been skipped *forever* on Android.  The exclusion dates back to this line:
 http://searchfox.org/mozilla-central/rev/b70337e19a136ae911afc40e3aa0866973765834/testing/mochitest/android.json#331
...and before that, we had a whitelist of tests-to-actually-run instead of a blacklist of tests-to-skip.

So, this directory has never been run on Android, AFIACT. :(

> so this test is never exercised in that configuration. I
> hope that some day we will exercise it on that configuration, and in that
> case it would be useful to keep the code rather than have to reinvent it to
> fix the failure that would result.

Very much agreed. I'm going to:
 (A) file a followup bug on getting these tests running on Android (which I don't have cycles to drive at the moment but which definitely feels like something we should track/aspire-to)
 (B) Revert all my changes in "Part 2" here, and instead make Part 2 have a comment pointing at the bug that I'm suggesting filing, to say that we should adjust the B2G checks in that testcase to be Android checks instead, as-needed.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Sounds good, thanks!

Comment 9

[Daniel Holbert [:dholbert]]

I filed bug 1355206 on getting this folder of tests to run on Android.

Comment 10

[Daniel Holbert [:dholbert]]

Comment on attachment 8856678 [details]
Bug 1355179 part 2: Add comment about porting B2G checks to be Android checks in test_event_target_radius.html.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/128616/diff/1-2/

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/67ed44b3f67e
part 1: Remove always-true-now checks for !=B2G in test_reftests_with_caret.html. r=kats
https://hg.mozilla.org/integration/autoland/rev/6aec2cb43d81
part 2: Add comment about porting B2G checks to be Android checks in test_event_target_radius.html. r=kats

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/67ed44b3f67e
https://hg.mozilla.org/mozilla-central/rev/6aec2cb43d81