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

capture: fix disabled()/global_and_fixture_disabled() enabling capturing when it was disabled #7651

Merged
merged 1 commit into from Aug 24, 2020

Conversation

bluetech
Copy link
Member

The CaptureManager.global_and_fixture_disabled() context manager (and CaptureFixture.disabled() which calls it) did suspend(); ...; resume() but if the capturing was already suspended, the resume() would resume it when it shouldn't.

This caused caused some messages to be swallowed when --log-cli is used because it uses global_and_fixture_disabled when capturing is not necessarily resumed.

…ing when it was disabled

The `CaptureManager.global_and_fixture_disabled()` context manager (and
`CaptureFixture.disabled()` which calls it) did `suspend(); ...;
resume()` but if the capturing was already suspended, the `resume()`
would resume it when it shouldn't.

This caused caused some messages to be swallowed when `--log-cli` is
used because it uses `global_and_fixture_disabled` when capturing is not
necessarily resumed.
@bluetech
Copy link
Member Author

The code that coverage says is uncovered is definitely executed by the test_disabled_capture_fixture_twice test, but maybe because it runs in a subprocess, coverage doesn't know about it? (not sure how it works exactly)

@nicoddemus nicoddemus added the needs backport applied to PRs, indicates that it should be ported to the current bug-fix branch label Aug 22, 2020
@nicoddemus
Copy link
Member

The code that coverage says is uncovered is definitely executed by the test_disabled_capture_fixture_twice test, but maybe because it runs in a subprocess, coverage doesn't know about it? (not sure how it works exactly)

That's possible, coverage does need special handling to be enabled from subprocesses:

https://coverage.readthedocs.io/en/coverage-5.2.1/subprocess.html#configuring-python-for-sub-process-coverage

@bluetech
Copy link
Member Author

Thanks for the link @nicoddemus, looks a bit too involved for me to get working ATM, so I opened #7679 to track this.

@bluetech bluetech merged commit bb38ae9 into pytest-dev:master Aug 24, 2020
@bluetech bluetech deleted the capture-safe-disable branch August 24, 2020 15:04
bluetech added a commit to bluetech/pytest that referenced this pull request Sep 4, 2020
capture: fix disabled()/global_and_fixture_disabled() enabling capturing when it was disabled

(cherry picked from commit bb38ae9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs backport applied to PRs, indicates that it should be ported to the current bug-fix branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants