Closed Bug 906378 Opened 11 years ago Closed 11 years ago

Intermittent layout/style/test/test_transitions_per_property.html | application timed out after 330 seconds with no output, usually while testing "transitions not supported for property background-image" (or "image-orientation")

Categories

(Core :: Layout, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla26
Tracking Status
firefox24 --- unaffected
firefox25 --- unaffected
firefox26 --- fixed
firefox-esr24 --- unaffected

People

(Reporter: philor, Assigned: bhackett1024)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #859807 +++

https://tbpl.mozilla.org/php/getParsedLog.php?id=26691346&tree=Mozilla-Inbound
Windows XP 32-bit mozilla-inbound pgo test mochitest-5 on 2013-08-17 15:23:55 PDT for push ed08676c3e70
slave: t-xp32-ix-101

15:27:47     INFO -  80382 INFO TEST-PASS | /tests/layout/style/test/test_transitions_per_property.html | transitions not supported for property background-image value repeating-radial-gradient(closest-corner ellipse at 45px, red, blue)
15:33:17  WARNING -  TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_transitions_per_property.html | application timed out after 330 seconds with no output
I think this has now morphed into:
https://tbpl.mozilla.org/php/getParsedLog.php?id=26759895&tree=Mozilla-Central#error0
{
03:15:42     INFO -  84691 INFO TEST-PASS | /tests/layout/style/test/test_transitions_per_property.html | transitions not supported for property background-image value radial-gradient(red -50%, yellow -25%, green, blue)
03:15:42     INFO -  84692 INFO TEST-PASS | /tests/layout/style/test/test_transitions_per_property.html | transitions not supported for property background-image value -moz-radial-gradient(cover, red, blue)
03:15:42     INFO -  84693 INFO TEST-PASS | /tests/layout/style/test/test_transitions_per_property.html | transitions not supported for property background-image value radial-gradient(red -50%, yellow -25%, green, blue)
03:15:42     INFO -  84694 INFO TEST-PASS | /tests/layout/style/test/test_transitions_per_property.html | transitions not supported for property background-image value -moz-radial-gradient(circle, red, blue)
03:15:42     INFO -  84695 INFO TEST-PASS | /tests/layout/style/test/test_transitions_per_property.html | transitions not supported for property background-image value radial-gradient(red -50%, yellow -25%, green, blue)
03:15:42     INFO -  84696 INFO TEST-PASS | /tests/layout/style/test/test_transitions_per_property.html | transitions not supported for property background-image value -moz-radial-gradient(ellipse closest-corner, red, blue)
03:15:42     INFO -  84697 INFO TEST-PASS | /tests/layout/style/test/test_transitions_per_property.html | transitions not supported for property background-image value radial-gradient(red -50%, yellow -25%, green, blue)
03:32:22    ERROR - timed out after 1000 seconds of no output
}
No way we're going to star that no-test-mentioned 1000 second timeout well. That doesn't strike me as a good idea.

https://tbpl.mozilla.org/php/getParsedLog.php?id=26791358&tree=Mozilla-Inbound
:dholbert is looking at the dependant bug 859807 first - we will see if dealing with that has a positive impact on this one, though they are different.
Daniel, I'm assigning this to you so that it doesn't show up in the "nobody is paying attention to this bug" list - I understand we're going to track bug 859807, see if it stops failing, and see if this one maybe stops failing as well (even though we know they're different bugs)?
If this is not the case, please unassign yourself, unless you are also going to look at this one.
Assignee: nobody → dholbert
(In reply to Milan Sreckovic [:milan] from comment #145)
> Daniel, I'm assigning this to you [...] I understand we're going to track
> bug 859807, see if it stops failing, and see if this one maybe stops failing
> as well (even though we know they're different bugs)?
> If this is not the case, please unassign yourself, unless you are also going
> to look at this one.

Looks like bug 859807 may be fixed, but it didn't fix this one. (which isn't surprising)

Unassigning, as directed. [and clarifying summary slightly]
Assignee: dholbert → nobody
Summary: Intermittent layout/style/test/test_transitions_per_property.html | application timed out after 330 seconds with no output → Intermittent layout/style/test/test_transitions_per_property.html | application timed out after 330 seconds with no output, usually while testing "transitions not supported for property background-image"
Er, ignore that "TODO: TEST THIS".

I tested that the only differences between the output of the test before and after the patch are:
 * a massive reduction in the number of background-image tests
 * noise in the interpolation-of-transitions tests, which just seem to vary

(I'm also a little curious about the "reference to undefined property options.bytes" and "reference to undefined property exn.stack" that showed up during both the before and the after run, though.)
Comment on attachment 801297 [details] [diff] [review]
Make the test for transitions not being supported in test_transitions_per_property.html only use about 50 values for each property to avoid the O(N^2) case blowing up.

>+// Return a consistent sampling of |count| values out of |array|.
>+function sample_array(array, count) {
>+  var ratio = array.length / count;

For foolproofing, perhaps it'd be worth adding a check to ensure count is nonzero, before you divide by it?

Maybe with:
 ok(count > 0, "caller asked us to get less than 1 value from array");

Or alternately, you could gracefully handle it with something like:
 if (count <= 0) {
   return [];
 }

>+      all_values = [].concat(info.initial_values.slice(0,2),
>+                             sample_array(info.initial_values.slice(2), 6),
>+                             info.other_values.slice(0, 10),
>+                             sample_array(info.other_values.slice(12), 40));
>+    }

I think you want s/12/10/ on the last line there, right?  (Otherwise you're preventing indices 10 and 11 from *ever* being sampled in this code, which seems arbitrary.)

Otherwise looks good!

r=me
Attachment #801297 - Flags: review?(dholbert) → review+
(In reply to David Baron [:dbaron] (needinfo? me) from comment #164)
> (I'm also a little curious about the "reference to undefined property
> options.bytes" and "reference to undefined property exn.stack" that showed
> up during both the before and the after run, though.)

(Bug 885615 is filed on an instance of that warning in a osfile_async_worker.js; that's probably what we're hitting?  I'd bet that the "async" nature of that file means it could conceivably print that warning at arbitrary spots in test logs. So anyway, not something to worry about here, probably.)
(In reply to Daniel Holbert [:dholbert] from comment #165)
> For foolproofing, perhaps it'd be worth adding a check to ensure count is
> nonzero, before you divide by it?
> 
> Maybe with:
>  ok(count > 0, "caller asked us to get less than 1 value from array");
> 
> Or alternately, you could gracefully handle it with something like:
>  if (count <= 0) {
>    return [];
>  }

(Or maybe a combination of those two -- maybe adding an "ok(false, ...)" line just before the "return []", so that negative or zero counts will still get graceful behavior but will also definitely get noticed & fixed.)
https://hg.mozilla.org/mozilla-central/rev/2b26501fd203
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Darn; looks like this isn't fixed, per Comment 174.  (Still, it's worth keeping the landed patch in; it may reduce the incidence, and it'll surely will save us some spinning-our-wheels time on this test.)

In the latest log, the timeout is during "transitions not supported for property image-orientation". (Makes sense that it's during a later property now, since the patch makes us spend less time on each property, for properties with lots of values to test.)
Summary: Intermittent layout/style/test/test_transitions_per_property.html | application timed out after 330 seconds with no output, usually while testing "transitions not supported for property background-image" → Intermittent layout/style/test/test_transitions_per_property.html | application timed out after 330 seconds with no output, usually while testing "transitions not supported for property background-image" (or "image-orientation")
Well, then I'm going to point out the regressionwindow-wanted keyword again.  My working theory is that something introduced a hang bug, probably at a lower level in the code (e.g., JS).
Assignee: dbaron → nobody
Target Milestone: mozilla26 → ---
First instance I see was starred in bug 859807 comment 42. You can green arrow down from there to see if anything looks interesting.
https://tbpl.mozilla.org/?rev=af90d4723727&tree=Mozilla-Inbound
Retriggers on Try seem to be strongly pointing at bug 903802 as the regressing push. 30 retriggers on the push prior don't timeout.

Here's a Try push for the backout:
https://tbpl.mozilla.org/?tree=Try&rev=570b644a8005
Flags: needinfo?(bhackett1024)
It would be pretty bizarre if that bug caused any regressions, though it could have tickled something preexisting I guess.  Does anyone know what the test is doing when it times out?  Is it hanging, stuck in the event loop, or just progressing slowly?
(In reply to Brian Hackett (:bhackett) from comment #181)
> Does anyone know what the
> test is doing when it times out?  Is it hanging, stuck in the event loop, or
> just progressing slowly?

It's inside this loop:
http://mxr.mozilla.org/mozilla-central/source/layout/style/test/test_transitions_per_property.html?rev=2b26501fd203&mark=655-666&force=1#655

It mysteriously hangs with no output, and then (after 330 seconds) it suddenly wakes up and prints out one final line of output (which was maybe just waiting to be flushed, for some reason?), with the same timestamp as the "application timed out after 330 seconds with no output" message.

I don't think we have any backtraces from the hang, unfortunately, but it'd probably be handy if we could get one.
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #180)
> Here's a Try push for the backout:
> https://tbpl.mozilla.org/?tree=Try&rev=570b644a8005

50 M5 runs without a failure. So how big of a deal would it be if I went ahead and pushed this backout to inbound while this is investigated further?
https://hg.mozilla.org/mozilla-central/rev/c5fd139db781
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Assignee: ryanvm → bhackett1024
Flags: needinfo?(bhackett1024)
You need to log in before you can comment on or make changes to this bug.