Add expect -- a light-weight alternative to insta#5101

matklad · 2020-06-27T16:46:18Z

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!):

self-updating tests, no need for a separate tool
fewer features (only inline snapshots, no redactions)
fewer deps (no yaml, no persistence)
tighter integration with editor
first-class snapshot object, which can be used to write test functions (as opposed to testing macros)
trivial to tweak for rust-analyzer needs, by virtue of being a workspace member.

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

matklad · 2020-06-27T16:47:39Z

See

for general explanation about snapshot testing

crates/expect/src/lib.rs

bjorn3 · 2020-06-27T17:59:19Z

crates/expect/src/lib.rs

+                break;
+            }
+            line_start += line.len();
+        }


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.

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

bjorn3 · 2020-06-27T18:01:59Z

crates/expect/src/lib.rs

+        for pos in &mut [&mut range.start, &mut range.end] {
+            **pos += insert;
+            **pos -= delete
+        }


Did you mean

Suggested change

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;

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

It's also wrong, needs to be in the opposite direction =/

bjorn3 · 2020-06-27T18:04:13Z

crates/expect/src/lib.rs

+    fn test_patchwork() {
+        let mut patchwork = Patchwork::new("one two three".to_string());
+        patchwork.patch(4..7, "zwei");
+        patchwork.patch(0..3, "один");


I assume these are one and two in Russian, right?

Russian and German

Co-authored-by: bjorn3 <bjorn3@users.noreply.github.com>

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.

matklad · 2020-06-27T20:36:22Z

r? @flodiebold as you had some concerns about this approach :-)

matklad · 2020-06-27T21:08:08Z

Here's how the test failure looks in terminal:

crates/rust-analyzer/src/handlers.rs

Co-authored-by: Veetaha <veetaha2@gmail.com>

matklad · 2020-06-27T23:54:46Z

crates/expect/src/lib.rs

+
+fn workspace_root() -> PathBuf {
+    Path::new(
+        &env::var("CARGO_MANIFEST_DIR").unwrap_or_else(|_| env!("CARGO_MANIFEST_DIR").to_owned()),


I don't remember :-) This is copy-pasted from project_dir in xtask, should probably contain more info in git blame.

Yup, this explains it

matklad · 2020-06-30T08:33:51Z

bors r+

bors · 2020-06-30T08:45:10Z

Build succeeded:

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: ![expect](https://user-images.githubusercontent.com/1711539/90888810-3bcc8180-e3b7-11ea-9626-d06e89e1a0bb.gif) 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: ![expect](https://user-images.githubusercontent.com/1711539/90888810-3bcc8180-e3b7-11ea-9626-d06e89e1a0bb.gif) 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!

matklad force-pushed the who-needs-flying-cars-anyway branch 2 times, most recently from 95d0322 to 6bc9808 Compare June 27, 2020 16:58

matklad mentioned this pull request Jun 27, 2020

Hand-rolling mini insta? #3835

Closed

matklad force-pushed the who-needs-flying-cars-anyway branch 4 times, most recently from 4cf08bd to e757870 Compare June 27, 2020 17:22

matklad added 2 commits June 27, 2020 19:22

Add light-weight snapshot testing library with editor integration

03c5a66

Add example expect test for goto definition

be265ec

matklad force-pushed the who-needs-flying-cars-anyway branch from e757870 to be265ec Compare June 27, 2020 17:22

bjorn3 reviewed Jun 27, 2020

View reviewed changes

crates/expect/src/lib.rs Outdated Show resolved Hide resolved

bjorn3 suggested changes Jun 27, 2020

View reviewed changes

matklad and others added 3 commits June 27, 2020 20:50

Update crates/expect/src/lib.rs

d21dae7

Co-authored-by: bjorn3 <bjorn3@users.noreply.github.com>

Update crates/expect/src/lib.rs

18e4e9f

Co-authored-by: bjorn3 <bjorn3@users.noreply.github.com>

Remove fragile test

175e48e

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.

matklad force-pushed the who-needs-flying-cars-anyway branch from 314187d to 175e48e Compare June 27, 2020 18:50

Add colors

a9b4fb0

bjorn3 approved these changes Jun 27, 2020

View reviewed changes

matklad added 2 commits June 27, 2020 21:33

style

53787c7

Fix potential overflow

3c1714d

Veetaha reviewed Jun 27, 2020

View reviewed changes

crates/rust-analyzer/src/handlers.rs Outdated Show resolved Hide resolved

Update crates/rust-analyzer/src/handlers.rs

a4f934e

Co-authored-by: Veetaha <veetaha2@gmail.com>

Veetaha reviewed Jun 27, 2020

View reviewed changes

flodiebold approved these changes Jun 28, 2020

View reviewed changes

bors bot merged commit d13ded6 into rust-lang:master Jun 30, 2020

matklad deleted the who-needs-flying-cars-anyway branch July 1, 2020 08:27

matklad mentioned this pull request Jul 3, 2020

SSR internal refactorings #5197

Merged

matklad referenced this pull request in CAD97/sorbus Jul 4, 2020

More smoke test serialization

6f10cf7

matklad mentioned this pull request Aug 21, 2020

Introduce expect snapshot testing library into rustc rust-lang/rust#75773

Merged

Wilfred mentioned this pull request Mar 13, 2024

Feature Request: Code Lens to run test with UPDATE_EXPECT=1 #10224

Open

-        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;

Conversation

matklad commented Jun 27, 2020

Editor Integration Implementation

Uh oh!

matklad commented Jun 27, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matklad commented Jun 27, 2020

Uh oh!

matklad commented Jun 27, 2020

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matklad commented Jun 30, 2020

Uh oh!

bors bot commented Jun 30, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants