insta icon indicating copy to clipboard operation
insta copied to clipboard

Failures within `glob!` & `allow_duplicates!` should show output

Open max-sixty opened this issue 9 months ago • 3 comments

If a normal snapshot doesn't match, we get a very nice output:

$ cargo insta test -p prql-compiler --check

...

failures:

---- semantic::resolver::test::test_functions_pipeline stdout ----
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Snapshot Summary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Snapshot file: prqlc/prql-compiler/src/semantic/resolver/snapshots/prql_compiler__semantic__resolver__test__functions_pipeline.snap
Snapshot: functions_pipeline
Source: prqlc/prql-compiler/src/semantic/resolver/mod.rs:140
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Expression: resolve_derive(
    r#"
            from a
            derive one = (foo | sum)
            "#,
)
.unwrap()
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
-old snapshot
+new results
────────────┬─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   17    17 │     kind:
   18    18 │       Union:
   19    19 │         - - ~
   20    20 │           - kind:
   21       │-          i
   22    21 │               Primitive: Int
   23    22 │             span: "2:4300-4303"
   24    23 │             name: ~
   25    24 │         - - ~
────────────┴─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

But in a glob! expression (edit: within allow_duplicates!), we don't:

---- test_sql_examples_generic stdout ----
thread 'test_sql_examples_generic' panicked at 'glob! resulted in 1 snapshot assertion failure', /Users/maximilian/.cargo/registry/src/index.crates.io-6f17d22bba15001f/insta-1.34.0/src/glob.rs:128:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

This is unfortunate since often in a glob! & allow_duplicates! we especially want to have the context. For example here we use glob & allow_duplicates to run over multiple queries and multiple dialects, and use description to communicate this so we know which dialect was running when we hit a failure — information that's now lost.

In addition, running with --accept --unreference=auto seems to clear any other snapshots resulting from the glob!; I guess because the test is interrupted but cargo-insta doesn't realize that when cleaning up snapshots. (edit: fixed with https://github.com/mitsuhiko/insta/pull/440)

(I realize these examples aren't minimally reproducible — I wanted to get something down but can come back and amend if that's helpful)

max-sixty avatar Nov 13 '23 23:11 max-sixty

Would it be possible to submit a PR here with an improved behavior? I'm not sure I have the cycles at the moment to look at this. I also get output from the glob failures locally so not sure why/how they do not show up.

mitsuhiko avatar Nov 16 '23 21:11 mitsuhiko

Totally reasonable! I would love to spend some more time on insta, and I will try and get to this specific thing. (though not in the very short term, so if anyone sees this, please don't let me stop you).

Possibly broadening out to reworking the INSTA_UPDATE logic would also tackle the final item.

Tangentially related — there are a few PRs I'm waiting for reviews on: https://github.com/mitsuhiko/insta/pull/385, https://github.com/mitsuhiko/insta-website/pull/17, https://github.com/mitsuhiko/insta-website/pull/18 — but also very much understand that the first one is quite a big change.

max-sixty avatar Nov 18 '23 21:11 max-sixty

FYI I updated this to specific to allow_duplicates!, given the discussion in https://github.com/PRQL/prql/pull/3865

max-sixty avatar Nov 30 '23 21:11 max-sixty