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

fix: ignore test_a_suffix snapshots when running test_a #607

Merged
merged 3 commits into from
Jul 7, 2022

Conversation

huonw
Copy link
Contributor

@huonw huonw commented Jul 5, 2022

Description

This fixes #596 by making the filename vs. snapshot location matching stricter: instead of allowing a filename of test_file to match if it's a substring of the location (e.g. FOOtest_fileBAR would match), it now only matches if either of these are true:

  • the filename exactly matches, ignoring the extension (e.g. test_file.ambr matches, while test_file_whatever.ambr does not)
  • the parent directory exactly matches (e.g. .../test_file/foo.bar matches, but .../test_file.foo/bar does not, nor .../test_file/subdirectory/foo.bar)

I've added tests for this, as well as expanding the existing ones for #529 to include checking test names that have matching suffices (especially relevant for tests/integration/test_snapshot_similar_names_file_extension.py, with the single file snapshots). All three sets of changed tests fail without the change to location.py.

Related Issues

Checklist

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

Additional Comments

Thanks for Syrupy!

Comment on lines 120 to 122
return self.filename == loc.stem or any(
self.filename == parent.name for parent in loc.parents
)
Copy link
Collaborator

@noahnu noahnu Jul 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is to handle the SingleFileExtension (image + json). Right now it's not strict enough since the parents will traverse all the way up to the root of the file system.

My proposed logic:

Suggested change
return self.filename == loc.stem or any(
self.filename == parent.name for parent in loc.parents
)
return self.filename == loc.stem or self.filename == loc.parent.name

This does break the ability for an extension to have additional depth in their snapshot directories, but I think that's an okay compromise (my plans for syrupy in v3 are to restrict the API a bit to make internal refactoring easier -- we were a bit overzealous with how "extensible" we made everything).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me 👍 I've made that update and validated it in the tests.

@noahnu noahnu merged commit 988a8ab into tophat:master Jul 7, 2022
tophat-opensource-bot pushed a commit that referenced this pull request Jul 7, 2022
## [2.3.1](v2.3.0...v2.3.1) (2022-07-07)

### Bug Fixes

* ignore test_a_suffix snapshots when running test_a ([#607](#607)) ([988a8ab](988a8ab))
@tophat-opensource-bot
Copy link
Collaborator

🎉 This PR is included in version 2.3.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@huonw huonw deleted the bugfix/596-suffices branch July 7, 2022 23:01
@huonw
Copy link
Contributor Author

huonw commented Jul 7, 2022

Woohoo, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unused snapshots when running on filename (test_a.py) that's the prefix of another (test_abc.py)
3 participants