From 43a97878ce14b72f0981164f87f2e35e14151312 Mon Sep 17 00:00:00 2001 From: Daniel Baumann Date: Sun, 7 Apr 2024 11:22:09 +0200 Subject: Adding upstream version 110.0.1. Signed-off-by: Daniel Baumann --- .../tests/docs/reviewing-tests/checklist.md | 157 +++++++++++++++++++++ .../tests/docs/reviewing-tests/email.md | 7 + .../web-platform/tests/docs/reviewing-tests/git.md | 83 +++++++++++ .../tests/docs/reviewing-tests/index.md | 62 ++++++++ .../tests/docs/reviewing-tests/reverting.md | 28 ++++ 5 files changed, 337 insertions(+) create mode 100644 testing/web-platform/tests/docs/reviewing-tests/checklist.md create mode 100644 testing/web-platform/tests/docs/reviewing-tests/email.md create mode 100644 testing/web-platform/tests/docs/reviewing-tests/git.md create mode 100644 testing/web-platform/tests/docs/reviewing-tests/index.md create mode 100644 testing/web-platform/tests/docs/reviewing-tests/reverting.md (limited to 'testing/web-platform/tests/docs/reviewing-tests') diff --git a/testing/web-platform/tests/docs/reviewing-tests/checklist.md b/testing/web-platform/tests/docs/reviewing-tests/checklist.md new file mode 100644 index 0000000000..be0f4d134e --- /dev/null +++ b/testing/web-platform/tests/docs/reviewing-tests/checklist.md @@ -0,0 +1,157 @@ +# Review Checklist + +The following checklist is provided as a guideline to assist in reviewing +tests; in case of any contradiction with requirements stated elsewhere in the +documentation it should be ignored +(please [file a bug](https://github.com/web-platform-tests/wpt/issues/new)!). + +As noted on the [reviewing tests](./index.md) page, nits need not block PRs +from landing. + + +## All tests + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +## Reftests Only + + + + + + + + + + + + +## Script Tests Only + + + + + + + + + + + + +## Visual Tests Only + + + + + + + + diff --git a/testing/web-platform/tests/docs/reviewing-tests/email.md b/testing/web-platform/tests/docs/reviewing-tests/email.md new file mode 100644 index 0000000000..55bbf44367 --- /dev/null +++ b/testing/web-platform/tests/docs/reviewing-tests/email.md @@ -0,0 +1,7 @@ +# Email Filters + +See the [GitHub support page](https://help.github.com/articles/about-email-notifications/) +for how to filter certain types of email notifications. These are the most +useful `Cc` addresses when reviewing in web-platform-tests: +- `review_requested@noreply.github.com` when you are added as a (suggested) `reviewer` on a pull request. +- `assign@noreply.github.com` when you are added as the `assignee` (i.e. as _the_ reviewer) on a pull request. diff --git a/testing/web-platform/tests/docs/reviewing-tests/git.md b/testing/web-platform/tests/docs/reviewing-tests/git.md new file mode 100644 index 0000000000..b74b4b77ae --- /dev/null +++ b/testing/web-platform/tests/docs/reviewing-tests/git.md @@ -0,0 +1,83 @@ +# Working with Pull Requests as a reviewer + +In order to do a thorough review, +it is sometimes desirable to have a local copy of the tests one wishes to review. + +Reviewing tests also often results in wanting a few things to be changed. +Generally, the reviewer should ask the author to make the desired changes. +However, sometimes the original author does not respond to the requests, +or the changes are so trivial (e.g. fixing a typo) +that bothering the original author seems like a waste of time. + +Here is how to do all that. + +## Trivial cases + +If it is possible to review the tests without a local copy, +but the reviewer still wants to make some simple tweaks to the tests before merging, +it is possible to do so via the Github web UI. + +1. Open the pull request. E.g. https://github.com/web-platform-tests/wpt/pull/1234 +2. Go to the ![Files changed](../assets/files-changed.png) view (e.g. https://github.com/web-platform-tests/wpt/pull/1234/files) +3. Locate the files you wish to change, and click the ![pencil](../assets/pencil-icon.png) icon in the upper right corner +4. Make the desired change +5. Write a commit message (including a good title) at the bottom +6. Make sure the ![Commit directly to the [name-of-the-PR-branch] branch.](../assets/commit-directly.png) radio button is selected. + + _Note: If the PR predates the introduction of this feature by Github, + or if the author of the PR has disabled write-access by reviewers to the PR branch, + this may not be available, + and your only option would be to commit to a new branch, creating a new PR._ +7. Click the ![Commit Changes](../assets/commitbtn.png) button. + + +## The Normal Way + +This is how to import the Pull Request's branch into your existing local +checkout of the repository. If you don't have one, go [fork][fork], +[clone][clone], and [configure][configure] it. + +1. Move into your local clone: `cd wherever-you-put-your-repo` +2. Add a remote for the PR author's repo: `git remote add git://github.com//.git` +3. Fetch the PR: `git fetch ` +4. Checkout that branch: `git checkout ` + + _The relevant ``, ``, and `` can be found by looking for this sentence in on the Github page of the PR: + ![Add more commits by pushing to the name-of-the-PR-branch branch on author-id/repo-name.](../assets/more-commits.png)_ + +If all you meant to do was reviewing files locally, you're all set. +If you wish to make changes to the PR branch: + +1. Make changes and [commit][commit] normally +2. Push your changes upstream: `git push ` + + _Note: If the PR predates the introduction of this feature by Github, + or if the author of the PR has disabled write-access by reviewers to the PR branch, + this will not work, and you will need to use the alternative described below._ + +If, instead of modifying the existing PR, you wish to make a new one based on it: + +1. Set up a new branch that contains the existing PR by doing one of the following: + 1. Create a new branch from the tip of the PR: + `git branch && git checkout ` + 2. Create a new branch from `master` and merge the PR into it: + `git branch master && git checkout && git merge ` +2. Make changes and [commit][commit] normally +3. Push your changes to **your** repo: `git push origin ` +4. Go to the Github Web UI to [submit a new Pull Request][submit]. + + _Note: You should also close the original pull request._ + +When you're done reviewing or making changes, +you can delete the branch: `git branch -d ` +(use `-D` instead of `-d` to delete a branch that has not been merged into master yet). + +If you do not expect work with more PRs from the same author, +you may also discard your connection to their repo: +`git remote remove ` + +[clone]: ../writing-tests/github-intro.html#clone +[commit]: ../writing-tests/github-intro.html#commit +[configure]: ../writing-tests/github-intro.html#configure-remote-upstream +[fork]: ../writing-tests/github-intro.html#fork-the-test-repository +[submit]: ../writing-tests/github-intro.html#submit diff --git a/testing/web-platform/tests/docs/reviewing-tests/index.md b/testing/web-platform/tests/docs/reviewing-tests/index.md new file mode 100644 index 0000000000..e313f84596 --- /dev/null +++ b/testing/web-platform/tests/docs/reviewing-tests/index.md @@ -0,0 +1,62 @@ +# Reviewing Tests + +In order to encourage a high level of quality in the W3C test +suites, test contributions must be reviewed by a peer. + +```eval_rst +.. toctree:: + :maxdepth: 1 + + checklist + email + git + reverting +``` + +## Test Review Policy + +The reviewer can be anyone (other than the original test author) that +has the required experience with both the spec under test and with +the [general test guidelines](../writing-tests/general-guidelines). + +The review must happen in public, but there is no requirement for it +to happen in any specific location. In particular if a vendor is +submitting tests that have already been publicly reviewed in their own +review system, that review may be carried forward. For other submissions, we +recommend using GitHub's built-in review tools. + +Regardless of what review tool is used, the review must be clearly +linked in the pull request. + +In general, we tend on the side of merging things with nits (i.e., +anything sub-optimal that isn't absolutely required to be right) and +then opening issues to leaving pull requests open indefinitely waiting +on the original submitter to fix them; when tests are being upstreamed +from vendors it is frequently the case that the author has moved on to +working on other things as tests frequently only get pushed upstream +once the code lands in their implementation. + +To assist with test reviews, a [review checklist](checklist) is available. + +[GitHub.com allows reviewers to formally signal their approval of a pull +request through a dedicated user +interface.](https://help.github.com/en/articles/about-pull-request-reviews) +Every pull request submitted to WPT must be approved by at least one project +collaborator before it can be merged. + +## Notifications + +META.yml files are used to indicate who should be notified of pull +requests. If you are interested in receiving notifications of proposed +changes to tests in a given directory, feel free to add your GitHub account +username to the `suggested_reviewers` list in the META.yml file. + +## Finding contributions to review + +Here are a few search filters to find things to review: + +* [Open PRs (excluding vendor exports)](https://github.com/web-platform-tests/wpt/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Aopen+-label%3A%22mozilla%3Agecko-sync%22+-label%3A%22chromium-export%22+-label%3A%22webkit-export%22+-label%3A%22servo-export%22+-label%3Avendor-imports) +* [Reviewed but still open PRs (excluding vendor exports)](https://github.com/web-platform-tests/wpt/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Aopen+-label%3Amozilla%3Agecko-sync+-label%3Achromium-export+-label%3Awebkit-export+-label%3Aservo-export+-label%3Avendor-imports+review%3Aapproved+-label%3A%22do+not+merge+yet%22+-label%3A%22status%3Aneeds-spec-decision%22) (Merge? Something left to fix? Ping other reviewer?) +* [Open PRs without reviewers](https://github.com/web-platform-tests/wpt/pulls?q=is%3Apr+is%3Aopen+label%3Astatus%3Aneeds-reviewers) +* [Open PRs with label `infra` (excluding vendor exports)](https://github.com/web-platform-tests/wpt/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Aopen+label%3Ainfra+-label%3A%22mozilla%3Agecko-sync%22+-label%3A%22chromium-export%22+-label%3A%22webkit-export%22+-label%3A%22servo-export%22+-label%3Avendor-imports) +* [Open PRs with label `docs` (excluding vendor exports)](https://github.com/web-platform-tests/wpt/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Aopen+label%3Adocs+-label%3A%22mozilla%3Agecko-sync%22+-label%3A%22chromium-export%22+-label%3A%22webkit-export%22+-label%3A%22servo-export%22+-label%3Avendor-imports) diff --git a/testing/web-platform/tests/docs/reviewing-tests/reverting.md b/testing/web-platform/tests/docs/reviewing-tests/reverting.md new file mode 100644 index 0000000000..277ccb047a --- /dev/null +++ b/testing/web-platform/tests/docs/reviewing-tests/reverting.md @@ -0,0 +1,28 @@ +# Reverting Changes + +Testing is imperfect and from time to time changes are merged into master which +break things for users of web-platform-tests. Such breakage can include: + + * Failures in Travis or Taskcluster runs for this repository, either on the + master branch or on pull requests following the breaking change. + + * Breakage in browser engine repositories which import and run + web-platform-tests, such as Chromium, Edge, Gecko, Servo and WebKit. + + * Breakage in results collections systems for results dashboards, such as + [wpt.fyi](https://wpt.fyi). + + * Breakage in supplemental tooling used by working groups, such as the + [CSS build system][]. + +When such breakage happens, if the maintainers of the affected systems request +it, pull requests to revert the original change should normally be approved and +merged as soon as possible. (When the original change itself was fixing a +serious problem, it's a judgement call, but prefer the fastest path to a stable +state acceptable to everyone.) + +Once a revert has happened, the maintainers of the affected systems are +expected to work with the original patch author to resolve the problem so that +the change can be relanded. A reasonable timeframe to do so is within one week. + +[CSS build system]: https://github.com/web-platform-tests/wpt/tree/master/css/tools -- cgit v1.2.3