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 that untouched snapshots are not deleted in targeting mode #123

Merged
merged 23 commits into from
Feb 8, 2020

Conversation

mcataford
Copy link
Contributor

@mcataford mcataford commented Jan 31, 2020

Description

This PR adds some handling around filenames that can be passed to pytest to target testfiles. Previously, passing in file paths would run the tests on these paths, and consider snapshots that weren't touched as unused -- even though they were actually used by other tests not targeted by pytest in that instance.

By passing down a list of targeted files and using a filtering function, this takes out snapshots that aren't related to the targeted files, if pytest is passed some paths. This way, syrupy no longer purges snapshots by mistake.

Related Issues

This addresses #119.

Checklist

  • This PR has sufficient test coverage.
  • I have updated the CHANGELOG.md.

src/syrupy/utils.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Feb 5, 2020

Codecov Report

Merging #123 into master will not change coverage by %.
The diff coverage is n/a.

@@            Coverage Diff            @@
##            master      #123   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           16        16           
  Lines          705       715   +10     
=========================================
+ Hits           705       715   +10     

src/syrupy/__init__.py Outdated Show resolved Hide resolved
src/syrupy/__init__.py Show resolved Hide resolved
src/syrupy/extensions/base.py Outdated Show resolved Hide resolved
src/syrupy/utils.py Outdated Show resolved Hide resolved
src/syrupy/report.py Outdated Show resolved Hide resolved
src/syrupy/report.py Outdated Show resolved Hide resolved
src/syrupy/utils.py Outdated Show resolved Hide resolved
tests/test_integration_default.py Show resolved Hide resolved
src/syrupy/report.py Outdated Show resolved Hide resolved
CHANGELOG.md Outdated
@@ -10,6 +10,10 @@ From v1.0.0 onwards, this project adheres to [Semantic Versioning](https://semve

- Up to date with releases.

## [v0.3.2](https://github.com/tophat/syrupy/compare/v.0.3.1...v0.3.2)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@noahnu Is this the correct way to go about writing out the changelog manually?

@mcataford mcataford marked this pull request as ready for review February 7, 2020 22:09
@mcataford mcataford changed the title [WIP] Targeted snapshots Ensure that untouched snapshots are not deleted in targeting mode Feb 7, 2020
@mcataford mcataford requested a review from noahnu February 7, 2020 22:10
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@iamogbz iamogbz left a comment

Choose a reason for hiding this comment

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

This looks great! 🎉

src/syrupy/report.py Outdated Show resolved Hide resolved
src/syrupy/report.py Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@noahnu noahnu left a comment

Choose a reason for hiding this comment

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

Amazing!

@noahnu noahnu merged commit 0fbd86e into master Feb 8, 2020
@noahnu noahnu deleted the targeted-snapshots branch February 8, 2020 00:42
@noahnu
Copy link
Collaborator

noahnu commented Feb 8, 2020

@all-contributors please add @mcataford for code, test

@allcontributors
Copy link
Contributor

@noahnu

I've put up a pull request to add @mcataford! 🎉


target_snaps = {snap.location for snap in self.used}

return item.location in target_snaps
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think item.location in self.used would have just worked here without needing to create target_snaps

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.

3 participants