Skip to content
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: improve console coverage to check ignoreErrors #11531

Closed

Conversation

yoichi-watanabe
Copy link

@yoichi-watanabe yoichi-watanabe commented Feb 24, 2017

This PR can cover some uncovered lines on lib/console.js.
see:https://coverage.nodejs.org/coverage-29ff16f04373d434/root/console.js.html
I checked write function if ignoreErrors is false.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Feb 24, 2017
@mscdex mscdex added the console Issues and PRs related to the console subsystem. label Feb 24, 2017
@Trott
Copy link
Member

Trott commented Feb 24, 2017

@nodejs/testing

const { Writable } = require('stream');
const assert = require('assert');
const ignoreErrors = false;
const expected = 'foobarbaz';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you either scope this to the first test, or name it something else. This isn't actually "expected" output in the second test.

@cjihrig cjihrig mentioned this pull request Mar 2, 2017
3 tasks
});
const c = new Console(err, err, ignoreErrors);
c.log(expected);
}, /^Error: foobar$/);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our linter will fail as there is no newline at the end of the file.


assert.throws(() => {
const err = new Writable({
write: common.mustCall((chunk, enc, callback) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused parameters.

@jasnell
Copy link
Member

jasnell commented Mar 17, 2017

ping @chipstar :-)

@BridgeAR
Copy link
Member

Ping @yoichi-watanabe

@jasnell jasnell added the stalled Issues and PRs that are stalled. label Aug 29, 2017
@BridgeAR
Copy link
Member

Closing due to long inactivity. @yoichi-watanabe please feel free to leave a comment to open this PR again if you want to follow up on it or just open a new PR. I am sorry that your PR could not land as is and your work is much appreciated nevertheless!

@BridgeAR BridgeAR closed this Sep 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
console Issues and PRs related to the console subsystem. stalled Issues and PRs that are stalled. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants