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: unused snapshot not filtered out when specific node #531

Merged
merged 1 commit into from
Aug 18, 2021
Merged

Conversation

noahnu
Copy link
Collaborator

@noahnu noahnu commented Aug 2, 2021

@noahnu noahnu changed the title test: add coverage for duplicate test case names fix: unused snapshot falsely detected when tests have the same name Aug 2, 2021
@noahnu noahnu requested a review from iamogbz August 2, 2021 18:15
@noahnu noahnu marked this pull request as ready for review August 2, 2021 18:15
@codecov
Copy link

codecov bot commented Aug 2, 2021

Codecov Report

Merging #531 (9a0bef3) into master (c8ed886) will decrease coverage by 0.08%.
The diff coverage is 92.85%.

@@             Coverage Diff             @@
##            master     #531      +/-   ##
===========================================
- Coverage   100.00%   99.91%   -0.09%     
===========================================
  Files           19       19              
  Lines         1105     1114       +9     
===========================================
+ Hits          1105     1113       +8     
- Misses           0        1       +1     

@noahnu noahnu changed the title fix: unused snapshot falsely detected when tests have the same name fix: unused snapshot not filtered out when specific node Aug 2, 2021
Comment on lines +204 to +207
and self._provided_nodes_match_name(
snapshot_location=snapshot_location,
snapshot_name=snapshot.name,
provided_nodes=provided_nodes,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This bit might actually be dead code. It's never hit in tests. I don't think it's harmful at the moment, so will loop back to see if it can be removed, or if there's some edge case we're not testing.

Comment on lines +362 to +367
if not provided_nodes:
return True
for node_path in provided_nodes:
if snapshot_name in ".".join(node_path):
return True
return False
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This syntax is easier to debug.

"test_2.py::test_a",
)
result.stdout.re_match_lines("2 snapshots passed")
assert result.ret == 0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Duplicated this test file since locations for images are handled a bit differently, better safe than sorry.

@noahnu noahnu merged commit d0c8ca8 into master Aug 18, 2021
@noahnu noahnu deleted the bugfix/529 branch August 18, 2021 00:39
tophat-opensource-bot pushed a commit that referenced this pull request Aug 18, 2021
## [1.4.1](v1.4.0...v1.4.1) (2021-08-18)

### Bug Fixes

* unused snapshot not filtered out when tests have similar names, close [#529](#529) ([#531](#531)) ([d0c8ca8](d0c8ca8))
@tophat-opensource-bot
Copy link
Collaborator

🎉 This PR is included in version 1.4.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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 snapshot falsely detected when tests have same names
2 participants