Closed Bug 623405 Opened 14 years ago Closed 14 years ago

Permanent orange: REFTEST TEST-UNEXPECTED-FAIL | reftests/svg/smil/anim-text-rotate-01.svg and REFTEST TEST-UNEXPECTED-FAIL | reftests/svg/dynamic-text-04.svg and then Permanent orange: TEST-UNEXPECTED-PASS | reftests/svg/smil/anim-text-rotate-01.svg

Categories

(Core :: SVG, defect)

x86_64
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b10

People

(Reporter: dholbert, Assigned: philor)

References

Details

Attachments

(3 files, 1 obsolete file)

Attached file reftest failure log (deleted) —
It seems we're looking into enabling unit tests on WinXP, and the reftests: anim-text-rotate-01.svg dynamic-text-04.svg are among the few perma-orange reftests that are blocking the un-hiding of the winXP box(es). http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1294265343.1294266592.424.gz#err12 Rev3 WINNT 5.1 mozilla-central opt test reftest on 2011/01/05 14:09:03 s: talos-r3-xp-038 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/svg/smil/anim-text-rotate-01.svg | image comparison (==) REFTEST TEST-UNEXPECTED-FAIL | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/svg/dynamic-text-04.svg | image comparison (==) I'm filing just one bug for these two tests, because the failures are pretty similar (a few letters have the wrong rotation), so I'm guessing the underlying cause is the same.
Summary: text rotation reftest failures on WinXP: anim-text-rotate-01.svg & dynamic-text-04.svg → svg text rotation reftest failures on WinXP: anim-text-rotate-01.svg & dynamic-text-04.svg
Looks to me like this is bug 421587
Whiteboard: [xp-orange]
Attached patch patch v.1 (selectively disable tests) (obsolete) (deleted) — Splinter Review
This isn't overwhelmingly accurate, since the real condition is more like fails-if(winWidget&&!someVarietyOfAccelerationNotYetDefined), but as bug 623450 is working its way up to discovering, we're utterly wrong about layersGPUAccelerated on WinXP, and that's currently the single thing we have for accelOrNot. This will at least get our tinderboxes working, even though it won't cover every sort of developer's machine.
Assignee: nobody → philringnalda
Status: NEW → ASSIGNED
Attachment #502724 - Flags: review?(longsonr)
OS: Linux → Windows XP
Comment on attachment 502724 [details] [diff] [review] patch v.1 (selectively disable tests) If this unblocks turning things on on tinderbox then that's fine, but do file followups as appropriate (or leave this bug open). r=jwatt
Attachment #502724 - Flags: review?(longsonr) → review+
Comment on attachment 502724 [details] [diff] [review] patch v.1 (selectively disable tests) (/me renames patch away from "Fix", for clarity's sake, since it's not so much a fix as a targeted test-disabling)
Attachment #502724 - Attachment description: Fix v.1 → patch v.1 (selectively disable tests)
Attached patch Fix v.2 (deleted) — Splinter Review
No, I'll assert again that I am fixing this bug, for which the non-abbreviated summary is "Permanent orange: REFTEST TEST-UNEXPECTED-FAIL | reftests/svg/smil/anim-text-rotate-01.svg and REFTEST TEST-UNEXPECTED-FAIL | reftests/svg/dynamic-text-04.svg." As comment 1 and the comment I'm adding to the end of the lines say, the "-FAIL" part of that is not this bug, it's bug 421587. This bug is the "-UNEXPECTED-" part, which I'm fixing by making it expected. This isn't some odd expedient half-measure, this is how annotating failing tests is supposed to work: hg blame tells you what bug annotated it, fails-if tells you by becoming an UNEXPECTED-PASS if something changes to cause it to no longer fail, the comment tells you what bug is supposed to make it pass. However, thinking about that did make me look at the existing annotation, which doesn't actually work that way. If it had been "asserts-if(gtk2Widget, 10) ... # bug 569770" then the harfbuzz bug would have noticed that it was causing a test failure by fixing it, that it was expected that it would fix it, and would have removed the annotation, but skip-if isn't self-healing like fails-if or asserts-if, which is why it's supposed to only be a desperate measure when the test crashes or hangs in some condition. So this patch takes out that annotation, since the tryserver says that it does indeed no longer assert on Linux.
Attachment #502724 - Attachment is obsolete: true
Attachment #502821 - Flags: review?(longsonr)
(In reply to comment #5) > No, I'll assert again that I am fixing this bug, [...] > the "-FAIL" part of that is not this bug, it's bug 421587. This bug is the "-UNEXPECTED-" part Ok, fair enough.
Attachment #502821 - Flags: review?(longsonr) → review+
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b10
Honestly, I don't have the slightest idea what made me think that "drawn in a random spot" somehow meant "drawn in a random spot which will never be the right spot."
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
What fun. I'm pretty sure what happened is this: Unlike dynamic-text-04.svg, anim-text-rotate-01.svg actually was failing because of bug 625800. http://hg.mozilla.org/mozilla-central/rev/60cbff9cbab0 landed the patch for that, along with another cset that broke reftests. Four other pushes piled on. The other cset was backed out, but because we didn't close the tree, and things were backed up, the backout didn't build on Windows at all, and the next push didn't build Windows opt, and thus didn't get WinXP tests. Then the next two pushes got coalesced, built Windows opt, ran working Windows opt reftests for the first time in nine pushes, and the unexpected passes began.
Attached patch "Fix" the unexpected pass (deleted) — Splinter Review
Attachment #504195 - Flags: review?(dholbert)
Comment on attachment 504195 [details] [diff] [review] "Fix" the unexpected pass If we're consistently passing, might as well remove the 'fails' annotation, I guess. Do we know what changed to make us start passing?
Attachment #504195 - Flags: review?(dholbert) → review+
(In reply to comment #26) > Do we know what changed to make us start passing? See comment 23 ;)
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
This happened again today: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1298487014.1298488395.13658.gz REFTEST TEST-UNEXPECTED-PASS | file:///C:/talos-slave/test/build/reftest/tests/layout/reftests/svg/dynamic-text-04.svg | image comparison (==)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
That happened, but that ain't this, and that happened for the first time, not again. This was a test-unexpected-FAIL in a couple of tests, then because I didn't understand what had happened it morphed into also being about a consistent test-unexpected-PASS, but it is absolutely not about a single instance of passing after consistently failing for six weeks. Filed bug 636379.
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
Should we also file a new bug for comment 35?
It's really a fairly simple filter: "Is your failure permanent, happening every single run? If not, this is not the bug you are looking for." The filters beyond that, like "is it still January 2011?" and "has nothing at all changed since these slaves were first added?," mean that nothing else is going to be this bug either, so if someone accidentally fixes dynamic-text-04.svg to be a permanent UNEXPECTED-PASS that will also not be this bug, but for any intermittent orange, the first filter handles the question, and since I will never again need to see this as a reason why a hidden WinXP reftest run failed, maybe getting rid of the whiteboard will keep it from coming up again.
Summary: svg text rotation reftest failures on WinXP: anim-text-rotate-01.svg & dynamic-text-04.svg → Permanent orange: REFTEST TEST-UNEXPECTED-FAIL | reftests/svg/smil/anim-text-rotate-01.svg and REFTEST TEST-UNEXPECTED-FAIL | reftests/svg/dynamic-text-04.svg and then Permanent orange: TEST-UNEXPECTED-PASS | reftests/svg/smil/anim-text-rotate-01.svg
Whiteboard: [xp-orange]
(In reply to comment #36) > Should we also file a new bug for comment 35? Yes. I filed bug 645104.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: