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.

Sign up for GitHub

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

Jump to bottom

Revise test naming advice to discourage using issue numbers alone #2090

Merged
merged 1 commit into from
Oct 7, 2024
Merged

Revise test naming advice to discourage using issue numbers alone #2090

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions src/tests/best-practices.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ test that makes the life of another contributor as hard as possible.
>
> To make the life of another contributor as hard as possible, one might:
>
> - Only name the test after an issue, e.g. `issue-123456.rs`.
> - Name the test after an issue number alone without any other context, e.g.
> `issue-123456.rs`.
> - Have no comments at all on what the test is trying to exercise, no links to
> relevant context.
> - Include a test that is massive (that can otherwise be minimized) and
Expand All @@ -40,23 +41,28 @@ related tests.

- Name the test after what it's trying to exercise or prevent regressions of.
- Keep it concise.
- Avoid including issue numbers in test names.
- Avoid using issue numbers alone as test names.
- Avoid starting the test name with `issue-xxxxx` prefix as it degrades
auto-completion.

> **Avoid issue numbers in test names**
> **Avoid using only issue numbers as test names**
>
> Prefer including them as links or `#123456` in test comments instead.
> Prefer including them as links or `#123456` in test comments instead. Or if it
> makes sense to include the issue number, also include brief keywords like
> `macro-external-span-ice-123956.rs`.
>
> ```text
> tests/ui/typeck/issue-123456.rs // bad
> tests/ui/typeck/issue-123456-asm-macro-external-span-ice.rs // bad
> tests/ui/typeck/issue-123456-asm-macro-external-span-ice.rs // bad (for tab completion)
> tests/ui/typeck/asm-macro-external-span-ice-123456.rs // good
> tests/ui/typeck/asm-macro-external-span-ice.rs // good
> ```
>
> `issue-123456.rs` does not tell you immediately anything about what the test
> is actually exercising meaning you need to do additional searching. Including
> the issue number in the test name is really noisy for finding relevant tests
> by what they're exercising (if you `ls` a test directory and get a bunch of
> `issue-xxxxx` prefixes). We can link to the issue in a test comment.
> the issue number in the test name as a prefix makes tab completion less useful
> (if you `ls` a test directory and get a bunch of `issue-xxxxx` prefixes). We
> can link to the issue in a test comment.
>
> ```rs
> //! Check that `asm!` macro including nested macros that come from external
Expand Down