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

Allow Configuring Fast Fail Behavior for Non-Glob Executions #512

Closed
0xcaff opened this issue Jun 25, 2024 · 4 comments
Closed

Allow Configuring Fast Fail Behavior for Non-Glob Executions #512

0xcaff opened this issue Jun 25, 2024 · 4 comments

Comments

@0xcaff
Copy link

0xcaff commented Jun 25, 2024

I've been a longtime user of insta but I previously found it to fall short around parameterized testing, namely generating snapshot tests for a bunch of test data. Recently, I discovered the glob feature and when looking into how it worked, I found Setting.set_snapshot_suffix

I tried using Setting.set_snapshot_suffix to setup my own parametrized testing like this:

#[cfg(test)]
mod tests {
    use super::*;

    const TEST_CASES: [&'static str; 8] = [
        "playnext=caller",
        "type=Dream dev_tag=AWARD_MmPick",
        "type=Scene",
        "ids=c827264df38e330,c919063738a6a0e AND type=Element OR type=Scene",
        "visibility=Remixable",
        "liked=\"\"",
        "dev_tag=CATEGORY_Movie type=Dream",
        "Gg gg AND type=Dream",
    ];

    #[test]
    fn test_tokenize() {
        for (idx, input) in TEST_CASES.iter().enumerate() {
            let mut settings = insta::Settings::clone_current();

            settings.set_snapshot_suffix(format!("{}", idx));

            settings.bind(|| {
                let tokens = tokenize(input);
                insta::assert_debug_snapshot!(tokens);
            });
        }
    }
}

but it seems like currently insta exits on the first test failure. It looks like glob gets around this by disabling fast failure here

insta/insta/src/runtime.rs

Lines 501 to 514 in acb1ce5

let fail_fast = {
#[cfg(feature = "glob")]
{
if let Some(top) = crate::glob::GLOB_STACK.lock().unwrap().last() {
top.fail_fast
} else {
true
}
}
#[cfg(not(feature = "glob"))]
{
true
}
};

For me there are use cases where I'd like parameterized snapshot tests but the input data is small and I'd like to define it inline with my test. I think there are others who'd find this useful too.

Would you be open to a PR adding an option to settings which allows for specifying fast fail for a specific test. I'm thinking if the setting for the current test is set to fast fail, and the snapshot match fails, fail immediately otherwise continue.

@0xcaff 0xcaff changed the title Allow Configuring Fast Fail Behavior for Non Glob Executions Allow Configuring Fast Fail Behavior for Non-Glob Executions Jun 25, 2024
@0xcaff
Copy link
Author

0xcaff commented Jun 25, 2024

It seems like when I use

cargo insta test --review

The fast fail behavior is disabled but when I run cargo test, it is still enabled. I will be using this as a workaround for now.

@max-sixty
Copy link
Collaborator

Not the maintainer but IIUC force_pass determines whether the test panics immediately or not:

insta/insta/src/runtime.rs

Lines 527 to 536 in acb1ce5

if update_result != SnapshotUpdateBehavior::InPlace && !ctx.tool_config.force_pass() {
if fail_fast && ctx.tool_config.output_behavior() != OutputBehavior::Nothing {
println!(
"{hint}",
hint = style(
"Stopped on the first failure. Run `cargo insta test` to run all snapshots."
)
.dim(),
);
}

(the names do seem confusing and possible we could consolidate these settings here — do we actually need two different variables there?)

IIRC cargo insta test by default will continue on failures, while cargo test requires an env var for that behavior. Is that what you see too?

@0xcaff
Copy link
Author

0xcaff commented Jun 25, 2024

Woah thanks for the super fast reply. Interesting, seems like there's more dials to turn that I wasn't aware of. I didn't know insta had all these great features. I've been using the default RustRover test runner (triggered by the gutter buttons).

IIRC cargo insta test by default will continue on failures, while cargo test requires an env var for that behavior. Is that what you see too?

Just tried it out, seems like this is indeed the case!

@max-sixty
Copy link
Collaborator

Cool!

Possible these can be consolidated — I think force_pass is used by cargo-insta to indicate to the insta tests that cargo-insta will handle the failures at the end, and so insta shouldn't panic on each failing test. I don't think it needs to be available as a user config in cargo-insta.

Whereas --no-fail-fast mirrors the cargo test config.

@mitsuhiko let us know if you see this and happen to remember... as part of the effort to simplify things a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants