New issue
Advanced search Search tips

Issue 800570 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 801357
issue 831975



Sign in to add a comment

[WPT] Support test names containing ? (URL params)

Project Member Reported by robertma@chromium.org, Jan 9 2018

Issue description

There are a lot of websocket WPT with the following meta tags:
  <meta name="variant" content="">
  <meta name="variant" content="?wss">

This configuration will lead to two test entries under the file in manifest, e.g.

  "websockets/binary/001.html": [
    [
     "/websockets/binary/001.html",
     {}
    ],
    [
     "/websockets/binary/001.html?wss",
     {}
    ]
  ],

Now the two test names are websockets/binary/001.html and websockets/binary/001.html?wss. The latter, unfortunately, breaks a few presumptions in our infra, namely how to derive actual/expected output filenames from a test name. We currently simply take the basename of the test name, and append -expected.txt or -actual.txt. So both tests resolve to websockets/binary/001-{expected,actual}.txt.

Two tools are known to have problems:
1. test result viewer https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/fast/harness/results.html?q=results.html&sq=package:chromium&l=437
2. webkit-patch rebaseline https://cs.chromium.org/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py?type=cs&q=_test_root&l=83


The actual output files in the result directory have names like 001.html_wss-actual.txt. Presumably, the name mangling of the output files is done differently.

We shall decide on a consistent naming convention for this case, and then go over all the places that need to mangle test names.
 
Cc: robertma@chromium.org
 Issue 797743  has been merged into this issue.
Summary: [WPT] Support test names containing ? (URL params) (was: [WPT] Support test names containing ? (e.g. "001.html?wss", generated by the "variant" meta tag))
There are more URL params supported by wptserve, e.g. run_type.

Support was added to webkitpy along with .any.js/.worker.js in https://crrev.com/c/461722.

The code responsible for mangling output filenames is at https://cs.chromium.org/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py?q=test_result_writer&dr&l=150
Cc: -robertma@chromium.org
Owner: robertma@chromium.org
Status: Started (was: Available)
So essentially all the ?wss ?run_type etc. tests are broken right now because we can't find the right baselines. This is rather high-priority. Starting to solve the following issues in order:

1. baseline searching
2. rebaseline
3. result viewer
Blockedon: 801357
One more item to fix: TestImporter.update_all_test_expectations_files
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 17 2018

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

commit e94065df8cc8e51a9801105460739c80d5c490ec
Author: Robert Ma <robertma@chromium.org>
Date: Wed Jan 17 18:27:30 2018

[webkitpy] Supports query strings in layout test names

Previously, only test_result_writer had a special case dealing with test
names containing query strings, which was not sufficient.

This CL unifies all test name / output filename mangling into base.Port.
Rebaseline and TestResultWriter now both call Port.output_filename.

The mangling rule was also changed for better consistency. For example,
external/wpt/foo.html?wss would previously have baseline named
external/wpt/foo.html_wss-expected.txt; but now it'll be named
external/wpt/foo_wss-expected.txt. The change of naming schema won't be
a big trouble, because tests with query strings in their names didn't
work and couldn't be rebaselined anyway.

Lastly, this CL also fixes some code health issues nearby (outdated
docs, duplicate definitions, dead code, etc.).

Bug: 800570
Change-Id: If24c3e1be8f2f665ca133066bf9bc9acdf342858
Reviewed-on: https://chromium-review.googlesource.com/868639
Commit-Queue: Robert Ma <robertma@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529815}
[modify] https://crrev.com/e94065df8cc8e51a9801105460739c80d5c490ec/third_party/WebKit/Tools/Scripts/webkitpy/common/system/filesystem.py
[modify] https://crrev.com/e94065df8cc8e51a9801105460739c80d5c490ec/third_party/WebKit/Tools/Scripts/webkitpy/common/system/filesystem_mock.py
[modify] https://crrev.com/e94065df8cc8e51a9801105460739c80d5c490ec/third_party/WebKit/Tools/Scripts/webkitpy/common/system/filesystem_unittest.py
[modify] https://crrev.com/e94065df8cc8e51a9801105460739c80d5c490ec/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py
[modify] https://crrev.com/e94065df8cc8e51a9801105460739c80d5c490ec/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py
[modify] https://crrev.com/e94065df8cc8e51a9801105460739c80d5c490ec/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
[modify] https://crrev.com/e94065df8cc8e51a9801105460739c80d5c490ec/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base.py
[modify] https://crrev.com/e94065df8cc8e51a9801105460739c80d5c490ec/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base_unittest.py
[modify] https://crrev.com/e94065df8cc8e51a9801105460739c80d5c490ec/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 17 2018

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

commit 0337ce8d62dae6855170876d1cedc0ac5c632114
Author: Robert Ma <robertma@chromium.org>
Date: Wed Jan 17 22:07:49 2018

Support test names with query strings in result viewer

Python-side changes (run-webkit-tests) were made at
https://crrev.com/c/868639 . This CL adjusts the test result viewer
accordingly.

Bug: 800570
Change-Id: I180aa57b9c84b16e812594a5b48c9a5c1c2e5620
Reviewed-on: https://chromium-review.googlesource.com/871473
Reviewed-by: Aleks Totic <atotic@chromium.org>
Commit-Queue: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529910}
[modify] https://crrev.com/0337ce8d62dae6855170876d1cedc0ac5c632114/third_party/WebKit/LayoutTests/fast/harness/results.html

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 17 2018

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 18 2018

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

commit 901dfc8c50721bc2fa7a4513b21e685894655539
Author: Robert Ma <robertma@chromium.org>
Date: Thu Jan 18 21:53:19 2018

Rebaseline tests with query strings in the names

Apart from a few timed out tests, one test has non-deterministic output, and
the other is pass-failure flaky.

Bug: 800570, 709227,  803200 , 803558
Change-Id: I86d5a14f26fcab6b862cd2e1b2108c10db4ee94e
Reviewed-on: https://chromium-review.googlesource.com/871138
Commit-Queue: Robert Ma <robertma@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530284}
[modify] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/html/syntax/parsing/html5lib_menuitem-element_run_type=uri-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/html/syntax/parsing/html5lib_menuitem-element_run_type=write-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/html/syntax/parsing/html5lib_menuitem-element_run_type=write_single-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/html/syntax/parsing/html5lib_template_run_type=uri-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/html/syntax/parsing/html5lib_template_run_type=write-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/html/syntax/parsing/html5lib_template_run_type=write_single-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/html/syntax/parsing/html5lib_tests11_run_type=uri-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/html/syntax/parsing/html5lib_tests11_run_type=write-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/html/syntax/parsing/html5lib_tests11_run_type=write_single-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/html/syntax/parsing/html5lib_tests25_run_type=uri-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/html/syntax/parsing/html5lib_tests25_run_type=write-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/html/syntax/parsing/html5lib_tests25_run_type=write_single-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/html/syntax/parsing/html5lib_webkit02_run_type=uri-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/html/syntax/parsing/html5lib_webkit02_run_type=write-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/html/syntax/parsing/html5lib_webkit02_run_type=write_single-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/websockets/constructor/002-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/websockets/constructor/002_wss-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/websockets/cookies/007-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/websockets/cookies/007_wss-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/websockets/interfaces/WebSocket/close/close-connecting-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/websockets/interfaces/WebSocket/close/close-connecting_wss-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/websockets/interfaces/WebSocket/close/close-nested-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/websockets/interfaces/WebSocket/close/close-nested_wss-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/websockets/interfaces/WebSocket/readyState/003-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/websockets/interfaces/WebSocket/readyState/003_wss-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/websockets/unload-a-document/002-expected.txt
[add] https://crrev.com/901dfc8c50721bc2fa7a4513b21e685894655539/third_party/WebKit/LayoutTests/external/wpt/websockets/unload-a-document/002_wss-expected.txt

Status: Fixed (was: Started)
Everything fixed except TestImporter.update_all_test_expectations_files, which is tracked separately
Status: Started (was: Fixed)
Actually, found one more place to fix: orphaned baseline removal
Project Member

Comment 11 by bugdroid1@chromium.org, Jan 19 2018

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

commit fe4fd1988327508f61cc4d2bd3ed11035570c32e
Author: Robert Ma <robertma@chromium.org>
Date: Fri Jan 19 01:53:01 2018

[WPT import] Temporarily disable orphaned baseline removal

Importer is now trying to remove baselines for test with query strings
in the names, e.g.
https://chromium-review.googlesource.com/c/chromium/src/+/874798/1

We were lucky as the CL above didn't cause any actual damage (the
rebaseline step added all removed baselines back), but it's too risky.
Temporarily disable this feature before I fix it.

TBR=qyearsley

Bug: 800570
Change-Id: Id1f4b484badb7e2a651dbf6e37ccffe3d25fe7eb
Reviewed-on: https://chromium-review.googlesource.com/874726
Reviewed-by: Robert Ma <robertma@chromium.org>
Commit-Queue: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530388}
[modify] https://crrev.com/fe4fd1988327508f61cc4d2bd3ed11035570c32e/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py

Friendly ping from ecosystem-infra sheriff. It's been 60 days since last activity on this bug. Any update?
Status: Assigned (was: Started)
Thanks for the reminder. I'll work on the blocker issue 801357 first. Meanwhile, changing the status to assigned as I'm not currently working on this issue.
Blockedon: 831975
Project Member

Comment 15 by bugdroid1@chromium.org, May 4 2018

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

commit 6f0b5d97163ec24c5651475285449890d316734b
Author: Robert Ma <robertma@chromium.org>
Date: Fri May 04 14:15:35 2018

[blinkpy] Find manifest items for tests by URL

When running WPT in web_tests, test names are more like URLs instead of
file paths (to be precise, the part of a test name after external/wpt is
a WPT URL), because WPT can generate multiple tests (variations) for a
single test file (e.g. .any.js, ?run_type).

Therefore, when asking questions like "is a test slow?", we need to find
the manifest item for that test by URL instead of by file path; whereas
if we want to know if a file in external/wpt is a test file, we find
the manifest item(s) for that file by file path.

This CL separates the two types of manifest queries, and chooses the
correct one for each call site. Besides, unit tests are improved to
cover the WPT variations (.any.js & ?run_type).

Bug:  831975 , 800570
Change-Id: I230d5ec7df06b7df1387da1b93788d6cae55d153
Reviewed-on: https://chromium-review.googlesource.com/1043160
Commit-Queue: Robert Ma <robertma@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556043}
[modify] https://crrev.com/6f0b5d97163ec24c5651475285449890d316734b/third_party/blink/tools/blinkpy/w3c/wpt_manifest.py
[modify] https://crrev.com/6f0b5d97163ec24c5651475285449890d316734b/third_party/blink/tools/blinkpy/web_tests/port/base.py
[modify] https://crrev.com/6f0b5d97163ec24c5651475285449890d316734b/third_party/blink/tools/blinkpy/web_tests/port/base_unittest.py

Project Member

Comment 16 by bugdroid1@chromium.org, May 4 2018

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

commit 266b8bce041014954439d75c47607d38ebea4cb8
Author: CJ DiMeglio <lethalantidote@chromium.org>
Date: Fri May 04 16:31:25 2018

Revert "[blinkpy] Find manifest items for tests by URL"

This reverts commit 6f0b5d97163ec24c5651475285449890d316734b.

Reason for revert: Causing Webkit Linux Trusty Leak to fail
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20Leak/

Original change's description:
> [blinkpy] Find manifest items for tests by URL
> 
> When running WPT in web_tests, test names are more like URLs instead of
> file paths (to be precise, the part of a test name after external/wpt is
> a WPT URL), because WPT can generate multiple tests (variations) for a
> single test file (e.g. .any.js, ?run_type).
> 
> Therefore, when asking questions like "is a test slow?", we need to find
> the manifest item for that test by URL instead of by file path; whereas
> if we want to know if a file in external/wpt is a test file, we find
> the manifest item(s) for that file by file path.
> 
> This CL separates the two types of manifest queries, and chooses the
> correct one for each call site. Besides, unit tests are improved to
> cover the WPT variations (.any.js & ?run_type).
> 
> Bug:  831975 , 800570
> Change-Id: I230d5ec7df06b7df1387da1b93788d6cae55d153
> Reviewed-on: https://chromium-review.googlesource.com/1043160
> Commit-Queue: Robert Ma <robertma@chromium.org>
> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#556043}

TBR=qyearsley@chromium.org,robertma@chromium.org

Change-Id: I5fb2c257967e32d1e4fce4277c10d77635e303d4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  831975 , 800570
Reviewed-on: https://chromium-review.googlesource.com/1044567
Reviewed-by: CJ DiMeglio <lethalantidote@chromium.org>
Commit-Queue: CJ DiMeglio <lethalantidote@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556083}
[modify] https://crrev.com/266b8bce041014954439d75c47607d38ebea4cb8/third_party/blink/tools/blinkpy/w3c/wpt_manifest.py
[modify] https://crrev.com/266b8bce041014954439d75c47607d38ebea4cb8/third_party/blink/tools/blinkpy/web_tests/port/base.py
[modify] https://crrev.com/266b8bce041014954439d75c47607d38ebea4cb8/third_party/blink/tools/blinkpy/web_tests/port/base_unittest.py

Project Member

Comment 17 by bugdroid1@chromium.org, May 4 2018

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

commit a244e7595ef2030d72c6bac7dc8880bdac573ffa
Author: Robert Ma <robertma@chromium.org>
Date: Fri May 04 18:27:42 2018

Reland "[blinkpy] Find manifest items for tests by URL"

This is a reland of 6f0b5d97163ec24c5651475285449890d316734b

A baseline is added for a test that no longer (always) times out because
of this fix. The test has a few failing subtests so a baseline is needed.

Original change's description:
> [blinkpy] Find manifest items for tests by URL
>
> When running WPT in web_tests, test names are more like URLs instead of
> file paths (to be precise, the part of a test name after external/wpt is
> a WPT URL), because WPT can generate multiple tests (variations) for a
> single test file (e.g. .any.js, ?run_type).
>
> Therefore, when asking questions like "is a test slow?", we need to find
> the manifest item for that test by URL instead of by file path; whereas
> if we want to know if a file in external/wpt is a test file, we find
> the manifest item(s) for that file by file path.
>
> This CL separates the two types of manifest queries, and chooses the
> correct one for each call site. Besides, unit tests are improved to
> cover the WPT variations (.any.js & ?run_type).
>
> Bug:  831975 , 800570
> Change-Id: I230d5ec7df06b7df1387da1b93788d6cae55d153
> Reviewed-on: https://chromium-review.googlesource.com/1043160
> Commit-Queue: Robert Ma <robertma@chromium.org>
> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#556043}

Bug:  831975 , 800570
Change-Id: I65e818473153b134af8dc78677b1b678d196aada
Reviewed-on: https://chromium-review.googlesource.com/1044586
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Commit-Queue: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556126}
[add] https://crrev.com/a244e7595ef2030d72c6bac7dc8880bdac573ffa/third_party/WebKit/LayoutTests/external/wpt/WebCryptoAPI/derive_bits_keys/pbkdf2.https.worker-expected.txt
[modify] https://crrev.com/a244e7595ef2030d72c6bac7dc8880bdac573ffa/third_party/blink/tools/blinkpy/w3c/wpt_manifest.py
[modify] https://crrev.com/a244e7595ef2030d72c6bac7dc8880bdac573ffa/third_party/blink/tools/blinkpy/web_tests/port/base.py
[modify] https://crrev.com/a244e7595ef2030d72c6bac7dc8880bdac573ffa/third_party/blink/tools/blinkpy/web_tests/port/base_unittest.py

Poked issue 801357.
Cc: jeffcarp@chromium.org
 Issue 751844  has been merged into this issue.
Ping!

Sign in to add a comment