diff --git a/docs/contributing/Tree-hygiene.md b/docs/contributing/Tree-hygiene.md index 7ae7852bd6..c6704513ce 100644 --- a/docs/contributing/Tree-hygiene.md +++ b/docs/contributing/Tree-hygiene.md @@ -84,29 +84,58 @@ _See also: [What should I work on?](What-should-I-work-on.md)_ ## Tests -Every change in the flutter/engine, flutter/flutter, and flutter/packages repos must be tested, except for PRs that: +Every change in the flutter/engine, flutter/flutter, and flutter/packages repos +must be tested; consider using the code coverage tools to check that all your +new code is covered by tests (see [Test coverage for package:flutter](./testing/Test-coverage-for-package-flutter.md)). -* only remove code (no modified or added lines) to remove a feature or remove dead code. (Removing code to fix a bug still needs a test.) +There is an automatic exception for PRs that: + +* only remove code (no modified or added lines) to remove a feature or remove dead code. + * **removing code to fix a bug still needs a test**, the exemption does not apply * only affect comments (including documentation). -* only affect code inside the `.github` directory, `.cirrus.yml` or `.ci.yaml` config files. +* only affect code inside the `.github` directory or `.ci.yaml` config files. * only affect `.md` files. * are generated by automated bots (rollers). -* have an _explicit_ exemption (a message from a member of `@test-exemption-reviewer` on Discord starting with `test-exempt:`). - - -**In the repositories listed above, a bot will comment on your PR if you need an explicit exemption.** Do not land a PR that has had such a message from the bot unless it has an explicit exemption! The message tells you how to ask for an exemption. Please follow those instructions. - -**If the PR is in a repository that is not listed above, meaning is not supported by the bot, then it is the responsibility of the pull request reviewer to make sure that tests have been added to support the code change.** - -In particular, the following kinds of PRs are *not* automatically exempt and require an explicit comment even though the answer may be obvious: refactors with no semantic change (e.g. null safety migrations), configuration changes in the aforementioned repos, changes to analysis (fixing lints, turning on lints), changes to test infrastructure, manual dependency rolls, fixes to existing tests, cosmetic fixes to unpublished example apps. If a reviewer says a PR should have a test, then it needs a test regardless of the exemptions above. -PRs adding data-driven fixes require tests that fall under the test_fixes directory, but are not yet recognized by the bot as being tested. +Feel free to update the bot's logic to catch more cases that should be +automatically exempt ([in the cocoon repo](https://github.com/reidbaker/cocoon/blob/main/app_dart/lib/src/request_handlers/github/webhook_subscription.dart)). -Consider using the code coverage tools to check that all your new code is covered by tests (see [Test coverage for package:flutter](./testing/Test-coverage-for-package-flutter.md)). +> [!NOTE] +> PRs adding data-driven fixes require tests that fall under the test_fixes +> directory, but are _not_ yet recognized by the bot as being tested. -Feel free to update the bot's logic to catch more cases that should be automatically exempt (it's in the cocoon repo). +The following kinds of PRs are *not* automatically exempt and require an exemption even if the answer may be obvious: + +* refactors with no semantic change (e.g. null safety migrations) +* configuration changes in the aforementioned repos +* changes to analysis (fixing lints, turning on lints) +* changes to test infrastructure +* manual dependency rolls +* fixes to existing tests +* cosmetic fixes to unpublished example apps. + +**In the repositories listed above, a bot will comment on your PR if you need an explicit exemption.** + +> [!IMPORTANT] +> If the PR is in a repository that is not listed above, meaning is not +> supported by the bot, then it is _still_ a responsibility of the pull request +> reviewer to make sure that tests have been added to support the code change. + +### Test Exemptions + +A `@test-exemption-reviewer` member on [Discord](Chat.md) can exempt a PR by +commenting `test-exempt: `. + +The reviewer of the PR (_not_ the test exemption reviewer) should first: + +1. ensure that the PR meets the criteria for exemption +1. LGTM the PR before requesting the exemption, _or_ clarify why an LGTM was + not given. + +Exemption reviewers are a **small volunteer group** - _all_ reviewers should +feel empowered to ask for tests. ## Using git