feat: allow empty lines in snapshot

[George-Ogden]

Description

This PR allows empty lines in the snapshot file to be treated that way, without affecting other snapshot semantics.

Example

Say I have a test output that looks like this:

# Outputs README-like text

Including empty newlines above and below

<footer>Followed by a footer</footer>

This is saved in a snapshot as:

  """
  # Outputs README-like text
  
  Including empty newlines above and below
  
  <footer>Followed by a footer</footer>
  """

Therefore, there are two lines in this snapshot that include just spaces: " ".
This PR does not affect how the snapshots are saved, but if you trim the empty spaces on the blank lines, such as with a linting tool, the snapshot will load them as empty lines (currently, they are skipped).

Related Issues

• closes .ambr snapshot files have meaningful trailing whitespace #619, which asks for this feature

Checklist

• This PR has sufficient documentation.
• This PR has sufficient test coverage.
• This PR title satisfies semantic convention.

Additional Comments

No additional comments.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@           Coverage Diff           @@
##             main    #1042   +/-   ##
=======================================
  Coverage   97.29%   97.30%           
=======================================
  Files          21       21           
  Lines        1666     1669    +3     
=======================================
+ Hits         1621     1624    +3     
  Misses         45       45           

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[noahnu]

The previous elif checks if the line starts with the indent, which means L188 will only evaluate to true if we encounter a line with with less leading whitespace than the indentation size (2 chars).

It means these snapshots are equivalent:

# name: test_multiline_repr
⬚⬚Line1
⬚⬚Line2

⬚⬚Line3

# name: test_multiline_repr
⬚⬚Line1
⬚⬚Line2
⬚
⬚⬚Line3

# name: test_multiline_repr
⬚⬚Line1
⬚⬚Line2
⬚⬚
⬚⬚Line3

Having modified snapshots not break the test violates the soundness principle IMO and for that reason alone I'm going to reject this change.

That being said, if the motivation is due to tooling like editorconfig removing trailing whitespace, wouldn't it also remove trailing whitespace at the end of "non-empty" lines which this PR doesn't address?

If your IDE is stripping trailing whitespace, I recommend configuring an exception for amber files. Generally snapshot files shouldn't be modified by hand anyway.

[George-Ogden]

I see your point about soundness, but I think trailing whitespace is a different issue. While you're right that we could configure IDEs to handle snapshot files specially, it's a bit of a pain, especially for new users who run the test fine the first time, but then see errors afterwards. Trailing whitespace at the end of actual content lines usually matters, but on blank lines it's almost always accidental.

I also think this will affect a very small number of users, and it comes at such a small cost.