Skip to content

Commit 2ad7ca1

Browse files
authored
chore(bugbot): Add testing conventions code review rules (#18433)
This PR adds some rules to bugbot's rulest to flag some testing-related issues we'd like to avoid. Like with all AI rules, there are for sure exceptions to the new rules, so no problem with us ignoring any of these flags. But I think having an additional reminder that testing is necessary would be a good change. LMK what you think!
1 parent 741ad6a commit 2ad7ca1

File tree

1 file changed

+13
-0
lines changed

1 file changed

+13
-0
lines changed

.cursor/BUGBOT.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,16 @@ Do not flag the issues below if they appear in tests.
4545
convention as the `SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN` value.
4646
- When calling `startSpan`, check if error cases are handled. If flag that it might make sense to try/catch and call `captureException`.
4747
- When calling `generateInstrumentationOnce`, the passed in name MUST match the name of the integration that uses it. If there are more than one instrumentations, they need to follow the pattern `${INSTRUMENTATION_NAME}.some-suffix`.
48+
49+
## Testing Conventions
50+
51+
- When reviewing a `feat` PR, check if the PR includes at least one integration or E2E test.
52+
If neither of the two are present, add a comment, recommending to add one.
53+
- When reviewing a `fix` PR, check if the PR includes at least one unit, integration or e2e test that tests the regression this PR fixes.
54+
Usually this means the test failed prior to the fix and passes with the fix.
55+
If no tests are present, add a comment recommending to add one.
56+
- Check that tests actually test the newly added behaviour.
57+
For instance, when checking on sent payloads by the SDK, ensure that the newly added data is asserted thoroughly.
58+
- Flag usage of `expect.objectContaining` and other relaxed assertions, when a test expects something NOT to be included in a payload but there's no respective assertion.
59+
- Flag usage of conditionals in one test and recommend splitting up the test for the different paths.
60+
- Flag usage of loops testing multiple scenarios in one test and recommend using `(it)|(test).each` instead.

0 commit comments

Comments
 (0)