Skip to content
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

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

Closed
ilyagr opened this issue Dec 24, 2022 · 6 comments
Closed

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

ilyagr opened this issue Dec 24, 2022 · 6 comments
Labels
bug Something isn't working

Comments

@ilyagr
Copy link

ilyagr commented Dec 24, 2022

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 ilyagr added the bug Something isn't working label Dec 24, 2022
@ilyagr
Copy link
Author

ilyagr commented Dec 24, 2022

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

@mitsuhiko
Copy link
Owner

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
Copy link
Author

ilyagr commented Dec 25, 2022

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
Copy link
Owner

The entire logic about how the command line is created is found in this one function:

) -> Result<(process::Command, Option<Cow<'snapshot_ref, Path>>), Box<dyn Error>> {

@mitsuhiko
Copy link
Owner

Fixed in 1ae7bd2

@ilyagr
Copy link
Author

ilyagr commented Jan 3, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants