Add tests for TextIOWrapper.reconfigure() with invalid encoding/errors#154012
Add tests for TextIOWrapper.reconfigure() with invalid encoding/errors#154012magic-peach wants to merge 2 commits into
Conversation
…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'.
|
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 |
|
The following commit authors need to sign the Contributor License Agreement: |
There was a problem hiding this comment.
💡 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".
| with self.assertRaises(LookupError): | ||
| txt.reconfigure(encoding='utf-8\0') | ||
| with self.assertRaises(LookupError): | ||
| txt.reconfigure(encoding='nonexisting') |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
Can we please revert these unrelated changes here?
Summary
Replace four explicit TODO comments in
test_reconfigure_errorswith actual test assertions forTextIOWrapper.reconfigure().Changes
The following TODOs in
Lib/test/test_io/test_textio.py:1291-1299are replaced with actual tests:txt.reconfigure(encoding='utf-8\0')— verifies null byte in encoding name raisesLookupErrortxt.reconfigure(encoding='nonexisting')— verifies unknown encoding raisesLookupErrortxt.reconfigure(errors='ignore\0')— verifies null byte in error handler raisesLookupErrortxt.reconfigure(errors='nonexisting')— verifies unknown error handler raisesLookupErrorThese follow the same
assertRaises(LookupError)pattern already used forencoding='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.