mitsuhiko / insta

A snapshot testing library for rust
https://insta.rs
Apache License 2.0
2.26k stars 102 forks source link

Insta 1.41.0 finds spurious duplicate snapshots from tests in package root #678

Open wuggen opened 1 month ago

wuggen commented 1 month ago

What happened?

When insta is used in tests located in the package root (specified via the [[test]] Cargo.toml key), the CLI will find each generated snapshot twice, even though only one file is generated per snapshot.

Reproduction steps

The following is a minimal workspace setup to reproduce the issue:

$ ls
Cargo.toml test.rs

Cargo.toml:

[package]
name = "insta-testing"
version = "0.1.0"
edition = "2021"

[dev-dependencies]
insta = "=1.41.0"

[[test]]
name = "test"
path = "test.rs"

test.rs:

#[test]
fn some_test() {
    insta::assert_snapshot!("name", "content");
}

cargo insta test yields the following output and resulting workspace state:

$ cargo insta test
    Finished `test` profile [unoptimized + debuginfo] target(s) in 1.40s
     Running test.rs (target/debug/deps/test-68ebf84566e4320f)

running 1 test
stored new snapshot /home/julia/code/personal/insta-testing/snapshots/test__name.snap.new
test some_test ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.04s

info: 2 snapshots to review
use `cargo insta review` to review snapshots
$ tree -I target
.
├── Cargo.lock
├── Cargo.toml
├── snapshots
│   └── test__name.snap.new
└── test.rs

2 directories, 4 files

Running cargo insta review at this point will present the same snapshot twice. cargo insta pending-snapshots will list the same snap.new file twice. Running cargo insta accept will complain (once) about the snap.new file having been removed by another process, presumably upon failing to find it again after accepting and removing it the first time.

Insta Version

1.41.0

rustc Version

1.82.0

What did you expect?

I would have expected the snapshot to be found by the CLI only once; only reported as one snapshot from insta test and insta pending-snapshots, only presented once by insta review, only removed once by insta accept, etc.

Prior versions of insta display the expected behavior; setting the insta dependency version to =1.40.0 and installing cargo-insta@1.40.0 yields the expected behavior.

As far as I've been able to tell, this behavior only occurs when the test is in the package root directory. For instance, it does not occur (i.e. the expected behavior occurs) if test.rs is in the default tests directory, or if it's in a different subdirectory specified via the [[test]] key. However, changing the snapshot directory via with_settings! (or manually via Settings::bind) does not affect the behavior; it seems to depend only on the location of the test itself.

max-sixty commented 4 weeks ago

Thanks for the issue. We're more "advanced" about looking up the snapshots for each target now, including the paths that each target specifies — previously we would miss something outside the root (e.g. https://github.com/mitsuhiko/insta/issues/625)

Unfortunately that doesn't work so well here, because the file could belong to the root or to the tests target, and so insta is counting it as both.

I think probably the solution is to dedup when we're presenting the number of snapshots etc, though I'll think about whether there's some way of defining the file as belonging to only the tests target.

If you need a workaround in the short-term, passing --tests should work (lmk if it doesn't, would mean my understanding is wrong)

wuggen commented 4 weeks ago

Hm, doesn't seem to work. cargo insta test --tests yields the same behavior with the same setup.

max-sixty commented 4 weeks ago

OK thanks a lot @wuggen , I can reproduce, and my --tests suggestion wasn't correct. I will look into it shortly.