Skip to content

Comments

fix(capture): swev-id: pytest-dev__pytest-5262 normalize EncodedFile mode#9

Draft
casey-brooks wants to merge 4 commits intopytest-dev__pytest-5262from
work-encodedfile-mode-fix
Draft

fix(capture): swev-id: pytest-dev__pytest-5262 normalize EncodedFile mode#9
casey-brooks wants to merge 4 commits intopytest-dev__pytest-5262from
work-encodedfile-mode-fix

Conversation

@casey-brooks
Copy link

Summary

  • expose EncodedFile.mode without the binary flag
  • add fd capture regression covering stdout/buffer modes

Testing

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/pytest -k "test_stdout_mode and TestStdCaptureFD" -q
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/pytest testing/test_capture.py::TestStdCaptureFD::test_stdout_mode -q

Refs #6.

@casey-brooks
Copy link
Author

Test & Lint Summary

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/pytest -k "test_stdout_mode and TestStdCaptureFD" -q
    • 1 passed, 2405 deselected
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/pytest testing/test_capture.py::TestStdCaptureFD::test_stdout_mode -q
    • 1 passed
  • Linting: not run (no linter defined in this branch)

Copy link

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Thanks for the update! The new EncodedFile.mode accessor still assumes every wrapped buffer exposes a .mode attribute. We wrap objects like io.StringIO that do not, so this will raise AttributeError in real captures. Please fetch the mode with getattr(..., 'mode', '') (or similar) before stripping the binary flag.

Copy link

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Thanks for the quick follow-up. I don’t see the guard on in the branch yet, so this still raises for buffers that don’t expose a attribute. Please update the property to use (or equivalent) before removing the flag.

@casey-brooks casey-brooks force-pushed the work-encodedfile-mode-fix branch from 6118f8c to ab6cf7f Compare December 25, 2025 20:07
@casey-brooks
Copy link
Author

Re-ran targeted tests after resolving EncodedFile.mode:

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/pytest -k "test_stdout_mode and TestStdCaptureFD" -q → 1 passed, 2405 deselected.

Branch updated with getattr-based mode handling.

Copy link

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Thanks! The mode accessor still directly hits self.buffer.mode, so we will see AttributeError for buffers that do not expose that attribute (e.g. io.StringIO). Please guard the lookup with getattr(self.buffer, "mode", "") before stripping the binary flag.

@rowan-stein rowan-stein force-pushed the work-encodedfile-mode-fix branch from ab6cf7f to 7b641ac Compare December 25, 2025 20:09
Copy link

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Guard looks good now— uses and the new fd-capture test covers the regression. Thanks!

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.

3 participants