|
| 1 | +# Testing |
| 2 | + |
| 3 | +Developing lints for Clippy is a Test-Driven Development (TDD) process because |
| 4 | +our first task before implementing any logic for a new lint is to write some test cases. |
| 5 | + |
| 6 | +## Develop Lint with Tests |
| 7 | + |
| 8 | +It is not necessarily that every Clippy developer enjoys having the compiler screaming |
| 9 | +test failures in their face. Some embrace TDD *while* others learn to peacefully co-exist with it. |
| 10 | + |
| 11 | +When we develop Clippy, we enter a complex and chaotic realm full of |
| 12 | +programmatic issues, stylistic errors, illogical code and non-adherence to convention. |
| 13 | +Tests are the first layer of order we can leverage to define when and where |
| 14 | +we want a new lint to trigger or not. |
| 15 | + |
| 16 | +Moreover, writing tests first help Clippy developers to find a balance for |
| 17 | +the first iteration of and further enhancements for a lint. |
| 18 | +With test cases on our side, we will not have to worry about over-engineering |
| 19 | +a lint on its first version nor missing out some obvious edge cases of the lint. |
| 20 | +This approach empowers us to iteratively enhance each lint. |
| 21 | + |
| 22 | +## Clippy UI Tests |
| 23 | + |
| 24 | +We use **UI tests** for testing in Clippy. |
| 25 | +These UI tests check that the output of Clippy is exactly as we expect it to be. |
| 26 | +Each test is just a plain Rust file that contains the code we want to check. |
| 27 | + |
| 28 | +The output of Clippy is compared against a `.stderr` file. |
| 29 | +Note that you don't have to create this file yourself. |
| 30 | +We'll get to generating the `.stderr` files with the command [`cargo dev bless`](#cargo-dev-bless) (seen later on). |
| 31 | + |
| 32 | +### Write Test Cases |
| 33 | + |
| 34 | +Let us now think about some tests for our imaginary `foo_functions` lint, |
| 35 | +We start by opening the test file `tests/ui/foo_functions.rs` that was created by `cargo dev new_lint`. |
| 36 | + |
| 37 | +Update the file with some examples to get started: |
| 38 | + |
| 39 | +```rust |
| 40 | +#![warn(clippy::foo_functions)] |
| 41 | +// Impl methods |
| 42 | +struct A; |
| 43 | +impl A { |
| 44 | + pub fn fo(&self) {} |
| 45 | + pub fn foo(&self) {} |
| 46 | + pub fn food(&self) {} |
| 47 | +} |
| 48 | + |
| 49 | +// Default trait methods |
| 50 | +trait B { |
| 51 | + fn fo(&self) {} |
| 52 | + fn foo(&self) {} |
| 53 | + fn food(&self) {} |
| 54 | +} |
| 55 | + |
| 56 | +// Plain functions |
| 57 | +fn fo() {} |
| 58 | +fn foo() {} |
| 59 | +fn food() {} |
| 60 | + |
| 61 | +fn main() { |
| 62 | + // We also don't want to lint method calls |
| 63 | + foo(); |
| 64 | + let a = A; |
| 65 | + a.foo(); |
| 66 | +} |
| 67 | +``` |
| 68 | + |
| 69 | +Without actual lint logic to emit the lint when we see a `foo` function name, |
| 70 | +these tests are still quite meaningless. |
| 71 | +However, we can now run the test with the following command: |
| 72 | + |
| 73 | +```sh |
| 74 | +$ TESTNAME=foo_functions cargo uitest |
| 75 | +``` |
| 76 | + |
| 77 | +Clippy will compile and it will conclude with an `ok` for the tests: |
| 78 | + |
| 79 | +``` |
| 80 | +...Clippy warnings and test outputs... |
| 81 | +test compile_test ... ok |
| 82 | +test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.48s |
| 83 | +``` |
| 84 | + |
| 85 | +This is normal. After all, we wrote a bunch of Rust code but we haven't really |
| 86 | +implemented any logic for Clippy to detect `foo` functions and emit a lint. |
| 87 | + |
| 88 | +As we gradually implement our lint logic, we will keep running this UI test command. |
| 89 | +Clippy will begin outputting information that allows us to check if the output is |
| 90 | +turning into what we want it to be. |
| 91 | + |
| 92 | +> _Note:_ You can run multiple test files by specifying a comma separated list: |
| 93 | +> `TESTNAME=foo_functions,bar_methods,baz_structs`. |
| 94 | +### `cargo dev bless` |
| 95 | + |
| 96 | +Once we are satisfied with the output, we need to run this command to |
| 97 | +generate or update the `.stderr` file for our lint: |
| 98 | + |
| 99 | +```sh |
| 100 | +$ TESTNAME=foo_functions cargo uitest |
| 101 | +# (Output is as we want it to be) |
| 102 | +$ cargo dev bless |
| 103 | +``` |
| 104 | + |
| 105 | +This write the emitted lint suggestions and fixes to the `.stderr` file, |
| 106 | +with the reason for the lint, suggested fixes, and line numbers, etc. |
| 107 | + |
| 108 | +> _Note:_ we need to run `TESTNAME=foo_functions cargo uitest` every time before we run |
| 109 | +> `cargo dev bless`. |
| 110 | +Running `TESTNAME=foo_functions cargo uitest` should pass then. When we |
| 111 | +commit our lint, we need to commit the generated `.stderr` files, too. |
| 112 | + |
| 113 | +In general, you should only commit files changed by `cargo dev bless` for the |
| 114 | +specific lint you are creating/editing. |
| 115 | + |
| 116 | +> _Note:_ If the generated `.stderr`, and `.fixed` files are empty, |
| 117 | +> they should be removed. |
| 118 | +## Cargo Lints |
| 119 | + |
| 120 | +The process of testing is different for Cargo lints in that now we are |
| 121 | +interested in the `Cargo.toml` manifest file. |
| 122 | +In this case, we also need a minimal crate associated with that manifest. |
| 123 | + |
| 124 | +Imagine we have a new example lint that is named `foo_categories`, we can run: |
| 125 | + |
| 126 | +```sh |
| 127 | +$ cargo dev new_lint --name=foo_categories --pass=late --category=cargo |
| 128 | +``` |
| 129 | + |
| 130 | +After running `cargo dev new_lint` we will find by default two new crates, |
| 131 | +each with its manifest file: |
| 132 | + |
| 133 | +* `tests/ui-cargo/foo_categories/fail/Cargo.toml`: this file should cause the |
| 134 | + new lint to raise an error. |
| 135 | +* `tests/ui-cargo/foo_categories/pass/Cargo.toml`: this file should not trigger |
| 136 | + the lint. |
| 137 | + |
| 138 | +If you need more cases, you can copy one of those crates (under `foo_categories`) and rename it. |
| 139 | + |
| 140 | +The process of generating the `.stderr` file is the same as for other lints |
| 141 | +and prepending the `TESTNAME` variable to `cargo uitest` works for Cargo lints too. |
| 142 | + |
| 143 | +Overall, you should see the following changes when you generate a new Cargo lint: |
| 144 | + |
| 145 | +```sh |
| 146 | +$ git status |
| 147 | +On branch foo_categories |
| 148 | +Changes not staged for commit: |
| 149 | + (use "git add <file>..." to update what will be committed) |
| 150 | + (use "git restore <file>..." to discard changes in working directory) |
| 151 | + modified: CHANGELOG.md |
| 152 | + modified: clippy_lints/src/cargo/mod.rs |
| 153 | + modified: clippy_lints/src/lib.rs |
| 154 | +Untracked files: |
| 155 | + (use "git add <file>..." to include in what will be committed) |
| 156 | + clippy_lints/src/cargo/foo_categories.rs |
| 157 | + tests/ui-cargo/foo_categories/ |
| 158 | +``` |
| 159 | + |
| 160 | +## Rustfix Tests |
| 161 | + |
| 162 | +If the lint you are working on is making use of structured suggestions, the test |
| 163 | +file should include a `// run-rustfix` comment at the top. |
| 164 | + |
| 165 | +Structured suggestions tell a user how to fix or re-write certain code that has been linted, they are usually linted with [`span_lint_and_sugg`](https://doc.rust-lang.org/beta/nightly-rustc/clippy_utils/diagnostics/fn.span_lint_and_sugg.html). |
| 166 | + |
| 167 | +The `// run-rustfix` comment will additionally run [rustfix] for our test. |
| 168 | +Rustfix will apply the suggestions from the lint to the test file code and |
| 169 | +compare that to the contents of a `.fixed` file. |
| 170 | + |
| 171 | +Use `cargo dev bless` to automatically generate the `.fixed` file after running the tests. |
| 172 | + |
| 173 | +[rustfix]: https://github.com/rust-lang/rustfix |
| 174 | +## Testing Manually |
| 175 | + |
| 176 | +Manually testing against an example file can be useful if you have added some |
| 177 | +`println!`s and the test suite output becomes unreadable. |
| 178 | + |
| 179 | +To try Clippy with your local modifications, run from the working copy root. |
| 180 | + |
| 181 | +```sh |
| 182 | +$ cargo dev lint input.rs |
| 183 | +``` |
0 commit comments