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