mitsuhiko / insta

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

Using `--runner nextest` with `--test` results in an error about `--doc` #317

Closed ilyagr closed 1 year ago

ilyagr commented 1 year ago

What happened?

Running cargo insta test --test-runner nextest --test test_resolve_command works, but leaves a error:

$ cargo insta test --test-runner nextest --test test_resolve_command
    Finished test [unoptimized + debuginfo] target(s) in 0.11s
    Starting 7 tests across 1 binaries
        PASS [   0.324s] jujutsu::test_resolve_command test_file_vs_dir
        PASS [   0.398s] jujutsu::test_resolve_command test_edit_delete_conflict_input_files
        PASS [   0.431s] jujutsu::test_resolve_command test_too_many_parents
        PASS [   0.451s] jujutsu::test_resolve_command test_baseless_conflict_input_files
        PASS [   0.459s] jujutsu::test_resolve_command test_normal_conflict_input_files
        PASS [   0.676s] jujutsu::test_resolve_command test_multiple_conflicts
        PASS [   0.778s] jujutsu::test_resolve_command test_resolution
------------
     Summary [   0.780s] 7 tests run: 7 passed, 0 skipped
error: Can't mix --doc with other target selecting options

The error is on the last line.

Update: It does cause harm: if there are any snapshots that changed, the command does not tell you about it.

Reproduction steps

  1. Clone https://github.com/martinvonz/jj/
  2. cargo install cargo-insta cargo-nextest (I have cargo-nextest 0.9.47)
  3. Update: Change some of the snapshots in test_resolve_command.rs.
  4. Run cargo insta test --test-runner nextest --test test_resolve_command
  5. See the above output. Note that you are not told to run cargo insta review. (It does seem to work if executed manually)

Insta Version

cargo-insta 1.23.0

rustc Version

rustc 1.67.0-beta.2 (352eb59a4 2022-12-13)

What did you expect?

No error at the end of the output, and a notification to review the changed snapshots.

ilyagr commented 1 year ago

I updated the above; it seems this does cause harm if there are any snapshots to review.

mitsuhiko commented 1 year ago

I currently do not have the resources to fix this, but I believe it would be a rather straightforward change. If you print out the command that's being invoked in cargo-insta you can probably figure out what would need to be changed for nextest.

ilyagr commented 1 year ago

I might look into it, thanks for the pointer! I'd need to learn a bit more about the interface to cargo test and cargo-nextest. If I don't get to it, there's no rush.

mitsuhiko commented 1 year ago

The entire logic about how the command line is created is found in this one function: https://github.com/mitsuhiko/insta/blob/8006453e932826a0fc3396d239ba2c3c3cde175a/cargo-insta/src/cli.rs#L739

mitsuhiko commented 1 year ago

Fixed in 1ae7bd2bb472d7ba8124a4f200b0f1609348dddf

ilyagr commented 1 year ago

Thank you very much! Sorry I didn't get to it.