GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-138013: Remove `test_io` load_tests namespace manipulation by cmaloney · Pull Request #138366 · python/cpython

Conversation

@cmaloney

@cmaloney cmaloney commented Sep 2, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Reduce what happens in load_tests so that the next change, moving the Buffered* tests to test_bufferdio is purely mechanical movement and updating imports.

This adds two classes, one per I/O implementation, to act as dispatch to the implementation-specific mocks as well as module members. Previously the mappings CTestCase and PyTestCase provide were injected directly during load_tests.

CTestCase and PyTestCase inherit from unittest.TestCase so when the split happens default test discovery will work for the classes in test_bufferedio. test_general keeps a manual test list for this refactoring; some of the tests (ex. ProtocolsTest) aren't currently run and fixing that + helpers to not be picked up as tests is out of my current scope.

CTestCase and PyTestCase have an io class member which points to the implementation meaning that can be removed from individual test cases which now inherit from them.

Tested locally with ./python -m test test_io -uall,walltime,largefile,extralargefile -M16G -j8 -v


  1. Could this get skip news as its a test-only change?
  2. Could buildbots be run against this? Worried there are lots of platform-specific cases in these test cases and would rather proactively debug/fix than reactively (Ex. 4 tests are skipped for me on my 64 bit Linux box because: "only run in 32-bit address space" and "only when utf-8 mode is disabled")
  • Issue: Move Buffered tests from test_io to test_bufio #138013

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@cmaloney
pythongh-138013: Remove test_io load_tests namespace manipulation
4c87ba8
Reduce what happens in `load_tests` so that the next change,
moving the `Buffered*` tests to `test_bufferdio` is purely mechanical
movement and updating imports.

This adds two classes, one per I/O implementation, to act as dispatch to
the implementation-specific mocks as well as module members. Previously
the mappings CTestCase and PyTestCase provide were injected directly
during `load_tests`.

CTestCase and PyTestCase inherit from `unittest.TestCase` so when the
split happens default test discovery will work for the classes in
`test_bufferedio`. `test_general` keeps a manual test list for this
refactoring; some of the tests (ex. `ProtocolsTest`) aren't currently
run and fixing that + helpers to not be picked up is out of my current
scope.

CTestCase and PyTestCase have an `io` class member which points to the
implementation meaning that can be removed from individual test cases
which now inherit from them.
@bedevere-app bedevere-app Bot added awaiting review tests Tests in the Lib/test dir labels Sep 2, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Sep 2, 2025
Move Buffered tests from test_io to test_bufio #138013
Closed
@cmaloney
pythongh-133982: Refer to implementation specific mocks
751788f
This code is picking up `MockRawIO` which is defined globally in the
module but these should use the mock specific to the I/O implementation
being tested.
@cmaloney

cmaloney commented Sep 2, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Added another commit to fix a gh-133982 issue I found while testing out the split. There are a number of test classes defined inside specific test cases that were referring to MockRawIO directly rather than self.MockRawIO

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

auvipy
auvipy approved these changes Sep 2, 2025
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting core review and removed awaiting review labels Sep 2, 2025
@vstinner vstinner added skip news 🔨 test-with-buildbots Test PR w/ buildbots; report in status section labels Sep 2, 2025
@bedevere-bot

bedevere-bot commented Sep 2, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @vstinner for commit 751788f 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F138366%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Sep 2, 2025
@vstinner

vstinner commented Sep 2, 2025

Copy link
Copy Markdown
Member

Could this get skip news as its a test-only change?

Done.

Could buildbots be run against this?

Done.

cmaloney reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner
vstinner approved these changes Sep 2, 2025
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

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.

LGTM.

./python -m test test_io --list-cases output is unchanged with this change.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Lib/test/test_io/test_general.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/test/test_io/test_general.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Sep 2, 2025
@cmaloney cmaloney mentioned this pull request Sep 2, 2025
gh-127647: Add typing.Reader and Writer protocols #127648
Merged
@cmaloney @vstinner
Apply suggestions from code review
1ebd6ff
Remove unnecessary pass through self

Co-authored-by: Victor Stinner <[email protected]>
@cmaloney

cmaloney commented Sep 3, 2025

Copy link
Copy Markdown
Contributor Author

One bot failure buildbot/AMD64 Windows 11 Refleaks PR in test_remote_pdb which looks independent (Code this PR doesn’t touch), so I think this is good to go once the suggestion application finishes in Github actions bots

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner
vstinner merged commit 6dd21e9 into python:main Sep 3, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Sep 3, 2025
@vstinner

vstinner commented Sep 3, 2025

Copy link
Copy Markdown
Member

Merged, thanks.

cmaloney reacted with hooray emoji
All reactions
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@cmaloney
cmaloney deleted the test_io_simplify_load_tests branch September 3, 2025 16:49
lkollar pushed a commit to lkollar/cpython that referenced this pull request Sep 9, 2025
@cmaloney @vstinner @lkollar
pythongh-138013: Remove test_io load_tests namespace manipulation (p…
1118a81
…ython#138366)

Reduce what happens in `load_tests` so that the next change,
moving the `Buffered*` tests to `test_bufferdio` is purely mechanical
movement and updating imports.

This adds two classes, one per I/O implementation, to act as dispatch to
the implementation-specific mocks as well as module members. Previously
the mappings CTestCase and PyTestCase provide were injected directly
during `load_tests`.

CTestCase and PyTestCase inherit from `unittest.TestCase` so when the
split happens default test discovery will work for the classes in
`test_bufferedio`. `test_general` keeps a manual test list for this
refactoring; some of the tests (ex. `ProtocolsTest`) aren't currently
run and fixing that + helpers to not be picked up is out of my current
scope.

CTestCase and PyTestCase have an `io` class member which points to the
implementation meaning that can be removed from individual test cases
which now inherit from them.

This code is picking up `MockRawIO` which is defined globally in the
module but these should use the mock specific to the I/O implementation
being tested.

Co-authored-by: Victor Stinner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes
+1 more reviewer
@auvipy auvipy auvipy approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@cmaloney @bedevere-bot @vstinner @auvipy
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.