search

GoogleChromeLabs / chromium-bidi

Implementation of WebDriver BiDi for Chromium
https://googlechromelabs.github.io/chromium-bidi/
Apache License 2.0
92 stars 32 forks source link

Document how to update WPT expectations _in chromium_ when rolling the mapper #1306

Closed thiagowfx closed 12 months ago

thiagowfx commented 1 year ago

As of Aug 29th 2023 (https://chromium-review.googlesource.com/c/chromium/src/+/4789885), whenever we roll the mapper into chromium, we need to update the WPT expectations checked into the blink source tree.

Goal: Document this process.

Context: https://bugs.chromium.org/p/chromium/issues/detail?id=1482539#c2

cc @Lightning00Blade

thiagowfx commented 1 year ago

Note: Followed up internally today.

WeizhongX commented 1 year ago

Once the CL is ready, run the command below in your local checkout repo: //third_party/blink/tools/blink_tool.py update-metadata --build=linux-blink-rel

After the test completes on the builder, rerun that command to update metadata. Commit the changes (if there are any), and upload the new patch. You should be ready to go.

The document is here: https://chromium.googlesource.com/chromium/src/+/HEAD/docs/testing/web_platform_tests_wptrunner.md#updating-expectations

thiagowfx commented 1 year ago

@WeizhongX We have an issue: https://chromium-review.googlesource.com/c/chromium/src/+/4941659

In this CL, third_party/blink/tools/blink_tool.py update-metadata --build="linux-blink-rel" --verbose is a no-op. Example output:

% third_party/blink/tools/blink_tool.py update-metadata --build="linux-blink-rel" --verbose
2023-10-18 20:13:54,445 - blinkpy.common.system.log_utils: [DEBUG] Debug logging enabled.
2023-10-18 20:13:54,469 - blinkpy.common.system.executive: [DEBUG] "sw_vers -productVersion" took 0.02s
2023-10-18 20:13:54,483 - blinkpy.common.system.executive: [DEBUG] "sw_vers -productVersion" took 0.01s
2023-10-18 20:13:54,515 - manifest: [DEBUG] Opening manifest at /Users/tperrotta/Projects/chrome/src/third_party/blink/web_tests/wpt_internal/MANIFEST.json
2023-10-18 20:13:54,517 - manifest: [INFO] Updating manifest
2023-10-18 20:14:02,649 - manifest: [DEBUG] Computing manifest update for 2 items
2023-10-18 20:14:02,713 - manifest: [DEBUG] Opening manifest at /Users/tperrotta/Projects/chrome/src/third_party/blink/web_tests/external/wpt/MANIFEST.json
2023-10-18 20:14:02,832 - manifest: [INFO] Updating manifest
2023-10-18 20:14:14,683 - manifest: [DEBUG] Computing manifest update for 1 items
2023-10-18 20:14:17,727 - blinkpy.tool.commands.update_metadata: [DEBUG] Performing update with properties: 'flag_specific', 'os', 'port', 'product', 'virtual_suite'
2023-10-18 20:14:19,388 - blinkpy.common.system.executive: [DEBUG] "git cl issue" took 0.72s
2023-10-18 20:14:19,784 - blinkpy.common.system.executive: [DEBUG] "git cl issue" took 0.40s
2023-10-18 20:14:20,696 - blinkpy.common.system.executive: [DEBUG] "git cl status --field=patch" took 0.91s
2023-10-18 20:14:21,521 - urllib3.connectionpool: [DEBUG] Starting new HTTPS connection (1): cr-buildbucket.appspot.com:443
2023-10-18 20:14:21,823 - urllib3.connectionpool: [DEBUG] https://cr-buildbucket.appspot.com:443 "POST /prpc/buildbucket.v2.Builds/SearchBuilds HTTP/1.1" 200 None
2023-10-18 20:14:23,491 - urllib3.connectionpool: [DEBUG] https://cr-buildbucket.appspot.com:443 "POST /prpc/buildbucket.v2.Builds/Batch HTTP/1.1" 200 None
2023-10-18 20:14:23,507 - blinkpy.tool.commands.build_resolver: [INFO] All builds finished.
2023-10-18 20:14:24,035 - urllib3.connectionpool: [DEBUG] Starting new HTTPS connection (1): results.api.cr.dev:443
2023-10-18 20:14:24,339 - urllib3.connectionpool: [DEBUG] https://results.api.cr.dev:443 "POST /prpc/luci.resultdb.v1.ResultDB/QueryArtifacts HTTP/1.1" 200 8
2023-10-18 20:14:24,339 - blinkpy.tool.commands.update_metadata: [WARNING] All builds are missing report artifacts.
2023-10-18 20:14:24,339 - blinkpy.tool.commands.update_metadata: [WARNING] No reports to process.
2023-10-18 20:14:25,175 - blinkpy.tool.commands.update_metadata: [INFO] Updating expectations for up to 47796 test files.
2023-10-18 20:14:25,175 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'FileAPI/Blob-methods-from-detached-frame.html'
2023-10-18 20:14:25,175 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'FileAPI/BlobURL/cross-partition.tentative.https.html'
2023-10-18 20:14:25,175 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'FileAPI/FileReader/progress_event_bubbles_cancelable.html'
[...]
2023-10-18 20:14:27,994 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'xhr/xmlhttprequest-sync-block-defer-scripts.html'
2023-10-18 20:14:27,994 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'xhr/xmlhttprequest-sync-block-scripts.html'
2023-10-18 20:14:27,994 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'xhr/xmlhttprequest-sync-default-feature-policy.sub.html'
2023-10-18 20:14:27,994 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'xhr/xmlhttprequest-sync-not-hang-scriptloader.html'
2023-10-18 20:14:27,994 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'xhr/xmlhttprequest-timeout-aborted.html'
2023-10-18 20:14:27,994 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'xhr/xmlhttprequest-timeout-abortedonmain.html'
2023-10-18 20:14:27,994 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'xhr/xmlhttprequest-timeout-overrides.html'
2023-10-18 20:14:27,994 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'xhr/xmlhttprequest-timeout-overridesexpires.html'
2023-10-18 20:14:27,994 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'xhr/xmlhttprequest-timeout-reused.html'
2023-10-18 20:14:27,994 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'xhr/xmlhttprequest-timeout-simple.html'
2023-10-18 20:14:27,994 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'xhr/xmlhttprequest-timeout-synconmain.html'
2023-10-18 20:14:27,994 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'xhr/xmlhttprequest-timeout-twice.html'
2023-10-18 20:14:27,994 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'xhr/xmlhttprequest-timeout-worker-aborted.html'
2023-10-18 20:14:27,994 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'xhr/xmlhttprequest-timeout-worker-overrides.html'
2023-10-18 20:14:27,994 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'xhr/xmlhttprequest-timeout-worker-overridesexpires.html'
2023-10-18 20:14:27,994 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'xhr/xmlhttprequest-timeout-worker-simple.html'
2023-10-18 20:14:27,994 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'xhr/xmlhttprequest-timeout-worker-synconworker.html'
2023-10-18 20:14:27,994 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'xhr/xmlhttprequest-timeout-worker-twice.html'
2023-10-18 20:14:27,994 - blinkpy.tool.commands.update_metadata: [DEBUG] No change needed for 'xhr/xmlhttprequest-unsent.htm'
2023-10-18 20:14:28,616 - blinkpy.tool.commands.update_metadata: [INFO] Staged 0 metadata files.
2023-10-18 20:14:28,616 - blinkpy.tool.commands.update_metadata: [DEBUG] Updated metadata (took 5.11s)
third_party/blink/tools/blink_tool.py update-metadata  --verbose  10.19s user 25.31s system 100% cpu 35.490 total
thiagowfx commented 1 year ago

That said, the build is failing: https://ci.chromium.org/ui/p/chromium/builders/try/linux-blink-rel/98702/overview

Why is the blink tool not picking up the failures?

thiagowfx commented 1 year ago

cc @jrandolf @sadym-chromium

WeizhongX commented 1 year ago

The logs says "2023-10-18 20:14:24,339 - blinkpy.tool.commands.update_metadata: [WARNING] No reports to process." Looking at step 36 both shards timed-out, and failed to upload wpt report for the tests. That's why "blink_tool.py update-metadata" would not do anything.

Do you know why the bots are timing out on this change?

But I think the logs can be improved. We should not produce additional logs when it says "No reports to process".

thiagowfx commented 1 year ago

Do you know why the bots are timing out on this change?

Not sure. Maybe it's a headless vs headful issue.

Regardless of the reason though, to unblock this CL, I suppose the only way to move forward would be to update the expectations manually, correct? (there are only ~half a dozen or so of them, so it's not a lot of work)

If we update them manually to TIMEOUT now, will other expectations in the future also get stuck because of this one?

thiagowfx commented 1 year ago

But I think the logs can be improved. We should not produce additional logs when it says "No reports to process".

Happy to file a bug in monorail if you point me out to the right component. Or maybe you can file one yourself if it doesn't take too long :-)

WeizhongX commented 1 year ago

If those tests take too long to run, we can disable those tests for now, either by using __dir__.ini or the ini file for a specific test, see example in [1] & [2]. Mark them as Timeout won't help as that will still cause the builder to timeout.

Yes we will file a crbug for ourselves to fix the log issue.

1: third_party/blink/web_tests/wpt_internal/attribution-reporting/__dir__.ini 2: third_party/blink/web_tests/external/wpt/mathml/relations/html5-tree/tabindex-002.html.ini

thiagowfx commented 1 year ago

@WeizhongX

Is this the correct way to disable it?

https://chromium-review.googlesource.com/c/chromium/src/+/4994305/2

Will __dir__.ini persist subsequent repo synchronizations?

WeizhongX commented 1 year ago

__dir__.ini LGTM and I put a +1 to the CL. .ini files are chromium only files, so will not be overwritten by wpt-importer.

I pulled your CL to my local repo, and run run_wpt_tests.py --release --test-type=wdspec -p chrome external/wpt/webdriver/tests/bidi/browsing_context/activate/ reports all tests are skipped.

Looking at the result page (https://chromium-layout-test-archives.storage.googleapis.com/results.html?json=chromium/try/linux-blink-rel/99238/webdriver_tests_suite%20%28with%20patch%29/full_results_jsonp.js), the tests are not actually skipped. That might be due to a recent change at our side. We are in the process to use test expectations/baselines for Wptrunner, so we will also do this for webdriver tests. The change have been reverted and I will include you for review when we reland that change.

I have retried linux-blink-rel on your CL. Let's see if the tests are successfully skipped this time.

thiagowfx commented 1 year ago

A few changes are coming:

TL;DR: .ini files will be removed from chromium/src, TestExpectations will be used instead.