Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

test: improve coverage of the util module #8633

Merged
merged 1 commit into from
Sep 20, 2016

Conversation

targos
Copy link
Member

@targos targos commented Sep 17, 2016

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

util

Description of change

Add tests for untested branches and statements.
Change assert.equal to assert.strictEqual for consistency

/cc @nodejs/testing

@targos targos added util Issues and PRs related to the built-in util module. test Issues and PRs related to the tests. labels Sep 17, 2016
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Sep 17, 2016
Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

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

LGTM if the CI is happy

@targos
Copy link
Member Author

targos commented Sep 17, 2016

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

LGTM

process.on('warning', common.mustCall((warning) => {
assert.strictEqual(warning.name, 'DeprecationWarning');
assert.notStrictEqual(expected.indexOf(warning.message), -1,
`unexpected error message: "${warning.message}"`);
Copy link
Member

Choose a reason for hiding this comment

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

nit: I think assert.ok + .includes() might be more intuitive than .indexOf() !== -1 here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am working on a refactoring of all tests for deprecation warnings, including this one. I'll include your proposal in it.

Copy link
Contributor

@evanlucas evanlucas left a comment

Choose a reason for hiding this comment

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

LGTM

@santigimeno
Copy link
Member

LGTM

Add tests for untested branches and statements.
Change assert.equal to assert.strictEqual for consistency.

PR-URL: nodejs#8633
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
@targos
Copy link
Member Author

targos commented Sep 20, 2016

Landed in ebbd5cb

@targos targos closed this Sep 20, 2016
@targos targos merged commit ebbd5cb into nodejs:master Sep 20, 2016
@targos targos deleted the util-test-coverage branch September 20, 2016 07:46
@MylesBorins
Copy link
Contributor

@targos should this be backported?

targos added a commit to targos/node that referenced this pull request Oct 7, 2016
Add tests for untested branches and statements.
Change assert.equal to assert.strictEqual for consistency.

PR-URL: nodejs#8633
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
@targos
Copy link
Member Author

targos commented Oct 7, 2016

Backport: #8970

MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
Add tests for untested branches and statements.
Change assert.equal to assert.strictEqual for consistency.

PR-URL: #8633
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
Add tests for untested branches and statements.
Change assert.equal to assert.strictEqual for consistency.

PR-URL: #8633
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Fishrock123 pushed a commit that referenced this pull request Oct 11, 2016
Add tests for untested branches and statements.
Change assert.equal to assert.strictEqual for consistency.

PR-URL: #8633
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
rvagg pushed a commit that referenced this pull request Oct 18, 2016
Add tests for untested branches and statements.
Change assert.equal to assert.strictEqual for consistency.

PR-URL: #8633
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
MylesBorins pushed a commit that referenced this pull request Oct 26, 2016
Add tests for untested branches and statements.
Change assert.equal to assert.strictEqual for consistency.

PR-URL: #8633
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
@MylesBorins MylesBorins mentioned this pull request Oct 26, 2016
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
test Issues and PRs related to the tests. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants