Bug 38106 – [chromium] Check and rebaseline SVG test expectations for Chromium

• Component: SVG
• Status: RESOLVED FIXED

[Yuzo Fujishima]

https://bugs.webkit.org/show_bug.cgi?id=37986
has changed SVG rendering.

SVG test expectations for Chromium need to be changed accordingly.

[Yuzo Fujishima]

Committed r58239: <http://trac.webkit.org/changeset/58239>

[Yuzo Fujishima]

Just changed test expectation.
The issue remains.

[Tony Chang]

Created attachment 54265 [details]
Patch

[Tony Chang]

Committed r58241: <http://trac.webkit.org/changeset/58241>

[Tony Chang]

Comment on attachment 54265 [details]
Patch

Cleaned up some duplicate expectations.

[Jeremy Orlow]

All failures can be seen here: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&useWebKitCanary=true&tests=svg%2FW3C-SVG-1.1%2Fanimate-elem-31-t.svg%2Csvg%2FW3C-SVG-1.1%2Fanimate-elem-78-t.svg%2Csvg%2FW3C-SVG-1.1%2Fcolor-prop-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Fcoords-units-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Ffilters-color-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Ffilters-comptran-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpainting-render-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-02-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-03-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-04-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-06-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-07-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-09-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-11-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-12-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-13-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-14-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-15-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-16-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-17-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-18-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-pattern-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Fstruct-group-03-t.svg%2Csvg%2FW3C-SVG-1.1%2Fstruct-use-05-b.svg%2Csvg%2FW3C-SVG-1.1%2Fstyling-inherit-01-b.svg%2Csvg%2Fbatik%2Ffilters%2FfeTile.svg%2Csvg%2Fbatik%2Fmasking%2FmaskRegions.svg%2Csvg%2Fbatik%2Fpaints%2FgradientLimit.svg%2Csvg%2Fbatik%2Fpaints%2FpatternRegionA.svg%2Csvg%2Fbatik%2Fpaints%2FpatternRegions.svg%2Csvg%2Fbatik%2Ftext%2FtextEffect.svg%2Csvg%2Fbatik%2Ftext%2FtextEffect2.svg%2Csvg%2Fbatik%2Ftext%2FtextEffect3.svg%2Csvg%2Fbatik%2Ftext%2FtextProperties.svg%2Csvg%2Fcarto.net%2Ftabgroup.svg%2Csvg%2Fcustom%2FfeComponentTransfer-Discrete.svg%2Csvg%2Fcustom%2FfeComponentTransfer-Gamma.svg%2Csvg%2Fcustom%2FfeComponentTransfer-Linear.svg%2Csvg%2Fcustom%2FfeComponentTransfer-Table.svg%2Csvg%2Fcustom%2Ffill-fallback.svg%2Csvg%2Fcustom%2Fgradient-cycle-detection.svg%2Csvg%2Fcustom%2Fgradient-deep-referencing.svg%2Csvg%2Fcustom%2Finline-svg-in-xhtml.xml%2Csvg%2Fcustom%2Finvalid-css.svg%2Csvg%2Fcustom%2Fjs-late-gradient-and-object-creation.svg%2Csvg%2Fcustom%2Fjs-late-gradient-creation.svg%2Csvg%2Fcustom%2Fjs-late-pattern-and-object-creation.svg%2Csvg%2Fcustom%2Fjs-late-pattern-creation.svg%2Csvg%2Fcustom%2Fpattern-cycle-detection.svg%2Csvg%2Fcustom%2Fpattern-deep-referencing.svg%2Csvg%2Fcustom%2Fpattern-with-transformation.svg%2Csvg%2Fcustom%2Fstroke-fallback.svg%2Csvg%2Fcustom%2Fstroked-pattern.svg%2Csvg%2Fcustom%2Fuse-on-symbol-inside-pattern.svg%2Csvg%2Fhixie%2Ferror%2F003.xml%2Csvg%2Ftext%2Fselection-background-color.xhtml%2Csvg%2Ftext%2Fselection-styles.xhtml%2Csvg%2Ftext%2Ftext-gradient-positioning.svg%2Cfast%2Fbackgrounds%2Fsvg-as-background-2.html%2Csvg%2Fcustom%2Fgradient-stroke-width.svg%2Csvg%2Fcustom%2Fjs-update-gradient.svg%2Csvg%2FW3C-SVG-1.1%2Fmasking-mask-01-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-05-b.svg%2Csvg%2FW3C-SVG-1.1%2Fpservers-grad-19-b.svg%2Csvg%2Fcustom%2Fdominant-baseline-hanging.svg

[Jeremy Orlow]

http://trac.webkit.org/changeset/58244 may be related to this bug as well.

[Dimitri Glazkov (Google)]

Are you guys planning to submit new baselines for these? It's been a few days now.

[Jeremy Orlow]

I don't get it....were you planning on doing the rebaslining Yuzo?  If not, you either shouldn't have rolled the stuff in or (at very least) assigned a bug to someone to do it.

Anyway, I'll take this on.

[Jeremy Orlow]

Tomorrow that is.

[Yuzo Fujishima]

Sorry about this.
It was too big a regression for me to handle myself,
and I don't know how to find a good assignee.
(And I assumed that filing a bug is enough, which seems to be wrong.)

What is the good way to find a relevant (Chromium) developer?

[Jeremy Orlow]

(In reply to comment #11)
> Sorry about this.
> It was too big a regression for me to handle myself,
> and I don't know how to find a good assignee.
> (And I assumed that filing a bug is enough, which seems to be wrong.)

Filing a chrome bug used to just work out because Dimitri triaged them all, but most webkit bugs just get lost in the void.
 
> What is the good way to find a relevant (Chromium) developer?

I assume you were the last Gardener?  If so, probably what you should have done was rebaseline everything but then send the review to someone who knows SVG.  svn blameing one of the SVG directories for someone from Chrome would have been a start.  You could have also cc'ed someone like Dimitri and asked for help.

Anyway, we caught it early, so it's not that big of a deal.

[Dimitri Glazkov (Google)]

(In reply to comment #11)
> Sorry about this.
> It was too big a regression for me to handle myself,
> and I don't know how to find a good assignee.
> (And I assumed that filing a bug is enough, which seems to be wrong.)

I am wondering why you felt it was a difficult problem. Let me try to explain my approach to diagnosing it, hopefully it'll be useful in the future gardening stints.

The dashboard (see Jeremy's link in comment 6) clearly fingers http://trac.webkit.org/log/?verbose=on&rev=58212&stop_rev=58212, which is one revision that caused all these changes.

When you look at the log, this revision changes expectations for a large list of tests that matches nearly one-to-one the list of failures on the canaries.

If you look at the diffs on the new expectations, checked in at r58212 and compare them with the expectation changes on the dashboard, you'll see that they add the same type of RenderTree output:

http://trac.webkit.org/changeset/58212/trunk/LayoutTests/platform/mac/svg/W3C-SVG-1.1/animate-elem-31-t-expected.txt
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showLargeExpectations=true&tests=svg%2FW3C-SVG-1.1%2Fanimate-elem-31-t.svg&showExpectations=true&useWebKitCanary=true

Then we read up on the type of the change in the excellent ChangeLog that Nikolas provided and conclude that yes, all these are valid adjustments. You can even cc him (I just did) on the bug or ask on #webkit (he's WildFox) whether that's correct.

The tedious task of determining that is, well, tedious, but shouldn't take more than 30 mins.

The rest is easy -- just run rebaselining tool and commit the diffs. Does this make any sense?

[Jeremy Orlow]

svg-as-background-2.html might be a real failure

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fbackgrounds%2Fsvg-as-background-2.html&showExpectations=true&useWebKitCanary=true

[Dimitri Glazkov (Google)]

(In reply to comment #14)
> svg-as-background-2.html might be a real failure
> 
> http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fbackgrounds%2Fsvg-as-background-2.html&showExpectations=true&useWebKitCanary=true

Yes indeed! It fails for WebKit/mac too.

[Jeremy Orlow]

Actually, https://bugs.webkit.org/show_bug.cgi?id=38108 is already open for addressing fallout from that change.  Let's continue discussion about svg-as-background-2.html there.

[Marcus Bulach]

Created attachment 54570 [details]
Rebaseline SVG layout tests

[Jeremy Orlow]

Comment on attachment 54570 [details]
Rebaseline SVG layout tests

r=me  (reviewed over shoulder)

[WebKit Commit Bot]

Comment on attachment 54570 [details]
Rebaseline SVG layout tests

Rejecting patch 54570 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Jeremy Orlow', u'--force']" exit_code: 1
Last 500 characters of output:
ests/platform/chromium-win/svg/transforms/text-with-pattern-inside-transformed-html-expected.txt
patching file LayoutTests/platform/chromium-win/svg/transforms/text-with-pattern-with-svg-transform-expected.txt
patching file LayoutTests/platform/chromium-win/traversal/node-iterator-prototype-expected.txt
patching file LayoutTests/platform/chromium/test_expectations.txt
Hunk #1 FAILED at 2799.
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/chromium/test_expectations.txt.rej

Full output: http://webkit-commit-queue.appspot.com/results/1906076

[Yaar Schnitman]

There are missing expectations:
https://bugs.webkit.org/show_bug.cgi?id=38360

[Eric Seidel (no email)]

Unsure as to the status of this bug.  Was it eventually landed?

[Nikolas Zimmermann]

Changed component to SVG, so it shows up in my all-svg-bugs search.

[Adam Barth]

I'm assuming this was fixed since it's super old.