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

Failures within glob! & allow_duplicates! should show output #424

Open
max-sixty opened this issue Nov 13, 2023 · 3 comments
Open

Failures within glob! & allow_duplicates! should show output #424

max-sixty opened this issue Nov 13, 2023 · 3 comments

Comments

@max-sixty
Copy link
Sponsor Contributor

max-sixty commented Nov 13, 2023

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 #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)

@mitsuhiko
Copy link
Owner

mitsuhiko commented Nov 16, 2023

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.

@max-sixty
Copy link
Sponsor Contributor Author

max-sixty commented Nov 18, 2023

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: #385, mitsuhiko/insta-website#17, mitsuhiko/insta-website#18 — but also very much understand that the first one is quite a big change.

@max-sixty
Copy link
Sponsor Contributor Author

max-sixty commented Nov 30, 2023

FYI I updated this to specific to allow_duplicates!, given the discussion in PRQL/prql#3865

@max-sixty max-sixty changed the title Failures in glob! could show output Failures within glob! & allow_duplicates! should show output Nov 30, 2023
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