11:53:30 ERROR - 720 INFO TEST-UNEXPECTED-FAIL | toolkit/components/windowcreator/test/test_window_open_position_constraint.html | window should not be opened off-screen: got location 32767,32767

That looks suspiciously as though some coordinate arithmetic got truncated to 16-bit values somewhere, but offhand I don't know where that could happen. What's different about this build from our "normal" Windows builds (where AFAIK this test doesn't fail)?

(In reply to Jonathan Kew (:jfkthame) from comment #1) > That looks suspiciously as though some coordinate arithmetic got truncated > to 16-bit values somewhere, but offhand I don't know where that could > happen. What's different about this build from our "normal" Windows builds > (where AFAIK this test doesn't fail)? The coverage build is built with Clang instead of MSVC and is instrumented by the compiler to collect coverage counters. The differences in test results are usually restricted to timing (the coverage build is slower, so tests often timeout in this build but not in a normal debug build). The most likely explanation would be the different compiler I guess, or do we already run this test with Clang builds? I wasn't able to reproduce the failure locally.

Let's disable the test until it is fixed.

(In reply to Marco Castelluccio [:marco] from comment #3) > (In reply to Jonathan Kew (:jfkthame) from comment #1) > > That looks suspiciously as though some coordinate arithmetic got truncated > > to 16-bit values somewhere, but offhand I don't know where that could > > happen. What's different about this build from our "normal" Windows builds > > (where AFAIK this test doesn't fail)? > > The coverage build is built with Clang instead of MSVC and is instrumented > by the compiler to collect coverage counters. > The differences in test results are usually restricted to timing (the > coverage build is slower, so tests often timeout in this build but not in a > normal debug build). > > The most likely explanation would be the different compiler I guess, or do > we already run this test with Clang builds? We run it with clang builds on other platforms, I presume, but the problem may be occurring in Windows-specific widget code, in which case that's not relevant.

(In reply to Marco Castelluccio [:marco] from comment #4) > Created attachment 8933339 [details] [diff] [review] > Disable test until it is fixed > > Let's disable the test until it is fixed. If we disable it, we'll probably never look at it again. :( Which concerns me a bit, because ISTM that there are a couple of possibilities here.... either clang is miscompiling something in the Windows widget code, such that it truncates a computation somewhere, or (more likely) we have some code that's depending on undefined (or implementation-defined) behavior, and we're just getting lucky in the MSVC build. I suppose as long as it's disabled only for the ccov build, we'll see it again if MSVC builds ever start hitting it, or if we switch to clang builds for other purposes, but it feels like an issue we really ought to try and understand better.

Comment on attachment 8933339 [details] [diff] [review] Disable test until it is fixed Review of attachment 8933339 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/windowcreator/test/mochitest.ini @@ +6,5 @@ > skip-if = (toolkit == "cocoa" && e10s) # bug 1252223 > [test_bug499115.html] > [test_nsFind.html] > [test_private_window_from_content.html] > +skip-if = (os == "win" && ccov) # bug 1421715 Doesn't the skip-if annotation need to go *after* the [test...] line it refers to?

(In reply to Jonathan Kew (:jfkthame) from comment #6) > (In reply to Marco Castelluccio [:marco] from comment #4) > > Created attachment 8933339 [details] [diff] [review] > > Disable test until it is fixed > > > > Let's disable the test until it is fixed. > > If we disable it, we'll probably never look at it again. :( Which concerns > me a bit, because ISTM that there are a couple of possibilities here.... > either clang is miscompiling something in the Windows widget code, such that > it truncates a computation somewhere, or (more likely) we have some code > that's depending on undefined (or implementation-defined) behavior, and > we're just getting lucky in the MSVC build. > > I suppose as long as it's disabled only for the ccov build, we'll see it > again if MSVC builds ever start hitting it, or if we switch to clang builds > for other purposes, but it feels like an issue we really ought to try and > understand better. I agree, we should fix it, but unfortunately I don't have time to investigate more at the moment. I will try if I can reproduce it locally and provide STR if somebody wants to look more into the problem. (In reply to Jonathan Kew (:jfkthame) from comment #7) > Comment on attachment 8933339 [details] [diff] [review] > Disable test until it is fixed > > Review of attachment 8933339 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: toolkit/components/windowcreator/test/mochitest.ini > @@ +6,5 @@ > > skip-if = (toolkit == "cocoa" && e10s) # bug 1252223 > > [test_bug499115.html] > > [test_nsFind.html] > > [test_private_window_from_content.html] > > +skip-if = (os == "win" && ccov) # bug 1421715 > > Doesn't the skip-if annotation need to go *after* the [test...] line it > refers to? Yes, that's right. I disabled the wrong test (the commit message is wrong too). This is really interesting though, as the try build with this patch doesn't show test_window_open_position_constraint.html as failing.

Yes, that is curious.... have you run enough try jobs to know whether the original failure was consistent, or was it an intermittent issue all along? (Which might suggest it's more likely timing-related in some way.)

(In reply to Jonathan Kew (:jfkthame) from comment #9) > Yes, that is curious.... have you run enough try jobs to know whether the > original failure was consistent, or was it an intermittent issue all along? > (Which might suggest it's more likely timing-related in some way.) It has been running and failing consistently for days on mozilla-central. BTW, instead of disabling it, we can mark it as failing so that we at least run it and collect coverage from it.

Here's the patch to mark it as failing. BTW, I can't reproduce the failure locally.

Pushed by mcastelluccio@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/78c8c01f4c26 Mark test_window_open_position_constraint.html as failing on Windows coverage builds until it is fixed. r=jfkthame

The test was fixed and reenabled in bug 1448208.