search

mitsuhiko / insta

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

Regression running `cargo-insta` in workspace with crate in root #593

Closed grovesNL closed 4 weeks ago

grovesNL commented 1 month ago

What happened?

Hi! I've been running cargo-insta in a workspace for the past few years running the test command in the root of my workspace. The workspace has a crate in the root.

I recently updated cargo-insta (I'm not sure which version I was on before unfortunately - I reinstalled Rust) and now I can't seem to update inline snapshots outside of the root crate.

It seems to work fine if there isn't a crate at the root. I can move my project around to workaround this, but I know there are other projects set up this way too.

Reproduction steps

  1. git clone https://github.com/grovesNL/insta-bug-2024-09
  2. cargo test --all or cargo insta test --all
  3. The root snapshot can be reviewed and updated. The snapshot under insta-example will report a failure and create a pending snap file, but can't be reviewed.

Insta Version

1.40

rustc Version

rustc 1.81.0 (eeb90cda1 2024-09-04)

What did you expect?

All snapshots should be reviewable/accepted

max-sixty commented 1 month ago

Thanks for the issue and sorry for the trouble. Will look at this.

grovesNL commented 1 month ago

No problem at all! I can work around it easily enough - just wanted to mention it here in case anyone ran into the same thing.

max-sixty commented 1 month ago

~OK, I can repro, sorry about this. We fixed some bugs around workspaces in the most recent version and this much have fallen through.~

~For today, running cargo insta test --all --accept does seem to work (and then using git to review), so should unblock folks (or downgrading).~

cargo insta review --workspace works, see below

max-sixty commented 1 month ago

OK, it seems that since fixing the panic (https://github.com/mitsuhiko/insta/issues/396), we're "more consistent" about how we handle crates in a workspace, and so we now respect -p & --workspace cli arguments including for cargo insta review, which now requires cargo insta review --workspace to review snapshots of non-default crates

Possibly that's confusing and we should have cargo insta review run across all crates in a workspace, including those which aren't default crates? This makes more sense to me, though possibly that's because it's what I'm used to. (I also don't work with workspaces with non-default crates personally, so am less affected by this).

What do folks think? @mitsuhiko ?

grovesNL commented 1 month ago

Ah ok, that makes sense. I'm just used to running cargo insta review all the time, but either option sounds ok.

max-sixty commented 1 month ago

I will leave this open for feedback from @mitsuhiko or others

max-sixty commented 4 weeks ago

No feedback so will close, but if anyone feels strongly, please comment / reopen / open another issue