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

Support finding EWS runs from GitHub PRs in update-test-expectations #18037

Merged

Conversation

gsnedders
Copy link
Contributor

@gsnedders gsnedders commented Sep 21, 2023

1a61da0

Support finding EWS runs from GitHub PRs in update-test-expectations
https://bugs.webkit.org/show_bug.cgi?id=257158

Reviewed by Jonathan Bedard.

This adds support for fetching layout test results based on GitHub PR
statuses. To do this, we refactor test_expectation_updater to be able to
support multiple sources of layout tests results, and add various
subcommands to it. With the subcommands added, we rename the script to
update-test-expectations, and add a legacy
update-test-expectations-from-bugzilla which calls the bugzilla
subcommand.

* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/mocks/remote/git_hub.py:
(GitHub._commit_response):
* Tools/Scripts/update-test-expectations: Copied from Tools/Scripts/update-test-expectations-from-bugzilla.
* Tools/Scripts/update-test-expectations-from-bugzilla:
* Tools/Scripts/webkitpy/common/net/bugzilla/results_fetcher.py:
(lookup_ews_results_from_pr):
(lookup_ews_results_from_repo_via_pr):
* Tools/Scripts/webkitpy/common/net/bugzilla/results_fetcher_unittest.py:
(ResultsFetcherTest.test_is_relevant_results):
(ResultsFetcherTest.test_lookup_ews_results_from_bugzilla):
(ResultsFetcherGitHubTest):
(ResultsFetcherGitHubTest.webserver):
* Tools/Scripts/webkitpy/common/net/bugzilla/test_expectation_updater.py:
(configure_logging.LogHandler):
(argument_parser):
(TestExpectationUpdater.__init__):
(TestExpectationUpdater.fetch_from_bugzilla):
(TestExpectationUpdater):
(TestExpectationUpdater.fetch_from_github_pr):
(TestExpectationUpdater.do_update):
(main):
* Tools/Scripts/webkitpy/common/net/bugzilla/test_expectation_updater_unittest.py:
(TestExpectationUpdaterTest.test_update_test_expectations):

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

454825b

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   πŸ§ͺ api-wpe
  πŸ§ͺ webkitpy   πŸ§ͺ ios-wk2-wpt   πŸ§ͺ mac-wk1   πŸ›  gtk
  πŸ§ͺ api-ios   πŸ§ͺ mac-wk2   πŸ§ͺ gtk-wk2
  πŸ›  tv   πŸ§ͺ mac-AS-debug-wk2   πŸ§ͺ api-gtk
  πŸ§ͺ services   πŸ›  tv-sim   πŸ§ͺ mac-wk2-stress
  πŸ›  watch
βœ… πŸ›  πŸ§ͺ unsafe-merge   πŸ›  watch-sim

@gsnedders gsnedders self-assigned this Sep 21, 2023
@gsnedders gsnedders added Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases skip-ews Applied to prevent a change from being run on EWS labels Sep 21, 2023
@gsnedders
Copy link
Contributor Author

If we care about being able to update results from post-commit, we probably want to be getting status checks from a commit, rather than from a PR. And given that's how both GitHub and Bitbucket Server have these in their APIs, that's probably where it should live for us too.

@gsnedders gsnedders force-pushed the results_fetcher_github_status branch from 9d5e0f5 to b268d30 Compare October 6, 2023 23:11
@gsnedders
Copy link
Contributor Author

Okay, trying to apply this to #20412:

% ./Tools/Scripts/update-test-expectations-from-bugzilla -d github-pr 
Updating results from bot mac-wk2 (generic)
mac-wk2 archive: https://ews-build.s3-us-west-2.amazonaws.com/macOS-AppleSilicon-Sonoma-Debug-WK2-Tests-EWS/da9a3797-1341.zip
Traceback (most recent call last):
  File "/Volumes/gsnedders/projects/Safari/OS2/./Tools/Scripts/update-test-expectations-from-bugzilla", line 35, in <module>
    sys.exit(test_expectation_updater.main(sys.argv[1:], sys.stdout, sys.stderr))
  File "/Volumes/gsnedders/projects/Safari/OS2/Tools/Scripts/webkitpy/common/net/bugzilla/test_expectation_updater.py", line 211, in main
    updater.do_update()
  File "/Volumes/gsnedders/projects/Safari/OS2/Tools/Scripts/webkitpy/common/net/bugzilla/test_expectation_updater.py", line 171, in do_update
    self._update_for_generic_bot(platform_name)
  File "/Volumes/gsnedders/projects/Safari/OS2/Tools/Scripts/webkitpy/common/net/bugzilla/test_expectation_updater.py", line 119, in _update_for_generic_bot
    for test_name, expected_content in self._tests_to_update(platform_name).items():
  File "/Volumes/gsnedders/projects/Safari/OS2/Tools/Scripts/webkitpy/common/net/bugzilla/test_expectation_updater.py", line 113, in _tests_to_update
    return {result: zip_file.read(TestResultWriter.actual_filename(result, self.filesystem)) for result in results_to_update}
  File "/Volumes/gsnedders/projects/Safari/OS2/Tools/Scripts/webkitpy/common/net/bugzilla/test_expectation_updater.py", line 113, in <dictcomp>
    return {result: zip_file.read(TestResultWriter.actual_filename(result, self.filesystem)) for result in results_to_update}
  File "/AppleInternal/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/zipfile.py", line 1463, in read
    with self.open(name, "r", pwd) as fp:
  File "/AppleInternal/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/zipfile.py", line 1502, in open
    zinfo = self.getinfo(name)
  File "/AppleInternal/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/zipfile.py", line 1429, in getinfo
    raise KeyError(
KeyError: "There is no item named 'imported/w3c/web-platform-tests/editing/other/delete-in-child-of-head.tentative-actual.txt' in the archive"

@gsnedders gsnedders removed the skip-ews Applied to prevent a change from being run on EWS label Nov 15, 2023
@gsnedders gsnedders changed the title Support finding EWS runs from GitHub in update-test-expectations-from-bugzilla Support finding EWS runs from GitHub PRs in update-test-expectations Dec 16, 2023
source_remote = repository.default_remote
remote_repo = repository.remote(name=source_remote)
if not remote_repo.pull_requests:
msg = "Remote repo doesn't support pull requests"
Copy link
Member

Choose a reason for hiding this comment

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

Why not just raise ValueError directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see https://docs.astral.sh/ruff/rules/raw-string-in-exception/; essentially it avoids the string being duplicated in the traceback, which makes it somewhat harder to read


if existing_pr is None:
msg = "No PR found for current branch"
raise ValueError(msg)
Copy link
Member

Choose a reason for hiding this comment

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

Ditto on raising ValueError(<str>) directly.

@gsnedders gsnedders marked this pull request as ready for review December 16, 2023 00:37
@gsnedders gsnedders added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Dec 16, 2023
https://bugs.webkit.org/show_bug.cgi?id=257158

Reviewed by Jonathan Bedard.

This adds support for fetching layout test results based on GitHub PR
statuses. To do this, we refactor test_expectation_updater to be able to
support multiple sources of layout tests results, and add various
subcommands to it. With the subcommands added, we rename the script to
update-test-expectations, and add a legacy
update-test-expectations-from-bugzilla which calls the bugzilla
subcommand.

* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/mocks/remote/git_hub.py:
(GitHub._commit_response):
* Tools/Scripts/update-test-expectations: Copied from Tools/Scripts/update-test-expectations-from-bugzilla.
* Tools/Scripts/update-test-expectations-from-bugzilla:
* Tools/Scripts/webkitpy/common/net/bugzilla/results_fetcher.py:
(lookup_ews_results_from_pr):
(lookup_ews_results_from_repo_via_pr):
* Tools/Scripts/webkitpy/common/net/bugzilla/results_fetcher_unittest.py:
(ResultsFetcherTest.test_is_relevant_results):
(ResultsFetcherTest.test_lookup_ews_results_from_bugzilla):
(ResultsFetcherGitHubTest):
(ResultsFetcherGitHubTest.webserver):
* Tools/Scripts/webkitpy/common/net/bugzilla/test_expectation_updater.py:
(configure_logging.LogHandler):
(argument_parser):
(TestExpectationUpdater.__init__):
(TestExpectationUpdater.fetch_from_bugzilla):
(TestExpectationUpdater):
(TestExpectationUpdater.fetch_from_github_pr):
(TestExpectationUpdater.do_update):
(main):
* Tools/Scripts/webkitpy/common/net/bugzilla/test_expectation_updater_unittest.py:
(TestExpectationUpdaterTest.test_update_test_expectations):

Canonical link: https://commits.webkit.org/272152@main
@webkit-commit-queue webkit-commit-queue merged commit 1a61da0 into WebKit:main Dec 16, 2023
@webkit-commit-queue
Copy link
Collaborator

Committed 272152@main (1a61da0): https://commits.webkit.org/272152@main

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

@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Dec 16, 2023
@gsnedders gsnedders deleted the results_fetcher_github_status branch December 16, 2023 01:09
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