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

Whitespace issues with inline snapshots #117

Closed
michalt opened this issue Apr 20, 2020 · 8 comments
Closed

Whitespace issues with inline snapshots #117

michalt opened this issue Apr 20, 2020 · 8 comments

Comments

@michalt
Copy link

michalt commented Apr 20, 2020

I'm just starting to use insta and it's really cool. But I've run into some problems with whitespace and inline snapshots.

Let's start with:

#[test]
fn test_insta() {
    let input = r###"

    something
    "###;

    assert_display_snapshot!(input, @r###""###);
}

This obviously fails. Running cargo insta review and accepting the new output gives:

#[test]
fn test_insta() {
    let input = r###"

    something
    "###;

    assert_display_snapshot!(input, @r###"


        something
        
    "###);
}

But the test still fails with:

-old snapshot
+new results
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
input
────────────┬───────────────────────────────────────────────────────────────────────────────────────────────────────────────
    1       │-something
          1 │+
          2 │+
          3 │+    something
          4 │+
────────────┴───────────────────────────────────────────────────────────────────────────────────────────────────────────────

Am I doing something wrong here?

@mitsuhiko
Copy link
Owner

This is a somewhat known limitation. Leading whitespace with inline snapshots will cause this. I'm not sure if there is a good solution for this with the current model for whitespace trimming we have.

@sbdchd
Copy link

sbdchd commented Jun 21, 2020

I'm running into what I think is a related issue.

The following

use insta::assert_display_snapshot;

#[test]
fn test_foo() {
    let body_txt = r#"
# title

### subtitle
"#.trim_matches('\n');
    println!("{:#?}", body_txt);
    assert_display_snapshot!(body_txt, @r###"
    # title

    ### subtitle
    "###);
}

prompts a snapshot update and cargo insta accept doesn't fix the difference.

Instead what seems to work is removing the indent for the generated inline snapshot.

diff --git a/src/main.rs b/src/main.rs
index afdacba..1fce772 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -14,8 +14,8 @@ fn test_foo() {
 "#.trim_matches('\n');
     println!("{:#?}", body_txt);
     assert_display_snapshot!(body_txt, @r###"
-    # title
+# title
 
-    ### subtitle
+### subtitle
     "###);
 }

So when updating the snapshot I cargo insta accept and then remove the indent manually

@mitsuhiko
Copy link
Owner

My recommendation would be to prefix the string with a non whitespace character.

@colelawrence
Copy link

Does anyone have an example of an inline display snapshot that spans multiple lines working? I can't seem to get any combination of trimming or indenting to make things consistent. Would there be a difference between cargo insta test --accept and cargo test?

I see whitespace errors when I cargo insta test --accept and then cargo test right after:

#[test]
fn test_completions_when_algo() {
    insta::assert_display_snapshot!(completions_for("when algo|"),
    @r###"
    # "Services"
    <"Algolia">
      [Algolia] 

    # "Services"
    <"Magic">
      [Magic] 
    <"Maps">
      [Maps] 
    <"Salesforce">
      [Salesforce] 
    <"PagerDuty">
      [PagerDuty] 
    <"Twilio">
      [Twilio] 
    <"Slack">
      [Slack] 
    "###);
}

image

Currently using insta 1.6.0 and cargo-insta 1.6.0

@mitsuhiko
Copy link
Owner

@colelawrence that's a bug that #166 will solve.

@max-sixty
Copy link
Collaborator

To what extent could we solve this by removing leading whitespace from both the result and the snapshot?

The downside of that is the test will still pass even if they have different leading whitespace — inline snapshots would quietly allow for some false passes, rather than the current situation of loudly always raising a false fail.

An alternative advanced approach would be to try and calculate the exact leading whitespace from the location of the result, rather than the current approach of removing all leading whitespace. But that would make results indentation sensitive, and possibly have a tail of complication.

@mitsuhiko
Copy link
Owner

I feel like touching this is tricky and will trade one issue for another. A pretty crappy but trivial solution would be to provide a helper to add markers before and after.

@max-sixty
Copy link
Collaborator

I would vote to close this:

  • Asserting a specific indentation is basically intractable in inline snapshots — we should use file snapshots for those
  • Newline issues can be rolled into Newline proposal #457

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

5 participants