Skip to content

Add tests for TextIOWrapper.reconfigure() with invalid encoding/errors#154012

Open
magic-peach wants to merge 2 commits into
python:mainfrom
magic-peach:add-textio-reconfigure-tests
Open

Add tests for TextIOWrapper.reconfigure() with invalid encoding/errors#154012
magic-peach wants to merge 2 commits into
python:mainfrom
magic-peach:add-textio-reconfigure-tests

Conversation

@magic-peach

Copy link
Copy Markdown

Summary

Replace four explicit TODO comments in test_reconfigure_errors with actual test assertions for TextIOWrapper.reconfigure().

Changes

The following TODOs in Lib/test/test_io/test_textio.py:1291-1299 are replaced with actual tests:

  • txt.reconfigure(encoding='utf-8\0') — verifies null byte in encoding name raises LookupError
  • txt.reconfigure(encoding='nonexisting') — verifies unknown encoding raises LookupError
  • txt.reconfigure(errors='ignore\0') — verifies null byte in error handler raises LookupError
  • txt.reconfigure(errors='nonexisting') — verifies unknown error handler raises LookupError

These follow the same assertRaises(LookupError) pattern already used for encoding='locale\0' on the line above.

Motivation

These four test cases were explicitly marked as TODOs in the test file. Adding them improves test coverage for TextIOWrapper.reconfigure() error handling and removes stale TODO comments.

…text

Add focused test coverage for the domain-specific gettext functions
that were previously only tested indirectly. This extends the work
started in pythongh-134594 to cover all four domain-specific functions.

Each function is tested with a for loop and subTests covering:
- Existing translations
- Missing translations (fallback to original message)
- Nonexistent domains (fallback to original message)
- Context-based translations (dpgettext, dnpgettext)
- Plural forms (dngettext, dnpgettext)
- Wrong/missing contexts

Updated the TODO comment to reference the issue and additional
functions that now have coverage.
Replace four explicit TODO comments in test_reconfigure_errors with
actual test assertions:

- encoding='utf-8\0' raises LookupError (null byte in encoding name)
- encoding='nonexisting' raises LookupError (unknown encoding)
- errors='ignore\0' raises LookupError (null byte in error handler)
- errors='nonexisting' raises LookupError (unknown error handler)

These follow the same pattern already used in the test for
encoding='locale\0'.
@magic-peach
magic-peach requested a review from tomasr8 as a code owner July 18, 2026 16:08
@bedevere-app bedevere-app Bot added the tests Tests in the Lib/test dir label Jul 18, 2026
@bedevere-app

bedevere-app Bot commented Jul 18, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@python-cla-bot

Copy link
Copy Markdown

The following commit authors need to sign the Contributor License Agreement:

CLA not signed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 49a23f1fe7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1291 to +1294
with self.assertRaises(LookupError):
txt.reconfigure(encoding='utf-8\0')
with self.assertRaises(LookupError):
txt.reconfigure(encoding='nonexisting')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep invalid reconfigure cases implementation-specific

When this shared test runs for PyTextIOWrapperTest, these new assertRaises(LookupError) checks fail: _pyio.TextIOWrapper.reconfigure() only stores the new encoding/errors values here and does not look them up until an encoder/decoder is later created, so encoding='utf-8\0', encoding='nonexisting', and the analogous invalid errors cases below return normally. In a normal C build, utf-8\0 and invalid error-handler names are also accepted at reconfigure time because the C path passes nul-terminated strings and does not validate error handlers unless debug/dev-mode checks apply, so this makes test_io fail instead of adding passing coverage.

Useful? React with 👍 / 👎.

Comment thread Lib/test/test_gettext.py

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we please revert these unrelated changes here?

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

Labels

awaiting review tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants