Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.1k
gh-128595: Add test class helper to force no terminal colour#128687
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
- You can disable colorizing for the whole class in
setUpClass()
instead ofsetUp()
. - Would not it be simpler to implement the core functionality as a generator-based context manager? You can use
enterContext()
orenterClassContext()
with it. - You can use
test.support.os_helper.EnvironmentVarGuard()
to restore the environment andtest.support.swap_attr()
to restore_colorize.can_colorize
.
You can also implement this as a mixin instead of patching a method (use asuper()
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.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
…ce_not_colorized_test_class
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.
Oh nice, the new code is more readable, I prefer context managers :-)
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
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. Much clearer now!
afb9dc8
intopython:mainUh oh!
There was an error while loading.Please reload this page.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry,@hugovk, I could not cleanly backport this to
|
Thanks for the reviews! |
…lour (pythonGH-128687)(cherry picked from commitafb9dc8)Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
GH-128778 is a backport of this pull request to the3.13 branch. |
Uh oh!
There was an error while loading.Please reload this page.
Split out from#128498, as requested at#128498 (comment).