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

Ensure parameterized tests don't produce overlapping snapshot files. #267

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

bslatkin
Copy link

@bslatkin bslatkin commented Jan 30, 2025

The current escaping function replaces non-alphanumeric characters with the "_" character to ensure they are compatible with the filesystem's constraints. However, this normalization can cause a problem when two separate parameterized tests point at the same snapshot file after this normalization step.

This change modifies the normalization function to replace these characters with either their Unicode name or hexidecimal escape code. The resulting filenames are a bit verbose, but this approach ensures that these snapshots are always creating unique files.

If you revert the normalization function change, you'll note that the "overlap]" and "overlap[" tests in test_fmt.py will refer to the same snapshot file and the tests will fail and/or be flaky.

The current escaping function replaces non-alphanumeric characters with
the "_" character to ensure they are compatible with the filesystem's
constrainted. However, this normalization can cause a problem when two
separate parameterized tests point at the same snapshot file after this
normalization step.

This change modifies the normalization function to replace these
characters with either their Unicode name or hexidecimal escape code.
The resulting filenames are a bit verbose, but this approach ensures
that these snapshots are always creating unique files.

If you revert the normalization function change, you'll note that the
"overlap]" and "overlap[" tests in `test_fmt.py` will refer to the
same snapshot file and the tests will fail and/or be flaky.
Back to using the old character replacement algorithm again because
filenames were getting too long. Now putting a shake_128 hash on the
end of every snapshot file name to deal with any collisions. This
ends up only being 8 characters, which seems like a small price to
pay in order to ensure there are never any conflicts.
@bslatkin
Copy link
Author

bslatkin commented Feb 1, 2025

I ended up hitting issues with the old approach because filenames got too long. So now I've gone back to the same escaping algorithm that was there, but added a hash suffix of 8 characters that should break ties in the event the prefix is identical due to escaping.

Before this change it was hashing the escaped version of the strings,
which doesn't actually do anything to prevent collisions.
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

Successfully merging this pull request may close these issues.

1 participant