-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: migrate a pseudo_tty test to use assertSnapshot #47803
Conversation
Review requested:
|
@@ -0,0 +1,57 @@ | |||
[32m✔ should pass [90m(*ms)[39m[39m |
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.
Could we run these with ANSI colors support disabled to make easier-to-read snapshots?
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.
the entire purpose of this test is to test the colors and the reporter
b6f9dfd
to
12913f5
Compare
I spend hours to get a working Windows environment to figure out why tests were failing when it turns out this is the original behavior 😂 : node/test/pseudo-tty/testcfg.py Lines 143 to 145 in 76ae7be
|
Landed in 9e5e2f1 |
PR-URL: #47803 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
PR-URL: #47803 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
PR-URL: nodejs#47803 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
this is another followup for #47498 enabling running pseudo-tty snapshot tests