-
Notifications
You must be signed in to change notification settings - Fork 84
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
Propagate known pytest failure/skip reasons to pytest summary #865
Conversation
👈 Launch a binder notebook on this branch for commit 07e2d85 I will automatically update this comment whenever this PR is modified 👈 Launch a binder notebook on this branch for commit 1ea22fa 👈 Launch a binder notebook on this branch for commit 75274a9 👈 Launch a binder notebook on this branch for commit 97ea6b5 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files@@ Coverage Diff @@
## main #865 +/- ##
===========================================
- Coverage 73.88% 58.72% -15.17%
===========================================
Files 31 13 -18
Lines 2003 1100 -903
===========================================
- Hits 1480 646 -834
+ Misses 523 454 -69 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick only :)
@@ -130,7 +130,7 @@ def test_store_can_create_s3_fsspec_session(self): | |||
|
|||
|
|||
@pytest.mark.xfail( | |||
reason="This test reproduces a bug (#610) which has not yet been fixed." | |||
reason="Expected failure: Reproduces a bug (#610) that has not yet been fixed." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how I feel about including the status in the reason string, but don't feel strongly enough to request anything. If it's helping someone, it's helping someone!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this because xfail
, xfailed
and XFAIL
require you to know Pytest well enough to interpret them as "expected failure". As soon as you know that, the x = expected makes sense, but if you don't there's nothing really to help you get there.
I think it's a little more general contributor-friendly this way, but I'm fine leaving this as a "Google it" thing if you don't understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good enough reason for me ;)
1ea22fa
to
75274a9
Compare
Co-authored-by: Matt Fisher <[email protected]>
In #858, there was some confusion on what an
xfailed
test meant and why the unit test suit was still considered passing. This PR adds the-rxXs
flag to PyTest calls (via nox) so that skips and excepted failure reasons (if they were provided) are reported in the test summary:I've also updated the error message to clarify what's happening in the pytest summary.
I don't think there's much value to a changelog entry here, so I've not added one; happy to add one is anyone things otherwise.
Pull Request (PR) draft checklist - click to expand
contributing documentation
before getting started.
title such as "Add testing details to the contributor section of the README".
Example PRs: #763
example
closes #1
. SeeGitHub docs - Linking a pull request to an issue.
Update
If such a section does not exist, please create one. FollowCHANGELOG.md
with details about your change in a section titled## Unreleased
.Common Changelog for your additions.
Example PRs: #763
Update the documentation and/or theConsider new environment variables, function names,README.md
with details of changes to theearthaccess interface, if any.
decorators, etc.
Click the "Ready for review" button at the bottom of the "Conversation" tab in GitHub
once these requirements are fulfilled. Don't worry if you see any test failures in
GitHub at this point!
Pull Request (PR) merge checklist - click to expand
Please do your best to complete these requirements! If you need help with any of these
requirements, you can ping the
@nsidc/earthaccess-support
team in a comment and wewill help you out!
Request containing "pre-commit.ci autofix" to automate this.
📚 Documentation preview 📚: https://earthaccess--865.org.readthedocs.build/en/865/