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

Bug for subfolders in matches_snapshot_location #918

Open
epenet opened this issue Nov 5, 2024 · 4 comments
Open

Bug for subfolders in matches_snapshot_location #918

epenet opened this issue Nov 5, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@epenet
Copy link
Contributor

epenet commented Nov 5, 2024

Describe the bug

PyTestLocation.PyTestLocation does not correct take into account sub-folders

def matches_snapshot_location(self, snapshot_location: str) -> bool:
loc = Path(snapshot_location)
# "test_file" should match_"test_file.ext" or "test_file/whatever.ext", but not
# "test_file_suffix.ext"
return self.basename == loc.stem or self.basename == loc.parent.name

This can be seen on Home Assistant repository, runing pytest --snapshot-details tests/components/config/test_config_entries.py tests/test_config.py will incorrect identify unused snapshots

The structure looks like this:

  • /tests/test_config.py
  • /tests/snapshots/test_config.ambr
  • /tests/test_config_entries.py
  • /tests/snapshots/test_config_entries.ambr
  • /tests/components/config/test_config_entries.py
  • /tests/components/config/snapshots/test_config_entries.ambr

I would expect tests/snapshots/test_config_entries.ambr to be fully ignored, and not to be marked as unused.

@epenet
Copy link
Contributor Author

epenet commented Nov 5, 2024

Actually, I think the bug comes from the discovery process.
Discovery for /tests/test_config.py should not include /tests/snapshots/test_config_entries.ambr

@epenet
Copy link
Contributor Author

epenet commented Nov 5, 2024

Well it's very confusing and I'm not sure how best to solve.

  • It gets discovered because it is in the snapshot directory for /tests/test_config.py (it loads all snapshot files in directory)
  • It gets marked as unused because the name matches for /tests/components/config/test_config_entries.py (it ignores the directory)

I'm not sure how to get around that...

@epenet
Copy link
Contributor Author

epenet commented Nov 5, 2024

I think the check should be adjusted in the report - maybe looping through the extensions again

https://github.com/syrupy-project/syrupy/blob/e337c888120e7b1d86e8df08181665ebe85fa966/src/syrupy/report.py#L500C9-L500C34

@epenet
Copy link
Contributor Author

epenet commented Nov 6, 2024

Note: a workaround was implemented in home-assistant/core#129946

But I do believe that:

  • pytest tests/folder should discover all files in tests/folder/__snapshots__
  • pytest tests/folder/file.py should ONLY discover tests/folder/__snapshots__/file.ambr

@noahnu noahnu added the bug Something isn't working label Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants