test: add tests for console.[info|error|warn] #6538
Conversation
| @@ -25,11 +25,15 @@ assert.doesNotThrow(function() { | |||
| var custom_inspect = { foo: 'bar', inspect: function() { return 'inspect'; } }; | |||
|
|
|||
| var stdout_write = global.process.stdout.write; | |||
| var stderr_write = global.process.stderr.write; | |||
|
LGTM. CI is green. |
| @@ -76,17 +102,33 @@ assert.equal('foo bar\n', strings.shift()); | |||
| assert.equal('foo bar hop\n', strings.shift()); | |||
There was a problem hiding this comment.
This section is a bit of a mess in that it's not very readable and mixes the strings and errStrings checks up. This can likely be simplified by creating a secondary array with the expected strings in the right order then just doing a for-loop through each of strings and errStrings to check each one. The code would be much easier to read and there'd be much less of it :-)
There was a problem hiding this comment.
Sure, the first bunch of these can work that way since they're just testing equality. I'll do that.
For the ones doing indexOf, regex or other more complex tests, I think they're going to have to be on their own still. Some comments might be able to clarify them though.
|
LGTM. CI again: https://ci.nodejs.org/job/node-test-pull-request/2487/ |
|
hmmm looks like linting failed? |
|
LGTM |
Just copied the basic tests for log, as they're all the same thing as log in either stdout or stderr. Cleaned that up a bit. Also const-ified.
|
@evanlucas i fixed the lint |
|
Thanks....let's try the CI one more time https://ci.nodejs.org/job/node-test-pull-request/2501/ |
|
@evanlucas looks like a single failure, for unrelated jenkinsey reasons https://ci.nodejs.org/job/node-test-commit-arm/nodes=armv8-ubuntu1404/3137/console |
|
Last CI looks good (https://ci.nodejs.org/job/node-test-pull-request/2520/). Landing now. Thanks! |
Just copied the basic tests for log, as they're all the same thing as log in either stdout or stderr. Cleaned that up a bit. Also const-ified. PR-URL: #6538 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Just copied the basic tests for log, as they're all the same thing as log in either stdout or stderr. Cleaned that up a bit. Also const-ified. PR-URL: #6538 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Checklist
Affected core subsystem(s)
test
Description of change
Add tests for
console.[info|error|warn].Just copied the basic tests for
log, as they're all the same thingas
login eitherstdoutorstderr.