test: disable colors in test-assert.js #20695
Merged
Conversation
This file contains 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
Trott
approved these changes
May 13, 2018
trivikr
approved these changes
May 13, 2018
danbev
approved these changes
May 13, 2018
lpinca
approved these changes
May 13, 2018
lpinca
added
the
author ready
|
Is this the only issue here? Can we please verify that all |
|
I had only looked into test-assert.js because that one had the open issue. I just went through the other assert tests and fixed the other two that are broken due to colors. UPDATE: New CI that was good: https://ci.nodejs.org/job/node-test-pull-request/14875/ |
|
@cjihrig I guess those are all that fail but would you be so kind and just run the whole test suite once nevertheless? Some message tests could for example also fail but who knows if there are some other ones that could also fail. |
|
Is there a way to run the whole test suite with a TTY as stdout? I've never needed to do that before, so I'm not sure how. |
jasnell
approved these changes
May 14, 2018
When test/parallel/test-assert.js is run with a TTY as stdout, color codes in assertion messages cause the test to fail. This commit disables colors when stdout is a TTY. Fixes: nodejs#18967 PR-URL: nodejs#20695 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
When test/parallel/test-assert-deep.js is run with a TTY as stdout, color codes in assertion messages cause the test to fail. This commit disables colors when stdout is a TTY. PR-URL: nodejs#20695 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
When test/parallel/test-assert-checktag.js is run with a TTY as stdout, color codes in assertion messages cause the test to fail. This commit disables colors when stdout is a TTY. PR-URL: nodejs#20695 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins
pushed a commit
that referenced
this pull request
May 22, 2018
When test/parallel/test-assert.js is run with a TTY as stdout, color codes in assertion messages cause the test to fail. This commit disables colors when stdout is a TTY. Fixes: #18967 PR-URL: #20695 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins
pushed a commit
that referenced
this pull request
May 22, 2018
When test/parallel/test-assert-deep.js is run with a TTY as stdout, color codes in assertion messages cause the test to fail. This commit disables colors when stdout is a TTY. PR-URL: #20695 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins
pushed a commit
that referenced
this pull request
May 22, 2018
When test/parallel/test-assert-checktag.js is run with a TTY as stdout, color codes in assertion messages cause the test to fail. This commit disables colors when stdout is a TTY. PR-URL: #20695 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When
test/parallel/test-assert.jsis run with a TTY as stdout, color codes in assertion messages cause the test to fail. This commit disables colors when stdout is a TTY.Fixes: #18967
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes