.ambr snapshot files have meaningful trailing whitespace

[harrysarson]

Describe the bug

When generating snapshots from mutliline strings the text is idented in the snapshot file. This means that empty lines in multiline strings generate lines containing only (trailling) whitespace in the line.

Trimming this whitespace then invalidates the snapshot causing the test to fail.

To reproduce

def test(snapshot):
    lines = "\n".join([
        "line 1",
        "", # empty string
        "line 3"
    ])
    assert lines == snapshot

This test passes fine.

Then trim trailing whitespace on the generated snapshot.

Now the test fails.

Expected behavior

Snapshot files shouldn't have meaningful trailing whitespace.

[noahnu]

Snapshots are an exact representation of the data you are snapshotting. This includes whitespace. This is intentional.

If using something like editor config to trim whitespace, I'd recommend adding a rule to exclude ambr files.

[noahnu]

To provide some additional context: consider the snapshotting of an API response. If whitespace changes in the generated data, we want to know about it, because it will invalidate any cache layers. The hash of the file contents will also differ, and version control systems will also note differences.

If you don't want whitespaces in your snapshotted data, beyond modifying your linters, you could also trim whitespace in your code. If you don't want to modify your code itself, you could extend the snapshot fixture in pytest to do so there.

[harrysarson]

Sorry I should have been clear: there is no trailing whitespace in the string I am snapshotting. The trailing whitespace is added by syrupy to multiline strings that do not have trailing whitespace (but do have empty lines).

This is normally fine because the addition of trailing whitespace happens consistently (and so assertions pass) but when modifying files to remove trailing whitespace (e.g. vscode does this for me by default) the asserts then fail.

If you don't want whitespaces in your snapshotted data, beyond modifying your linters, you could also trim whitespace in your code. If you don't want to modify your code itself, you could extend the snapshot fixture in pytest to do so there.

I cannot fix this by modifying code.

The strings in question do not contain trailing whitepsace.

The only work around I have in application code is to remove empty lines or add a token character to the end of each line.

[noahnu]

I see. This is because each line is indented by the same amount. The idea is that the snapshot diff between:

data = ["a", "", "b"]

and

data = ["a", " ", "b"]

is exactly 1 character, the additional whitespace. In terms of correctness of the data, I think what you're suggesting could work, as long as we're consistent.

Let me think about this some more, so will re-open this as a feature request. I still think it makes sense to just add an exception for snapshot files to your linter. If you use an editorconfig, which most IDEs support (or at least through an extension/plugin), then you can just add the file extension for .ambr and set trim_trailing_whitespace=false. In fact, I'd recommend disabling all autoformatting in snapshot files. We're very explicit about line endings.

[noahnu]

but when modifying files to remove trailing whitespace (e.g. vscode does this for me by default)

To confirm, this is in snapshots or the source files? For snapshots, see the editorconfig comment. For source files, you should run the linter before generating the snapshots.

[harrysarson]

Thanks

To confirm, this is in snapshots or the source files?

In the snapshot files.

I think my specific request here is: (configurably?) generate snapshot files that do not contain trailing whitespace when snap-shotting multiline strings that contain empty lines (but not trailing whitespace).

[noahnu]

The amber serializer relies on indentation to detect snapshot data. Removing the indentation just for empty lines complicates the serializer implementation IMO. Also consider this scenario:

Before:

def test_case(snapshot):
    assert snapshot == ["", ""]

After:

def test_case(snapshot):
    assert snapshot == ["", " "]

In the git diff after making the above change, I expect a single character to be changed in the diff. If we instead were to truncate empty lines, the diff would appear larger, as it would contain the new whitespace plus the indentation.

For this reason, I would prefer not to implement this as a feature. Instead, I would recommend excluding ambr files from your IDE tooling. If trailing whitespaces are being truncated by editorconfig for example, you can add an exception for ambr files to preserve the whitespace.