mitsuhiko / insta

A snapshot testing library for rust
https://insta.rs
Apache License 2.0
2.26k stars 102 forks source link

How does `rounded_redaction` work? #592

Closed tnibler closed 2 months ago

tnibler commented 2 months ago

What happened?

Hi, I can not get rounded_redaction to do what I think it should do.

Reproduction steps

Here's a test:

#[test]
fn test() {
    let value: f32 = 0.12345678;
    insta::assert_yaml_snapshot!(
        value,
        {
            ".*" => insta::rounded_redaction(2)
        }
    );
}

Run it once, accept the snapshot. Then change the number to 0.12345688 and run it again.

Insta Version

1.40.0

rustc Version

1.80.0

What did you expect?

What I thought would happen: the test still passes, because we're rounding to 2 digits after the decimal.

What actually happens: the test fails.

Is this broken, or am I misunderstanding how this should work?

max-sixty commented 2 months ago

I don't know this code well, but to start — what number appears in the snapshot?

tnibler commented 2 months ago

This is the output:

Snapshot: test
Source: src/main.rs:8
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Expression: value
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
-old snapshot
+new results
────────────┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
    0       │-0.12345678
          0 │+0.12345688
────────────┴────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
To update snapshots run `cargo insta review`
Stopped on the first failure. Run `cargo insta test` to run all snapshots.
max-sixty commented 2 months ago

Thanks. Again responding quickly rather than completely:

looks like the value isn't being redacted at all, and possibly it needs to redact a field value rather than a raw variable. Here's the test in insta itself: https://github.com/mitsuhiko/insta/blob/ef7abb839e08932fdd32ce9a90e4593e138416c2/insta/tests/test_redaction.rs#L541

To test, you can try and more basic redaction and check that works.

(possibly this isn't designed well, not claiming this isn't an insta issue)

max-sixty commented 2 months ago

Coming back to this later in the day.

On reflection, I think the docs are not too confusing — redactions work on struct fields. So value in the example above needs to be a field of a struct.

To round a number as a string, passing a rounded float to assert_snapshot! is sufficient.

Does that make sense? I'll close but happy to take any feedback or PRs on the docs.

tnibler commented 2 months ago

Thanks for the quick replies :)

I just realized the underlying problem is a different one (which led me to try and reproduce this here and get more confused): redactions only affect what gets written, not the comparisons or deserialization! So the flow of:

will not work, because the full length values have already been written. I would have thought the redaction also gets applied when deserializing the snapshots back in, but it does not and in my example it would diff 0.12345678 against 0.12 and fail. I realize this makes little sense for the original purpose of masking UUIDs and whatnot, but for rounding it might actually be useful to apply it when reading as well. What do you think?

max-sixty commented 2 months ago

I'm not the source of truth here but one thought:

It seems quite rare that redactions on reads would be possible? It would reuqire the original value to be written without the redaction.

And often we don't want the value in the snapshot, so a passing snapshot would not pick up on the value erroneously being in the snapshot.

So it seems like a rare case that could additionally add some confusion...

tnibler commented 2 months ago

You're totally right, I think increasing rounding is the only case where changing the redaction retroactively even makes sense. Still, should I maybe add a sentence to the docs that clarifies this?

max-sixty commented 2 months ago

You're totally right, I think increasing rounding is the only case where changing the redaction retroactively even makes sense. Still, should I maybe add a sentence to the docs that clarifies this?

I do think a note in the docs would be useful stating that the only redacted values will be struct fields; possibly with your example as something that won't do any redaction, if you're up for it...