New issue
Advanced search Search tips

Issue 649691 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 621599



Sign in to add a comment

update-w3c-test-expectations invokes rebaseline-cl with non-existent tests.

Project Member Reported by qyears...@chromium.org, Sep 23 2016

Issue description

Example build: https://build.chromium.org/p/chromium.infra.cron/builders/w3c-test-autoroller/builds/7716

There are two potential little fixes here:
 (1) webkit-patch rebaseline-cl should handle the case where it's passed a test that doesn't exist.
 (2) update-w3c-test-expectations shouldn't need to explicitly pass a list of tests.
 
Owner: qyears...@chromium.org
Status: Assigned (was: Available)
Alternative possible fix: Don't invoke rebaseline-cl in update-w3c-test-expectations; instead invoke it in update-w3c-deps without a list of specific tests.

I think that rebaseline-cl could handle the task of finding out which tests are rebaselineable, which would simplify the logic in update-w3c-test-expectations, which would then *only* be responsible for updating TestExpectations.
Blockedon: -621599
Blocking: 621599
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 5 2016

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

commit c4e50671c1d6124c8cc4b315f9308a365da79df4
Author: qyearsley <qyearsley@chromium.org>
Date: Wed Oct 05 18:26:27 2016

Change update-w3c-test-expectations to only rebaseline existing tests.

In this CL:
 - Extract a method get_modified_existing_tests which outputs warnings
   for tests that don't exist.
 - In get_modified_existing_tests, when invoking git diff, add --diff-filter
   so that tests that don't exist shouldn't be gotten in the first place.
 - Update tests and expand test coverage.

BUG= 649691 

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

[modify] https://crrev.com/c4e50671c1d6124c8cc4b315f9308a365da79df4/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations.py
[modify] https://crrev.com/c4e50671c1d6124c8cc4b315f9308a365da79df4/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations_unittest.py

Status: Fixed (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c4e50671c1d6124c8cc4b315f9308a365da79df4

commit c4e50671c1d6124c8cc4b315f9308a365da79df4
Author: qyearsley <qyearsley@chromium.org>
Date: Wed Oct 05 18:26:27 2016

Change update-w3c-test-expectations to only rebaseline existing tests.

In this CL:
 - Extract a method get_modified_existing_tests which outputs warnings
   for tests that don't exist.
 - In get_modified_existing_tests, when invoking git diff, add --diff-filter
   so that tests that don't exist shouldn't be gotten in the first place.
 - Update tests and expand test coverage.

BUG= 649691 

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

[modify] https://crrev.com/c4e50671c1d6124c8cc4b315f9308a365da79df4/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations.py
[modify] https://crrev.com/c4e50671c1d6124c8cc4b315f9308a365da79df4/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations_unittest.py

Comment 8 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 16 2016

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

commit daf1ddbd71f55b0765611a7273c74860dbd3d6b2
Author: qyearsley <qyearsley@chromium.org>
Date: Wed Nov 16 17:52:02 2016

In rebaseline-cl, don't check for local file existence.

I originally added filtering of tests to only include files that exist
locally in http://crrev.com/2371803003.

But now, it seems clearer that this was the wrong choice, since:

 1. Virtual tests don't exist on the local file system
 2. When --issue is passed (when downloading baselines for try jobs
    on another CL), the files don't exist locally.

In order to stop checking for file existence, another little issue
needed to be solved, which was that when getting the list of files
that were changed, the list included deleted files, which we don't
want to rebaseline; so this CL also fixes that.

BUG= 663411 , 656154 , 649691 

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

[modify] https://crrev.com/daf1ddbd71f55b0765611a7273c74860dbd3d6b2/third_party/WebKit/Tools/Scripts/webkitpy/common/net/rietveld.py
[modify] https://crrev.com/daf1ddbd71f55b0765611a7273c74860dbd3d6b2/third_party/WebKit/Tools/Scripts/webkitpy/common/net/rietveld_unittest.py
[modify] https://crrev.com/daf1ddbd71f55b0765611a7273c74860dbd3d6b2/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl.py
[modify] https://crrev.com/daf1ddbd71f55b0765611a7273c74860dbd3d6b2/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline_cl_unittest.py

Sign in to add a comment