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.

Sign up for GitHub

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

Jump to bottom

[WIP] TEST: junitxml: write_captured_output: assert for flakiness #5959

Closed
wants to merge 3 commits into from
Closed

[WIP] TEST: junitxml: write_captured_output: assert for flakiness #5959

wants to merge 3 commits into from

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Oct 14, 2019

No description provided.

@blueyed blueyed closed this Oct 15, 2019
@blueyed blueyed reopened this Oct 15, 2019
This was referenced Oct 15, 2019
Closed
Merged
Merged
Merged
@blueyed blueyed changed the title [WIP] TEST: junitxml: write_captured_output: assert for flakiness [W I P] TEST: junitxml: write_captured_output: assert for flakiness Oct 15, 2019
blueyed
blueyed commented Oct 15, 2019
View reviewed changes
src/_pytest/junitxml.py
@@ -207,6 +207,8 @@ def write_captured_output(self, report):
else:
content = content_err

# XXX: flaky in test suite, trying to figure out why.
assert content, repr([content, content_log, content_err])
if content:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nicoddemus
Any idea why this might be Falsy during tests?
Does this happen during failures of outer tests then only?

Does it make sense to cover this branch, or should we either assert it here then always (probably a bad idea since it is covered sometimes), or just ignore it for coverage (# pragma: no branch)?

Copy link
Member

Choose a reason for hiding this comment

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

Not sure, do you have an example failure?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No.

@nicoddemus
Copy link
Member

(@blueyed please use [WIP] instead of [W I P] so the WIP bot can hold back the PR)

@nicoddemus nicoddemus changed the title [W I P] TEST: junitxml: write_captured_output: assert for flakiness [WIP] TEST: junitxml: write_captured_output: assert for flakiness Oct 15, 2019
@blueyed blueyed closed this Oct 15, 2019
@blueyed blueyed deleted the junitxml-flaky branch October 15, 2019 23:21
@blueyed
Copy link
Contributor Author

blueyed commented Oct 15, 2019

@nicoddemus
JFI: the flakiness there marks a lot of PRs with an red X due to the flaky coverage.

@nicoddemus
Copy link
Member

Yeah I've noticed. I did not notice though it coming from junitxml, but I've often seen flaky coverage in capture.py:

https://codecov.io/gh/pytest-dev/pytest/compare/afabbb6346e1b76cf84f367c3198e3313fbef959...c481df2edec82f4557058129e9f7b40d4f3973d8/changes

@blueyed
Copy link
Contributor Author

blueyed commented Oct 16, 2019 

if (
        not sys.platform.startswith("win32")
        or sys.version_info[:2] < (3, 6)
        or hasattr(sys, "pypy_version_info")
    ):
        return

I assume that is only the case before all jobs have finished then?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicoddemus nicoddemus nicoddemus left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@blueyed @nicoddemus