-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Fix terminal reporter output not appearing with capture active #13848
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,6 @@ | |
| from typing import Literal | ||
| from typing import NamedTuple | ||
| from unittest.mock import Mock | ||
| from unittest.mock import patch | ||
|
|
||
| import pluggy | ||
|
|
||
|
|
@@ -3418,17 +3417,51 @@ def write_raw(s: str, *, flush: bool = False) -> None: | |
| def test_plugin_registration(self, pytester: pytest.Pytester) -> None: | ||
| """Test that the plugin is registered correctly on TTY output.""" | ||
| # The plugin module should be registered as a default plugin. | ||
| with patch.object(sys.stdout, "isatty", return_value=True): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain why need to change this test? |
||
| config = pytester.parseconfigure() | ||
| plugin = config.pluginmanager.get_plugin("terminalprogress") | ||
| assert plugin is not None | ||
| # Use a mock file with isatty returning True | ||
| from io import StringIO | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better to have the imports at the beginning of the file if they don't need to be "lazy". Also below. |
||
|
|
||
| class MockTTY(StringIO): | ||
| def isatty(self): | ||
| return True | ||
|
|
||
| def fileno(self): | ||
| return 1 | ||
|
|
||
| mock_file = MockTTY() | ||
| config = pytester.parseconfig() | ||
| # Manually trigger pytest_configure with our mock file | ||
| from _pytest.terminal import TerminalProgressPlugin | ||
| from _pytest.terminal import TerminalReporter | ||
|
|
||
| reporter = TerminalReporter(config, mock_file) | ||
| config.pluginmanager.register(reporter, "terminalreporter") | ||
| # Check that plugin would be registered based on isatty | ||
| if reporter.isatty(): | ||
| plugin = TerminalProgressPlugin(reporter) | ||
| config.pluginmanager.register(plugin, "terminalprogress") | ||
|
|
||
| retrieved_plugin = config.pluginmanager.get_plugin("terminalprogress") | ||
| assert retrieved_plugin is not None | ||
|
|
||
| def test_disabled_for_non_tty(self, pytester: pytest.Pytester) -> None: | ||
| """Test that plugin is disabled for non-TTY output.""" | ||
| with patch.object(sys.stdout, "isatty", return_value=False): | ||
| config = pytester.parseconfigure() | ||
| plugin = config.pluginmanager.get_plugin("terminalprogress") | ||
| assert plugin is None | ||
| # Use a mock file with isatty returning False | ||
| from io import StringIO | ||
|
|
||
| class MockNonTTY(StringIO): | ||
| def isatty(self): | ||
| return False | ||
|
|
||
| mock_file = MockNonTTY() | ||
| config = pytester.parseconfig() | ||
| # Manually trigger pytest_configure with our mock file | ||
| from _pytest.terminal import TerminalReporter | ||
|
|
||
| reporter = TerminalReporter(config, mock_file) | ||
| config.pluginmanager.register(reporter, "terminalreporter") | ||
| # Plugin should NOT be registered for non-TTY | ||
| plugin = config.pluginmanager.get_plugin("terminalprogress") | ||
| assert plugin is None | ||
|
|
||
| @pytest.mark.parametrize( | ||
| ["state", "progress", "expected"], | ||
|
|
@@ -3508,3 +3541,25 @@ def test_session_lifecycle( | |
| # Session finish - should remove progress. | ||
| plugin.pytest_sessionfinish() | ||
| assert "\x1b]9;4;0;\x1b\\" in mock_file.getvalue() | ||
|
|
||
|
|
||
| def test_terminal_reporter_write_with_capture(pytester: Pytester) -> None: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test seems to pass also in main, so needs some refinement if we want to prevent regression. |
||
| """Test that reporter.write() works correctly even with output capture active. | ||
|
|
||
| Regression test for issue #8973. | ||
| When calling reporter.write() with flush=True during test execution, | ||
| the output should appear in the terminal even when output capture is active. | ||
| """ | ||
| pytester.makepyfile( | ||
| """ | ||
| def test_reporter_write(request): | ||
| reporter = request.config.pluginmanager.getplugin("terminalreporter") | ||
| reporter.ensure_newline() | ||
| reporter.write("CUSTOM_OUTPUT", flush=True) | ||
| assert True | ||
| """ | ||
| ) | ||
| result = pytester.runpytest("-v") | ||
| # The custom output should appear in the captured output | ||
| result.stdout.fnmatch_lines(["*CUSTOM_OUTPUT*"]) | ||
| result.assert_outcomes(passed=1) | ||
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.
It seems weird to me that this is in config/ when config doesn't really care about it. It should be in terminal.py or maybe capture.py if we want to take care of it "at the source".