FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-128595: Add test class helper to force no terminal colour by hugovk · Pull Request #128687 · python/cpython · GitHub

/ cpython Public

gh-128595: Add test class helper to force no terminal colour - #128687

Merged
hugovk merged 10 commits into
python:mainfrom
hugovk:3.14-force_not_colorized_test_class
Jan 13, 2025
ConversationCommits (10)ChecksFiles changed
Merged

gh-128595: Add test class helper to force no terminal colour#128687
hugovk merged 10 commits into
python:mainfrom
hugovk:3.14-force_not_colorized_test_class

Conversation

hugovk commented Jan 9, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

hugovk added tests Tests in the Lib/test dir skip news needs backport to 3.13 bugs and security fixes labels Jan 9, 2025
bedevere-app Bot mentioned this pull request Jan 9, 2025
Closed
hugovk mentioned this pull request Jan 9, 2025
Merged
serhiy-storchaka reviewed Jan 10, 2025
View reviewed changes

serhiy-storchaka left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
  • You can disable colorizing for the whole class in setUpClass() instead of setUp().
  • Would not it be simpler to implement the core functionality as a generator-based context manager? You can use enterContext() or enterClassContext() with it.
  • You can use test.support.os_helper.EnvironmentVarGuard() to restore the environment and test.support.swap_attr() to restore _colorize.can_colorize.

You can also implement this as a mixin instead of patching a method (use a super() call in an overridden method). I do not say that it would be better, but it is just an alternative which you could have overlooked.

Comment thread Lib/test/test_code_module.py Outdated
erlend-aasland reviewed Jan 10, 2025
View reviewed changes
Comment thread Lib/test/support/__init__.py Outdated
erlend-aasland reviewed Jan 10, 2025
View reviewed changes
Comment thread Lib/test/support/__init__.py Outdated
vstinner reviewed Jan 10, 2025
View reviewed changes
Comment thread Lib/test/support/__init__.py Outdated
hugovk requested a review from iritkatriel as a code owner January 10, 2025 15:15
vstinner reviewed Jan 10, 2025
View reviewed changes

vstinner left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Oh nice, the new code is more readable, I prefer context managers :-)

Comment thread Lib/test/support/__init__.py Outdated
Comment thread Lib/test/support/__init__.py Outdated
serhiy-storchaka approved these changes Jan 10, 2025
View reviewed changes

serhiy-storchaka left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM. Much clearer now!

hugovk enabled auto-merge (squash) January 13, 2025 11:01
hugovk merged commit afb9dc8 into python:main Jan 13, 2025
hugovk deleted the 3.14-force_not_colorized_test_class branch January 13, 2025 11:05

This comment was marked as outdated.

Sign in to view

miss-islington-app Bot commented Jan 13, 2025

Copy link
Copy Markdown

Sorry, @hugovk, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker afb9dc887c6e8ae17b6a54c6124399e8bdc82253 3.13

hugovk commented Jan 13, 2025

Copy link
Copy Markdown
Member Author

Thanks for the reviews!

hugovk added a commit to hugovk/cpython that referenced this pull request Jan 13, 2025
[3.13] pythongh-128595: Add test class helper to force no terminal co…
131c9f0 
…lour (pythonGH-128687)

(cherry picked from commit afb9dc8)

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>

bedevere-app Bot commented Jan 13, 2025

Copy link
Copy Markdown

GH-128778 is a backport of this pull request to the 3.13 branch.

bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Jan 13, 2025
hugovk added a commit that referenced this pull request Jan 13, 2025
[3.13] gh-128595: Add test class helper to force no terminal colour (G…
afcf238 
…H-128687) (#128778)

Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
hugovk mentioned this pull request Jan 22, 2025
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

vstinner vstinner left review comments
serhiy-storchaka serhiy-storchaka approved these changes
erlend-aasland erlend-aasland approved these changes
iritkatriel Awaiting requested review from iritkatriel iritkatriel is a code owner

Assignees

hugovk

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.

4 participants

Back | FazBrowse Home | New Git URL