Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix test exclusion logic of wpt/referrer-policy test generator #14131

Merged
merged 1 commit into from
Nov 20, 2018

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

Previously, all worker-request, a-tag, area-tag tests were excluded
because "redirection" field was not included in |selection_pattern|,
and thus "worker-requests-with-swap-origin-redirect" and
"overhead-for-redirection" entries in "excluded_tests" in spec.src.json
were applied to all redirection types.

This CL

  • Includes "redirection" field in |selection_pattern| and
    instead remove "redirection" field from filenames, and
  • Regenerates tests.

Therefore, this CL

  • Adds new test files for worker-request, a-tag, area-tag, and
  • Renames existing test files.

The only manual changes are

  • referrer-policy/generic/tools/common_paths.py
  • third_party/WebKit/LayoutTests/SlowTests
  • third_party/WebKit/LayoutTests/SmokeTests
    and all other modifications are made by
    referrer-policy/generic/tools/generate.py.

Bug: 880027
Change-Id: Ie628f92b334d6e2adddc5957cc6cceaa8c69f322
Reviewed-on: https://chromium-review.googlesource.com/c/1330901
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609433}
warning: inexact rename detection was skipped due to too many files.
warning: you may want to set your diff.renameLimit variable to at least 1920 and retry the command.

Previously, all worker-request, a-tag, area-tag tests were excluded
because "redirection" field was not included in |selection_pattern|,
and thus "worker-requests-with-swap-origin-redirect" and
"overhead-for-redirection" entries in "excluded_tests" in spec.src.json
were applied to all redirection types.

This CL
- Includes "redirection" field in |selection_pattern| and
  instead remove "redirection" field from filenames, and
- Regenerates tests.

Therefore, this CL
- Adds new test files for worker-request, a-tag, area-tag, and
- Renames existing test files.

The only manual changes are
- referrer-policy/generic/tools/common_paths.py
- third_party/WebKit/LayoutTests/SlowTests
- third_party/WebKit/LayoutTests/SmokeTests
and all other modifications are made by
referrer-policy/generic/tools/generate.py.

Bug: 880027
Change-Id: Ie628f92b334d6e2adddc5957cc6cceaa8c69f322
Reviewed-on: https://chromium-review.googlesource.com/c/1330901
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609433}
warning: inexact rename detection was skipped due to too many files.
warning: you may want to set your diff.renameLimit variable to at least 1920 and retry the command.
Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already reviewed downstream.

@gsnedders
Copy link
Member

@Hexcles as much as I love the PR title, you should probably handle this better

@Hexcles
Copy link
Member

Hexcles commented Nov 19, 2018

Yikes...

@Hexcles
Copy link
Member

Hexcles commented Nov 19, 2018

@jugglinmike Taskcluster failed:

+ cd /home/test/web-platform-tests
+ ./tools/ci/taskcluster-run.py --commit-range 'HEAD^' chrome -- --channel=dev --verify
++ sudo Xvfb :99.0 -screen 0 1280x1024x24
INFO:manifest:Downloading manifest from https://github.com/web-platform-tests/wpt/releases/download/merge_pr_14101/MANIFEST-f02babe0650c3a6ee81d8c7542765af1b4761809.json.gz
INFO:manifest:Manifest downloaded
2018-11-19 21:11:49,563 - tc-run - INFO - Identifying tests affected in range 'HEAD^'...
Traceback (most recent call last):
  File "./tools/ci/taskcluster-run.py", line 103, in <module>
    main(**vars(parser.parse_args()))
  File "./tools/ci/taskcluster-run.py", line 61, in main
    tests = tests_affected(commit_range)
  File "./tools/ci/taskcluster-run.py", line 17, in tests_affected
    ], stderr=open(os.devnull, "w"))
  File "/usr/lib/python2.7/subprocess.py", line 223, in check_output
    raise CalledProcessError(retcode, cmd, output=output)
subprocess.CalledProcessError: Command '['python', './wpt', 'tests-affected', '--null', 'HEAD^']' returned non-zero exit status 1
[taskcluster 2018-11-19 21:11:50.745Z] === Task Finished ===
[taskcluster 2018-11-19 21:11:50.817Z] Unsuccessful task run with exit code: 1 completed in 43.316 seconds

Any idea what went wrong? Is this change too large?

@jugglinmike
Copy link
Contributor

The patch does modify a lot of files. The final command in the following script required about 10 minutes to complete on my system:

$ git fetch upstream refs/pull/14131/merge
$ git checkout FETCH_HEAD
$ python ./wpt tests-affected HEAD^

So while I can't reproduce the failure here, I suspect the script is crashing due to system memory limitations.

We can probably make the script more resilient (or maybe just stream the file names as they are discovered), but note that the subsequent stability check would certainly time out.

@Hexcles
Copy link
Member

Hexcles commented Nov 20, 2018

Thanks for the explanation, Mike. Let me admin-merge this PR.

I double checked the original CL in Chromium and it's mostly just pure renaming. Chromium CI also passed without a hitch, so I have pretty high confidence.

@Hexcles
Copy link
Member

Hexcles commented Nov 20, 2018

Also filed https://crbug.com/907118 for the funny title.

@Hexcles Hexcles changed the title Fix test exclusion logic of wpt/referrer-policy test generator warning: inexact rename detection was skipped due to too many files. warning: you may want to set your diff.renameLimit variable to at least 1920 and retry the command. Fix test exclusion logic of wpt/referrer-policy test generator Nov 20, 2018
@Hexcles Hexcles merged commit 0944ee2 into master Nov 20, 2018
@Hexcles Hexcles deleted the chromium-export-5ee777f6ec branch November 20, 2018 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants