Uh oh!
There was an error while loading. Please reload this page.
test: add tests for console.[info|error|warn] - #6538
Conversation
evanlucas
commented
May 3, 2016
cjihrig
commented
May 3, 2016
LGTM. CI is green. |
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.
evanlucas
commented
May 3, 2016
LGTM. CI again: https://ci.nodejs.org/job/node-test-pull-request/2487/ |
evanlucas
commented
May 3, 2016
hmmm looks like linting failed? |
jasnell
commented
May 3, 2016
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.
bengl
commented
May 4, 2016
@evanlucas i fixed the lint |
evanlucas
commented
May 4, 2016
Thanks....let's try the CI one more time https://ci.nodejs.org/job/node-test-pull-request/2501/ |
bengl
commented
May 6, 2016
@evanlucas looks like a single failure, for unrelated jenkinsey reasons https://ci.nodejs.org/job/node-test-commit-arm/nodes=armv8-ubuntu1404/3137/console |
evanlucas
commented
May 6, 2016
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>
evanlucas
commented
May 6, 2016
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.