Closed Bug 1142360 Opened 5 years ago Closed 5 years ago

Intermittent INFO TEST-UNEXPECTED-FAIL | test_bug570378-persian-4d.html test_bug570378-persian-4e.html test_bug570378-persian-4f.html,test_bug570378-persian-4g.html | reftest comparison: == ... (with bidi.numeral == 3 or 5) - expected PASS

Categories

(Core :: Layout: Text and Fonts, defect)

39 Branch
x86_64
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox37 --- fixed
firefox38 --- fixed
firefox39 --- fixed
firefox-esr31 --- unaffected
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: KWierso, Assigned: ehsan)

References

Details

(Keywords: intermittent-failure)

Attachments

(3 files)

15:28:57 INFO - 0 INFO SimpleTest START
15:28:58 INFO - 1 INFO TEST-START | layout/base/tests/test_bug1070851.html
15:29:11 INFO - 2 INFO TEST-OK | layout/base/tests/test_bug1070851.html | took 12612ms
15:29:13 INFO - 3 INFO TEST-START | layout/base/tests/test_bug1078327.html
15:29:24 INFO - 4 INFO TEST-OK | layout/base/tests/test_bug1078327.html | took 10874ms
15:29:25 INFO - 5 INFO TEST-START | layout/base/tests/test_bug1080360.html
15:29:30 INFO - 6 INFO TEST-OK | layout/base/tests/test_bug1080360.html | took 5436ms
15:29:31 INFO - 7 INFO TEST-START | layout/base/tests/test_bug1080361.html
15:29:37 INFO - 8 INFO TEST-OK | layout/base/tests/test_bug1080361.html | took 5403ms
15:29:37 INFO - 9 INFO TEST-START | layout/base/tests/test_bug1093686.html
15:29:44 INFO - 10 INFO TEST-OK | layout/base/tests/test_bug1093686.html | took 6185ms
15:29:44 INFO - 11 INFO TEST-START | layout/base/tests/test_bug570378-persian-4e.html
15:29:51 INFO - 12 INFO TEST-UNEXPECTED-FAIL | layout/base/tests/test_bug570378-persian-4e.html | Rendering of reftest bug570378-persian-4 is different with bidi.numeral == 4 - expected PASS
15:29:51 INFO - 13 INFO MEMORY STAT vsize after test: 105877504
15:29:51 INFO - 14 INFO MEMORY STAT residentFast after test: 43581440
15:29:51 INFO - 15 INFO MEMORY STAT heapAllocated after test: 21251992
15:29:51 INFO - 16 INFO TEST-OK | layout/base/tests/test_bug570378-persian-4e.html | took 7022ms
15:29:52 INFO - 17 INFO TEST-START | layout/base/tests/test_bug570378-persian-4f.html
15:29:57 INFO - 18 INFO TEST-OK | layout/base/tests/test_bug570378-persian-4f.html | took 4537ms
15:29:58 INFO - 19 INFO TEST-START | layout/base/tests/test_bug570378-persian-4g.html
15:30:03 INFO - 20 INFO TEST-OK | layout/base/tests/test_bug570378-persian-4g.html | took 5135ms
15:30:04 INFO - 21 INFO TEST-START | layout/base/tests/test_bug570378-persian-5a.html
15:30:08 INFO - 22 INFO TEST-OK | layout/base/tests/test_bug570378-persian-5a.html | took 3949ms
15:30:08 INFO - 23 INFO TEST-START | layout/base/tests/test_bug570378-persian-5b.html
15:30:13 INFO - 24 INFO TEST-OK | layout/base/tests/test_bug570378-persian-5b.html | took 4614ms
15:30:14 INFO - 25 INFO TEST-START | layout/base/tests/test_bug570378-persian-5c.html
15:30:19 INFO - 26 INFO TEST-OK | layout/base/tests/test_bug570378-persian-5c.html | took 4963ms
Flags: needinfo?(jfkthame)
Summary: Intermittent test_bug570378-persian-4e.html | Rendering of reftest bug570378-persian-4 is different with bidi.numeral == 4 - expected PASS → Intermittent test_bug570378-persian-4d.html,test_bug570378-persian-4e.html | Rendering of reftest bug570378-persian-4 is different with bidi.numeral == 4 - expected PASS
We should probably modify the mochitest so that it dumps images on failure.  (In fact, there should probably be shared code to do that if there isn't already.)
I think adding '--screenshot-on-fail' here:
https://dxr.mozilla.org/mozilla-central/source/testing/config/mozharness/b2g_emulator_config.py#61

will do the trick. Here's a try run that also removes --quiet (which hides subtest output from passing tests):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bac913488a11
I'll work on a patch to do that.
Note that this adds a parameter to the callback function so that we can
keep track of which canvas is for the test and which is for the
reference; previously we didn't care.

It also puts testfile and reffile in local variables so that they can be
reused, and then appends the extra bidi numeral information to reffile
when calling assertSnapshots so that the messages contain the same (or
more) data as before.
Attachment #8577485 - Flags: review?(ehsan)
(I tested one of the tests locally, that it still passes unmodified, and fails if I stick an "x" in the test.)
(In reply to Andrew Halberstadt [:ahal] from comment #87)
> I think adding '--screenshot-on-fail' here:
> https://dxr.mozilla.org/mozilla-central/source/testing/config/mozharness/
> b2g_emulator_config.py#61
> 
> will do the trick. Here's a try run that also removes --quiet (which hides
> subtest output from passing tests):
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=bac913488a11

--screenshot-on-fail probably won't be useful for these tests because they change prefs and then change the prefs back, so by the time the screenshot is taken, the pref would be back to its default value and the display changed back correspondingly.
Attachment #8577485 - Flags: review?(ehsan) → review+
I suspect this isn't really about those tests "failing" at all, but rather it's an artifact of the overall test run timing out. (Perhaps screenshots will help to clarify that, once we get them.)

IIRC, the layout/base/tests/test_bug570378-* series of tests run particularly slowly, due to the pref-toggling they do, so if we're going to run into timeout issues, the odds are good that these are the tests where it'll happen.

As an experiment, I pushed a try job where I hacked the manifest to skip most of those tests; I left just a token handful of them enabled.[1] On this job, I got 9 out of 10 green M9 runs; for comparison, retriggering M9 runs on a nearby inbound changeset[2] gave 2 out of 10 green.

Unless/until we can significantly increase the 120-minute timeout, or arrange to split these tests across multiple chunks, I suggest that temporarily disabling most of them on b2g-emulator may be the best thing to do here.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=bb1cd72d3176
[2] https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=8b0b98f81a14&filter-searchStr=emulator mochitest-9
Flags: needinfo?(jfkthame)
Nope, the failure is an actual visual failure -- and it's a failure to show the "broken image" icon, and showing the red dot in its place that we show if we can't load (or haven't yet loaded) the broken image icon.

(In my try run, I got a red dot for the test, and the broken image icon for the reference.)
I wonder if taking the snapshot right after the load event fires, as bidi_numeral_test.js does, is supposed to be sufficient.
Any thoughts on the red dot?  Is this something we've seen elsewhere?
Flags: needinfo?(seth)
(In reply to David Baron [:dbaron] (UTC-7) from comment #124)
> Any thoughts on the red dot?  Is this something we've seen elsewhere?

Probably the problem is that nsDisplayAltFeedback, being a display item that shows an image, needs a geometry item that inherits from nsImageGeometryMixin to be able to handle sync decoding correctly. Somehow it got overlooked when I updated all of the other display items that show images.

I'll file a bug about fixing it that blocks this bug.
Flags: needinfo?(seth)
Depends on: 1143267
I landed the test change on inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5f1d4592207f
that will make these tests dump images.

Note that they also change the failure message, so that the failure message will now be:
TEST-UNEXPECTED-FAIL | layout/base/tests/test_bug570378-persian-4d.html | reftest comparison: == bug570378-persian-4.html bug570378-persian-4-ref.html (with bidi.numeral == 3) - expected PASS
Keywords: leave-open
Summary: Intermittent test_bug570378-persian-4d.html,test_bug570378-persian-4e.html | Rendering of reftest bug570378-persian-4 is different with bidi.numeral == 4 - expected PASS → Intermittent INFO TEST-UNEXPECTED-FAIL | test_bug570378-persian-4d.html test_bug570378-persian-4e.html test_bug570378-persian-4f.html | reftest comparison: == ... (with bidi.numeral == 3 or 5) - expected PASS
There are a bunch more instances of this bug that were incorrectly annotated as bug 951509.
The screenshots seem to suggest that sometimes we end up using a different icon for the unloaded image frame, with red borders and a red dot within it.  I have never seen that icon, and have no idea where it's coming from.
It's the fallback that we draw if the image code hasn't loaded the broken-image or loading-image icon.

Seth thinks the problem is bug 1143267; see above.
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #172)
> The screenshots seem to suggest that sometimes we end up using a different
> icon for the unloaded image frame, with red borders and a red dot within it.
> I have never seen that icon, and have no idea where it's coming from.

See here:

https://dxr.mozilla.org/mozilla-central/source/layout/generic/nsImageFrame.cpp?from=nsImageFrame.cpp&case=true#1281
Seth, can you take this please?
Flags: needinfo?(seth)