914089 - b2g.json cleanup and reorder, part 3

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Martijn Wargers (dead)]

Attachment: all.diff

This is the last part of the b2g.json cleanup and fixing tests for it (previous one was bug 907995).

Comment 1

[Martijn Wargers (dead)]

Attachment: 914089.diff

Updated to tip and pushed this to try: https://tbpl.mozilla.org/?tree=Try&rev=165b385ce0da

Comment 2

[Martijn Wargers (dead)]

Attachment: 914089.diff

Ok, I put back test_rule_insertion.html in the b2g.json file, because I don't know why it's failing on try. It passes locally on b2g emulator. 
The one failure is: "monospace and serif text have sufficiently different widths"
Apparently, the monospace and serif font are the same on the b2g try device or something?

And I fixed the failure in layout/generic/test/test_bug392746.html

Pushed to try:
https://tbpl.mozilla.org/?tree=Try&rev=77e70ea2d581

I don't expect any failures anymore, so this should be ready for review.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 801776 [details] [diff] [review]
914089.diff

Review of attachment 801776 [details] [diff] [review]:
-----------------------------------------------------------------

thanks for the migration and cleanup.

::: layout/generic/test/test_bug470212.html
@@ +20,5 @@
>  function doShiftDrag(){
>    setTimeout(function() {
> +    SpecialPowers.DOMWindowUtils.sendMouseEvent('mousedown',  0, 50, 0, 1, 4);
> +    SpecialPowers.DOMWindowUtils.sendMouseEvent('mousemove',  70, 70, 0, 0, 4);
> +    SpecialPowers.DOMWindowUtils.sendMouseEvent('mousemove',  80, 500, 0, 0, 4);

why not do:
var wu = SpecialPowers.DOMWindowUtils ?

::: layout/style/test/test_rule_insertion.html
@@ +83,5 @@
>         return false;
>       }
> +     if (navigator.oscpu.match(/Linux/) ||
> +         navigator.oscpu.match(/Android/) ||
> +         SpecialPowers.Services.appinfo.name == "B2G") {

please add a comment to the makefile that includes this similar to this:
# test_rule_insertion.html is skipped on linux, android and b2g.

::: testing/mochitest/androidx86.json
@@ +244,5 @@
>   "layout/base/tests/test_bug332655-1.html": "",
>   "layout/base/tests/test_bug603550.html": "TIMED_OUT",
>   "layout/base/tests/test_bug629838.html": "",
>   "layout/base/tests/test_flush_on_paint.html": "",
>   "layout/base/tests/test_mozPaintCount.html": "",

fyi, there might be a merge conflict with this file as I reviewed another patch today that touches it as well

Comment 4

[Martijn Wargers (dead)]

Attachment: 914089.diff

(In reply to Joel Maher (:jmaher) from comment #3)
> why not do:
> var wu = SpecialPowers.DOMWindowUtils ?

Ok, did that.

> ::: layout/style/test/test_rule_insertion.html
> @@ +83,5 @@
> >         return false;
> >       }
> > +     if (navigator.oscpu.match(/Linux/) ||
> > +         navigator.oscpu.match(/Android/) ||
> > +         SpecialPowers.Services.appinfo.name == "B2G") {
> 
> please add a comment to the makefile that includes this similar to this:
> # test_rule_insertion.html is skipped on linux, android and b2g.

Only a part of test_rule_insertion.html is not running on linux and Android. On b2g.json this test file is still in the b2g.json file, but that one failure that is still there on b2g, can be fixed. So that comment doesn't really seem correct to me. 

Tryserver is closed, currently. I need to 10 times rerun all the test files that were removed from the exclude list.

Comment 5

[Martijn Wargers (dead)]

Comment on attachment 802598 [details] [diff] [review]
914089.diff

Review of attachment 802598 [details] [diff] [review]:
-----------------------------------------------------------------

Do you agree with my comment 4 about not adding that comment in Makefile.in?

Comment 6

[Martijn Wargers (dead)]

Pushed last attachment to try: https://tbpl.mozilla.org/?tree=Try&rev=381340db8641

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

I agree with you on the makefile comment.

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 802598 [details] [diff] [review]
914089.diff

Review of attachment 802598 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/tests/test_event_target_radius.html
@@ +83,5 @@
>    var e = document.getElementById(id);
>    e.hidden = !show;
>  }
>  
> +var mm;

why is this defined outside of runtest?

::: layout/forms/test/test_bug549170.html
@@ +18,5 @@
>    onmouseup="mouseHandler(event);"
>    onmousedown="mouseHandler(event);">
>  <textarea id='t'
>    onmouseup="mouseHandler(event);"
> +  onmousedown="mouseHandler(event);"></textarea><br>

why did you add a <br> here?

::: layout/generic/test/test_bug263683.html
@@ +42,5 @@
>  
>        function onLoad() {
> +        SpecialPowers.pushPrefEnv({'set': [[prefNameBG, "#EF0FFF"], [prefNameFG, "#FFFFFF"]]}, startTest);
> +      }
> +  

nit: extra whitespace

@@ +50,5 @@
>          // Take a snapshot now. This will be used to check that removing the
>          // ranges removes the highlighting correctly
>          var noHighlight = snapshotWindow(window);
>  
> +  

nit: extra whitespace

Comment 9

[Martijn Wargers (dead)]

(In reply to Joel Maher (:jmaher) from comment #8)
> > +var mm;
> 
> why is this defined outside of runtest?

Because it was defined as a global variable beforehand and I changed it now to use various functions and every function needs that variable.
Otherwise I would have to do:
var mm = document.getElementById("ruler").getBoundingClientRect().width;
in every function.

> > +  onmousedown="mouseHandler(event);"></textarea><br>
> 
> why did you add a <br> here?

This test is mimicking some mouse clicks with domwindoutils. On the B2G emulator, however, the test iframe contained inside the mochitest app isn't completely visible. In this case, it caused the "input id='ip'" element to not be visible and apparently, synthesizeMouseEvent can't be used on elements that aren't visible.
The <br> element makes those elements visible.

Comment 10

[Martijn Wargers (dead)]

Attachment: 914089.diff (for check-in)

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

I am fine with these additions, lets get these landed :)

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/cfb005bc3deb

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cfb005bc3deb