In update-w3c-test-expectations, also download baselines for non-imported tests.

In update-w3c-test-expectations, also download baselines for non-imported tests.

BUG=660888
Committed: https://crrev.com/6d2089d2dbb5a9ceefa7b2c4fd23d48288745c24
Cr-Commit-Position: refs/heads/master@{#428871}

[qyearsley, 2016-10-31 18:50:13]

qyearsley@chromium.org changed reviewers:
+ foolip@chromium.org, jsbell@chromium.org

[qyearsley, 2016-10-31 18:50:13]

Patch 1 of this CL would make it so that whenever update-w3c-test-expectations
is called, baselines and expectations in any location could be updated, not just
for tests in LayoutTests/imported/.

[qyearsley, 2016-10-31 18:55:27]

On 2016/10/31 at 18:50:13, qyearsley wrote:
> Patch 1 of this CL would make it so that whenever update-w3c-test-expectations
is called, baselines and expectations in any location could be updated, not just
for tests in LayoutTests/imported/.

Note, I'm not entirely sure that this is what we want to do; if the non-imported
tests are failing due to an update in testharness.js, then in general I think
that they shouldn't have new TestExpectations lines, but they may require new
baselines. Does that make sense?

[jsbell, 2016-10-31 19:34:15]

On 2016/10/31 18:55:27, qyearsley wrote:
> On 2016/10/31 at 18:50:13, qyearsley wrote:
> > Patch 1 of this CL would make it so that whenever
update-w3c-test-expectations
> is called, baselines and expectations in any location could be updated, not
just
> for tests in LayoutTests/imported/.
> 
> Note, I'm not entirely sure that this is what we want to do; if the
non-imported
> tests are failing due to an update in testharness.js, then in general I think
> that they shouldn't have new TestExpectations lines, but they may require new
> baselines. Does that make sense?

Code change lgtm but yeah...

re: the bug's #1 vs. #2 - I was worried about coupling imported/non-imported
testharness for this reason, but I think the forward-looking approach is to roll
the tools in along with the tests - since we'll hopefully soon be upstreaming
everything new so non-imported becomes the rare case. So... yeah, #2, i.e. this
patch or something similar.

re: baselines: In general, I prefer baselines to TestExpectations lines for
non-flaky (and non-crash, non-timeout) failures; do we indeed have consensus
about that? But consistency across imported/non-imported tests should also be a
priority. Should we look at doing baselines for imported tests as well?

[qyearsley, 2016-10-31 21:52:30]

Description was changed from

==========
In update-w3c-test-expectations, also update expectations for non-imported
tests.

BUG=660888
==========

to

==========
In update-w3c-test-expectations, also download baselines for non-imported tests.

BUG=660888
==========

[qyearsley, 2016-10-31 21:55:37]

On 2016/10/31 at 19:34:15, jsbell wrote:
> re: baselines: In general, I prefer baselines to TestExpectations lines for
non-flaky (and non-crash, non-timeout) failures; do we indeed have consensus
about that? But consistency across imported/non-imported tests should also be a
priority. Should we look at doing baselines for imported tests as well?

I think there's a consensus, since
https://www.chromium.org/developers/testing/webkit-layout-tests/testexpectations
says "If a test can be rebaselined, it should always be rebaselined instead of
adding lines to TestExpectations."

I think that as long as a change to testharness.js only causes other tests to
potentially have different console messages or fail in different ways, then
rebaselining all newly-failing tests should be enough. If changing
testharness.js or other resource files might cause tests to start crashing and
timing out, then we might need to add new lines to TestExpectations; but this
might not be an problem.

So for now, I'd like to commit the smaller simpler change of just calling
rebaseline-cl without --only-changed-tests, and see how the w3c-test-autoroller
does after that. Does that sound OK?

[jsbell, 2016-10-31 22:15:00]

On 2016/10/31 21:55:37, qyearsley wrote:
> I think that as long as a change to testharness.js only causes other tests to
> potentially have different console messages or fail in different ways, then
> rebaselining all newly-failing tests should be enough. If changing
> testharness.js or other resource files might cause tests to start crashing and
> timing out, then we might need to add new lines to TestExpectations; but this
> might not be an problem.

It should be rare but it'll happen - we've had cases in the past where someone
was using testharness improperly or relying on a bug that was fixed. But as long
as we (eventually) have some automated reporting when a team's tests start
failing due to a roll it seems worth it.

> So for now, I'd like to commit the smaller simpler change of just calling
> rebaseline-cl without --only-changed-tests, and see how the
w3c-test-autoroller
> does after that. Does that sound OK?

sgtm

[qyearsley, 2016-10-31 23:04:05]

The CQ bit was checked by qyearsley@chromium.org

[qyearsley, 2016-10-31 23:04:06]

The patchset sent to the CQ was uploaded after l-g-t-m from jsbell@chromium.org
Link to the patchset: https://codereview.chromium.org/2468553002/#ps20001
(title: "Change this CL to only remove --only-changed-tests.")

[commit-bot: I haz the power, 2016-10-31 23:04:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-31 23:55:00]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-31 23:57:20]

Description was changed from

==========
In update-w3c-test-expectations, also download baselines for non-imported tests.

BUG=660888
==========

to

==========
In update-w3c-test-expectations, also download baselines for non-imported tests.

BUG=660888

Committed: https://crrev.com/6d2089d2dbb5a9ceefa7b2c4fd23d48288745c24
Cr-Commit-Position: refs/heads/master@{#428871}
==========

[commit-bot: I haz the power, 2016-10-31 23:57:21]

Patchset 2 (id:??) landed as
https://crrev.com/6d2089d2dbb5a9ceefa7b2c4fd23d48288745c24
Cr-Commit-Position: refs/heads/master@{#428871}