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

feat: detect unused snapshots despite xdist #901

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

epenet
Copy link
Contributor

@epenet epenet commented Oct 15, 2024

Description

Improve xdist compatibility - with the ability to serialize/deserialize the report data.
See home-assistant/core#128162 for sample implementation

Related Issues

Known Limitations
pytest-xdist support only partially exists. There is no issue when it comes to reads however when you attempt to run pytest --snapshot-update, if running with more than 1 process, the ability to detect unused snapshots is disabled. See #535 for more information.
We welcome contributions to patch these known limitations.

Checklist

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

Additional Comments

No additional comments.

@epenet epenet changed the title Add ability to serialize reports Add ability to (de-)serialize reports for xdist compatibility Oct 15, 2024
@epenet epenet changed the title Add ability to (de-)serialize reports for xdist compatibility feat: add support for report serialization Oct 15, 2024
@epenet epenet changed the title feat: add support for report serialization feat: detect unused snapshots despite xdist Oct 17, 2024
@epenet epenet marked this pull request as ready for review October 17, 2024 07:57
@epenet epenet marked this pull request as draft October 17, 2024 11:11
@epenet epenet force-pushed the 20241015-1013 branch 2 times, most recently from aaf0912 to 83152c8 Compare October 18, 2024 08:28
Copy link

codecov bot commented Oct 21, 2024

Codecov Report

Attention: Patch coverage is 87.09677% with 8 lines in your changes missing coverage. Please review.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #901      +/-   ##
==========================================
- Coverage   97.74%   97.34%   -0.40%     
==========================================
  Files          21       21              
  Lines        1596     1658      +62     
==========================================
+ Hits         1560     1614      +54     
- Misses         36       44       +8     

@epenet epenet marked this pull request as ready for review October 21, 2024 12:23
@epenet
Copy link
Contributor Author

epenet commented Oct 28, 2024

After running successfully on Home Assistant - it was decided to revert it because serialization/de-serialization of the report was taking too long.
We may need to add an option so it only takes effect if --snapshot-update or --snapshot-details is set
Or maybe add a new settting --snapshot-xdist-report
See home-assistant/core#129311

@epenet epenet marked this pull request as draft October 28, 2024 09:59
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