-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add expect -- a light-weight alternative to insta #5101
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.
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
Conversation
See
for general explanation about snapshot testing |
95d0322
to
6bc9808
Compare
4cf08bd
to
e757870
Compare
e757870
to
be265ec
Compare
break; | ||
} | ||
line_start += line.len(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if you write expect![ [
? I think it should give an error in that case instead of just silently going to the end of the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that detecting this error condition doesn't worth additional code complexity. Another bug here is that we might hit false positive on ]]
, but I think that should be acceptable for internal tool. We can always wait until someone actually hits the case :)
crates/expect/src/lib.rs
Outdated
for pos in &mut [&mut range.start, &mut range.end] { | ||
**pos += insert; | ||
**pos -= delete | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean
for pos in &mut [&mut range.start, &mut range.end] { | |
**pos += insert; | |
**pos -= delete | |
} | |
range.start += insert; | |
range.start -= delete; | |
range.end += insert; | |
range.end -= delete; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This +/-
thing to avoid underflow is pretty intricate, so I'd rather write it once. Will simplify once rust-lang/rust#65819 is merged :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's also wrong, needs to be in the opposite direction =/
fn test_patchwork() { | ||
let mut patchwork = Patchwork::new("one two three".to_string()); | ||
patchwork.patch(4..7, "zwei"); | ||
patchwork.patch(0..3, "один"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume these are one and two in Russian, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Russian and German
Co-authored-by: bjorn3 <[email protected]>
Co-authored-by: bjorn3 <[email protected]>
This test needs to be updated after every change (it contains line number), which is annoying. It also fails on windows due to \, so it's easier to remove it.
314187d
to
175e48e
Compare
r? @flodiebold as you had some concerns about this approach :-) |
Co-authored-by: Veetaha <[email protected]>
|
||
fn workspace_root() -> PathBuf { | ||
Path::new( | ||
&env::var("CARGO_MANIFEST_DIR").unwrap_or_else(|_| env!("CARGO_MANIFEST_DIR").to_owned()), |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't remember :-) This is copy-pasted from project_dir
in xtask
, should probably contain more info in git blame.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, this explains it
bors r+ |
Snapshot testing is a technique for writing maintainable unit tests. Unlike usual `assert_eq!` tests, snapshot tests allow to *automatically* upgrade expected values on test failure. In a sense, snapshot tests are inline-version of our beloved UI-tests. Example:  A particular library we use, `expect_test` provides an `expect!` macro, which creates a sort of self-updating string literal (by using `file!` macro). Self-update is triggered by setting `UPDATE_EXPECT` environmental variable (this info is printed during the test failure). This library was extracted from rust-analyzer, where we use it for most of our tests. There are some other, more popular snapshot testing libraries: * https://github.com/mitsuhiko/insta * https://github.com/aaronabramov/k9 The main differences of `expect` are: * first-class snapshot objects (so, tests can be written as functions, rather than as macros) * focus on inline-snapshots (but file snapshots are also supported) * restricted feature set (only `assert_eq` and `assert_debug_eq`) * no extra runtime (ie, no `cargo insta`) See rust-lang/rust-analyzer#5101 for a an extended comparison. It is unclear if this testing style will stick with rustc in the long run. At the moment, rustc is mainly tested via integrated UI tests. But in the library-ified world, unit-tests will become somewhat more important (that's why use use `rustc_lexer` library-ified library as an example in this PR). Given that the cost of removal shouldn't be too high, it probably makes sense to just see if this flies!
…crum Introduce expect snapshot testing library into rustc Snapshot testing is a technique for writing maintainable unit tests. Unlike usual `assert_eq!` tests, snapshot tests allow to *automatically* upgrade expected values on test failure. In a sense, snapshot tests are inline-version of our beloved UI-tests. Example:  A particular library we use, `expect_test` provides an `expect!` macro, which creates a sort of self-updating string literal (by using `file!` macro). Self-update is triggered by setting `UPDATE_EXPECT` environmental variable (this info is printed during the test failure). This library was extracted from rust-analyzer, where we use it for most of our tests. There are some other, more popular snapshot testing libraries: * https://github.com/mitsuhiko/insta * https://github.com/aaronabramov/k9 The main differences of `expect` are: * first-class snapshot objects (so, tests can be written as functions, rather than as macros) * focus on inline-snapshots (but file snapshots are also supported) * restricted feature set (only `assert_eq` and `assert_debug_eq`) * no extra runtime (ie, no `cargo insta`) See rust-lang/rust-analyzer#5101 for a an extended comparison. It is unclear if this testing style will stick with rustc in the long run. At the moment, rustc is mainly tested via integrated UI tests. But in the library-ified world, unit-tests will become somewhat more important (that's why use use `rustc_lexer` library-ified library as an example in this PR). Given that the cost of removal shouldn't be too high, it probably makes sense to just see if this flies!
This PR implements a small snapshot-testing library. Snapshot updating is done by setting an env var, or by using editor feature (which runs a test with env-var set).
Here's workflow for updating a failing test:
Here's workflow for adding a new test:
Note that colorized diffs are not implemented in this PR, but should be easy to add (we already use them in test_utils).
Main differences from insta (which is essential for rust-analyzer development, thanks @mitsuhiko!):
I think eventually we should converge to a single snapshot testing library, but I am not sure that
expect
is exactly right, so I suggest rolling with both insta and expect for some time (if folks agree that expect might be better in the first place!).Editor Integration Implementation
The thing I am most excited about is the ability to update a specific snapshot from the editor. I want this to be available to other snapshot-testing libraries (cc @mitsuhiko, @aaronabramov), so I want to document how this works.
The ideal UI here would be a code action (:bulb:). Unfortunately, it seems like it is impossible to implement without some kind of persistence (if you save test failures into some kind of a database, like insta does, than you can read the database from the editor plugin). Note that it is possible to highlight error by outputing error message in rustc's format. Unfortunately, one can't use the same trick to implement a quick fix.
For this reason, expect makes use of another rust-analyzer feature -- ability to run a single test at the cursor position. This does need some expect-specific code in rust-analyzer unfortunately. Specifically, if rust-analyzer notices that the cursor is on
expect!
macro, it adds a special flag to runnable's JSON. However, given #5017 it is possible to approximate this well-enough without rust-analyzer integration. Specifically, an extension can register a special runner which checks (using regexes) if rust-anlyzer runnable covers text with specific macro invocation and do special magic in that case.closes #3835