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

Ignore warning always seems to print #319

Closed
max-sixty opened this issue Dec 27, 2022 · 6 comments
Closed

Ignore warning always seems to print #319

max-sixty opened this issue Dec 27, 2022 · 6 comments
Labels
bug Something isn't working

Comments

@max-sixty
Copy link
Sponsor Contributor

What happened?

I had a quick go at fixing this but didn't make progress so here's a placeholder issue:

Currently it seems we always get this warning, even when no tests are ignored:

warning: some paths are ignored, use --no-ignore if you have snapshots in ignored paths.

This happens on a simple cargo insta test on both insta and prql

Reproduction steps

cargo insta test

Insta Version

1.23

rustc Version

1.66

What did you expect?

Only warn if we're actually ignoring something

@max-sixty max-sixty added the bug Something isn't working label Dec 27, 2022
@mitsuhiko
Copy link
Owner

That's a limitation I'm not sure I can easily remove. Unfortunately the underlying ignore crate does not report if it actually ends up using the ignored paths.

@max-sixty
Copy link
Sponsor Contributor Author

OK yes, makes sense.

FWIW — without much context beyond my own — I would quite prefer to have no warning.

Currently in PRQL, running the standard test suite raises warnings suggesting it should be run with --no-ignore, when actually that would have no effect. And I'm not sure how often people are using snapshots in ignored paths (some! #314).

Something to print a warning when writing a snapshot into an ignored path would solve this nicely, but be some work. And #282 would let us silence the warning.

Thanks again. People really like insta in PRQL, it's invaluable.

@mitsuhiko
Copy link
Owner

FWIW I'm with you and I also hate the warning and would like to get rid of it. so I'm not opposed at all to find ways to remove it. One potential option would be to just have the thing walk a second time afterwards without ignores and see if it picks up extra tests.

@max-sixty
Copy link
Sponsor Contributor Author

max-sixty commented Dec 27, 2022

Without ignoring, it is much slower — at least on prql, 3.69s vs. 4.15s with --no-ignore for cargo insta test, including the fixed costs of running the tests.

Do you think it would be sufficient to raise an error if the root snapshot path were ignored? I could possibly add that here: https://github.com/mitsuhiko/insta/blob/master/cargo-insta/src/cargo.rs#L255

It doesn't cover the case where a sub-dir is ignored, or someone has ignored .snap, but could be quite rare. (@lu-zero if you have thoughts...)

@lu-zero
Copy link

lu-zero commented Dec 28, 2022

I fixed the problem on my side, so I'm less impacted.
The simplest thing is probably add a configuration knob to silence the message.

@max-sixty
Copy link
Sponsor Contributor Author

I'll close this, since it's much less prevalent after #320, but please feel free to reopen if you want to track broader solutions

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

3 participants