New issue
Advanced search Search tips

Issue 736811 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

rebaseline-cl doesn't handle ref-tests

Project Member Reported by reed@google.com, Jun 26 2017

Issue description

This bug tracks the tests that had to be manually suppressed so they could later be manually rebaselined (and/or converted to image tests).
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9ba995c00a01e3e16bb64afcbaeccf922e11bae8

commit 9ba995c00a01e3e16bb64afcbaeccf922e11bae8
Author: reed <reed@google.com>
Date: Mon Jun 26 18:03:20 2017

suppress tests that cannot be rebaselined automatically

These would found by trying to rebaseline this CL
https://chromium-review.googlesource.com/c/548536/

BUG= 736811 
NOTRY=True

Review-Url: https://codereview.chromium.org/2959703003
Cr-Commit-Position: refs/heads/master@{#482319}

[modify] https://crrev.com/9ba995c00a01e3e16bb64afcbaeccf922e11bae8/third_party/WebKit/LayoutTests/TestExpectations

Is this bug proposing a change or fix to rebaseline-cl?

AFAIK right now if there are failing ref tests in try jobs, or if you explicitly pass the name of a ref test to `webkit-patch rebaseline-cl`, the tool will say that it's going to try to rebaseline, but then print a message saying that there's nothing to do.

Note, in general, ref tests don't have baselines so the idea of rebaselining a ref test doesn't make sense.
Cc: qyears...@chromium.org
It's a bug for us to go back and deal with the suppressed ref tests.

What's going on is many ref tests make fragile assumptions about rasterization, and changes in Skia may "break" them.  In general we try to tweak them into passing, but the (unfortunate) fallback is to delete the ref and turn them into regular pixel tests.

An interesting twist are WPT tests: by default they seem to be treated as ref-tests (both when running layout tests and when rebaselining).  But then some of them appear to have .png overrides - which makes me thing they can be both:

find LayoutTests/external/wpt/2dcontext/fill-and-stroke-styles/ -name \*.png

LayoutTests/external/wpt/2dcontext/fill-and-stroke-styles/2d.fillStyle.parse.css-color-4-hsla-4.png
LayoutTests/external/wpt/2dcontext/fill-and-stroke-styles/2d.fillStyle.parse.rgba-clamp-1.png
LayoutTests/external/wpt/2dcontext/fill-and-stroke-styles/2d.fillStyle.parse.css-color-4-hsl-3.png
LayoutTests/external/wpt/2dcontext/fill-and-stroke-styles/2d.fillStyle.parse.hsla-clamp-2.png
LayoutTests/external/wpt/2dcontext/fill-and-stroke-styles/2d.fillStyle.parse.hex3.png
...

I'm not familiar with WPTs - do they get any special treatment in rebaseline-cl?
WPT tests do get special treatment in the test runner (run-webkit-tests) when determining what is a test, and whether it's a ref test, and where the ref files are - specifically, in order to be consistent with how web-platform-tests are expected to be run in other places, we use "MANIFEST.json", generated by the wpt manifest script.

For Chromium now, MANIFEST.json is autogenerated based on external/WPT_MANIFEST_BASE.json, and updated and cached at external/wpt/MANIFEST.json but not tracked in git.

Those png files listed there are completely ignored by our test runner, and AFAICT are mainly provided to help debug tests or manually check test results.

For example, this test:

http://w3c-test.org/2dcontext/fill-and-stroke-styles/2d.fillStyle.parse.css-color-4-hsl-3.html

... is a testharness test that makes an assertion about one particular pixel in a canvas -- and it provides a png image to help clarify what's expected.

I believe we don't run any tests in wpt as pixel tests -- so if something absolutely must be a pixel test, then we don't put it in wpt. But in most cases, making a ref test or testharness test should be possible.
Owner: fmalita@chromium.org
Status: Assigned (was: Untriaged)
Thanks, I was just looking at WPT.  So for changes which "break" some fragile ref-test, the only option is to suppress/mark as failing.  It's not ideal, but I see we already have a bunch of WPT suppressions (importing of that test suite appears to be WIP).
Project Member

Comment 6 by sheriffbot@chromium.org, Jul 12 2017

Labels: Hotlist-Google
Components: Blink>LayoutTests
Labels: -Hotlist-Google OS-All
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2a6664ac9ed9d11d24288dd76e3c784ecb2f97e9

commit 2a6664ac9ed9d11d24288dd76e3c784ecb2f97e9
Author: Florin Malita <fmalita@chromium.org>
Date: Wed Jul 12 19:26:30 2017

Rebaseline suppressions after SK_SUPPORT_LEGACY_COLORFILTER_FILTERSPAN

Deal with some tests which were suppressed for
https://chromium-review.googlesource.com/548536.

BUG= 736811 

Change-Id: Ic861f6682c40a63c0c301309327ada0621f3dcbd
Reviewed-on: https://chromium-review.googlesource.com/567280
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Mike Reed <reed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486054}
[modify] https://crrev.com/2a6664ac9ed9d11d24288dd76e3c784ecb2f97e9/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/2a6664ac9ed9d11d24288dd76e3c784ecb2f97e9/third_party/WebKit/LayoutTests/fast/canvas/canvas-fillPath-gradient-shadow.html
[delete] https://crrev.com/f65350f129df39a6f118902f865fb43945c24768/third_party/WebKit/LayoutTests/fast/css/text-overflow-ellipsis-multiple-shadows-expected.html
[add] https://crrev.com/2a6664ac9ed9d11d24288dd76e3c784ecb2f97e9/third_party/WebKit/LayoutTests/platform/linux/fast/css/text-overflow-ellipsis-multiple-shadows-expected.png
[add] https://crrev.com/2a6664ac9ed9d11d24288dd76e3c784ecb2f97e9/third_party/WebKit/LayoutTests/platform/mac/fast/css/text-overflow-ellipsis-multiple-shadows-expected.png
[add] https://crrev.com/2a6664ac9ed9d11d24288dd76e3c784ecb2f97e9/third_party/WebKit/LayoutTests/platform/mac/fast/css/text-overflow-ellipsis-multiple-shadows-expected.txt
[add] https://crrev.com/2a6664ac9ed9d11d24288dd76e3c784ecb2f97e9/third_party/WebKit/LayoutTests/platform/win/fast/css/text-overflow-ellipsis-multiple-shadows-expected.png
[add] https://crrev.com/2a6664ac9ed9d11d24288dd76e3c784ecb2f97e9/third_party/WebKit/LayoutTests/platform/win/fast/css/text-overflow-ellipsis-multiple-shadows-expected.txt

Status: Fixed (was: Assigned)

Sign in to add a comment