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

wpt_config_json should read from original config.json #11255

Merged
merged 1 commit into from Mar 8, 2023

Conversation

gsnedders
Copy link
Contributor

@gsnedders gsnedders commented Mar 8, 2023

cd83321

wpt_config_json should read from original config.json
https://bugs.webkit.org/show_bug.cgi?id=253602
rdar://106437906

Reviewed by Ben Nham.

This avoids any risk of a stale, copied config.json being read, as
might happen if a prior run-webkit-tests run exited uncleanly.

* Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py:
(wpt_config_json): This function now reads from the original, non-copied file and does the mutation.
(WebPlatformTestServer._prepare_config): Make this merely call wpt_config_json and write it to the new location.

Canonical link: https://commits.webkit.org/261389@main

9430a9d

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug   πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl   πŸ§ͺ ios-wk2   πŸ§ͺ api-mac   πŸ›  gtk
βœ… πŸ§ͺ webkitpy   πŸ§ͺ api-ios   πŸ§ͺ mac-wk1 ❌ πŸ§ͺ gtk-wk2
βœ… πŸ›  tv   πŸ§ͺ mac-wk2   πŸ§ͺ api-gtk
βœ… πŸ›  tv-sim   πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  watch βœ… πŸ§ͺ mac-wk2-stress
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch-sim

@gsnedders gsnedders self-assigned this Mar 8, 2023
@gsnedders gsnedders added the Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases label Mar 8, 2023
@bnham
Copy link
Contributor

bnham commented Mar 8, 2023

Just to make sure I understand what the PR is fixing, was this the problem that was occurring?

In the past we were calling wpt_config_json() before LayoutTests/imported/w3c/web-platform-tests/config.json had been written out via start() => _prepare_config(). This means that we could read a stale LayoutTests/imported/w3c/web-platform-tests/config.json in wpt_config_json() if the server exited uncleanly.

@gsnedders
Copy link
Contributor Author

gsnedders commented Mar 8, 2023

Just to make sure I understand what the PR is fixing, was this the problem that was occurring?

In the past we were calling wpt_config_json() before LayoutTests/imported/w3c/web-platform-tests/config.json had been written out via start() => _prepare_config(). This means that we could read a stale LayoutTests/imported/w3c/web-platform-tests/config.json in wpt_config_json() if the server exited uncleanly.

Correct. (Or if something else had left a config.json there, somehow.)

Previously in base_http_url etc. we ended up in the if not config when we first called those functions, but with the stale config.json there we ended up reading the stale config.json, and in this case we didn't have the h2 key.

@bnham bnham self-requested a review March 8, 2023 22:08
@gsnedders gsnedders added the merge-queue Applied to send a pull request to merge-queue label Mar 8, 2023
https://bugs.webkit.org/show_bug.cgi?id=253602
rdar://106437906

Reviewed by Ben Nham.

This avoids any risk of a stale, copied config.json being read, as
might happen if a prior run-webkit-tests run exited uncleanly.

* Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py:
(wpt_config_json): This function now reads from the original, non-copied file and does the mutation.
(WebPlatformTestServer._prepare_config): Make this merely call wpt_config_json and write it to the new location.

Canonical link: https://commits.webkit.org/261389@main
@webkit-commit-queue
Copy link
Collaborator

Committed 261389@main (cd83321): https://commits.webkit.org/261389@main

Reviewed commits have been landed. Closing PR #11255 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Mar 8, 2023
@gsnedders gsnedders deleted the wpt-config-mismatch branch March 9, 2023 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases
Projects
None yet
4 participants