feature: return structured errors instead of panicking and printing

[ajwerner]

Sometimes complex scenarios make insta hard to use because it panics and prints problems to stdout rather than returning structured errors. In general, the current behavior is a nice default, but it's not flexible in situations of parallel tests where you want to add more context, or in cases where some degree of flexibility is okay. Consider a case of a rare flakey test -- it's not a great place to be, but maybe it's better to detect and retry than to fail because deflaking is too expensive right now. Disabling the test is a bad answer..

It's a big ask, but it'd be a nice refactoring if there were a form of equivalent macros that returned some sort of structured result and then the existing macros could be implemented on top of it.

Maybe one day if I find the time I can help to implement this.

I suspect such a thing would also make the library itself easier to test.

[mitsuhiko]

Can you mock up an example of how you would use this in a test?

[ajwerner]

I need to update my notification settings. Sorry I missed this.

I'd want a macro that evaluates to Result<(), E> for some error type E. Let's call this new form of the macro check_snapshot. Then in places where today I invoke assert_snapshot!(...) I'd be able to do check_snapshot!(...)?.

The primary place this comes up for me right now is that I run a bunch of sub tests in parallel. When one fails today it panics. I eventually added some shenanigans to catch those panics, but I lose all structure and I lose the ability to add context when some of the tests produce multiple snapshots.

Additionally, there was one case where I wanted to be able to do something in response to the failure and that's hard to achieve with the panicking behavior.

[ajwerner]

I'm on my phone catching up on things in the airport but I'll make it concrete when I get back to the computer.

[mitsuhiko]

@ajwerner I hope you managed to get back to your computer ;)

So i went back to this issue and I'm still not sure I can full understand what you would want to do here. I think re-reading your comment the core issue is that insta is not great at supporting concurrency well? Maybe that is something that needs addressing instead?

[ajwerner]

Thanks for the ping :)

Concurrency support is part of what motivated this issue, and probably it's the biggest part.

I think I'd break this issue into two parts:

1. Have some way to get errors instead of panicking
   • Motivated primarily by concurrent testing
   • It'd be nice to be able to annotate the output with more context and to avoid it from being interleaved with other concurrent tests.
2. Make the errors structured in a way where the user can introspect what actually mismatched

Part 1 can be partially achieved today with catch_unwind:

std::panic::catch_unwind(|| {
    // ...
    insta::assert_json_snapshot!(/* ... */)
 }).map_err(|err| /* ... */)?;

It's not clear what should be printed if the snapshot fails, if anything. And, if nothing should be printed at the point where the failure is generated, then the returned error would need some way to produce its output later, probably via some method.

Today this prints the output inline as the failure occurs, but at least it provides some mechanism to do something sane in the face of concurrency.

Ideally I could do something like this instead:

insta::check_json_snapshot!(/* ... */)?;

The question for part 2 is what is the type of error that comes out of that macro, and what can users do with it. A fancy interface might allow the user to inspect which parts of the tree mismatched and to construct the diff on their own. That may well be far out of scope. What I've ended up doing for the one case where I needed to check if something specific was going on is to just parse the snapshot data and check explicitly. This worked out fine, but it was on my mind when I wrote the issue.

[mitsuhiko]

It's not clear what should be printed if the snapshot fails, if anything. And, if nothing should be printed at the point where the failure is generated, then the returned error would need some way to produce its output later, probably via some method.

I think the biggest challenge is workflow. Is that check supposed to generate new snapshots too?

Where does the concurrency come from in your case?

[ajwerner]

I think the biggest challenge is workflow. Is that check supposed to generate new snapshots too?

I think so. One interesting approach might be to have it happen on drop and to then have some mechanism to prevent that from happening, sort of like with tempfile.

Where does the concurrency come from in your case?

We want to run a bunch of tests in parallel and it's difficult to break them up in a way using something like testcase so that they are actually different top-level tests. Also there are multiple phases of each test that can run in parallel and produce different snapshots.

If there were something more like go's subtests in rust, then that'd probably work for me and this would all be fine. To some extent this all comes from using https://github.com/Michael-F-Bryan/include_dir with a directory full of inputs and wanting to run the test for each one and to have a set of corresponding snapshots for each input. I couldn't figure out how to automatically have a test generated for each one. Perhaps the answer is some sort of proc macro I write that depends on the inputs or some build script that depends on the inputs and generates the test cases. That all feels sort of worse/more magical than what I have today, which is to map over the inputs and then call futures::future::join_all.

[max-sixty]

To some extent this all comes from using https://github.com/Michael-F-Bryan/include_dir with a directory full of inputs and wanting to run the test for each one and to have a set of corresponding snapshots for each input. I couldn't figure out how to automatically have a test generated for each one.

Does glob! satisfy this?