test: replace assert.equal with assert.strictEqual #9842

brad-decker · 2016-11-29T19:09:38Z

Checklist

make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
commit message follows commit guidelines

Affected core subsystem(s)

test

Description of change

Using NodeTodo I learned of a need to swap out the .equal function
with .strictEqual in a few test files.
https://twitter.com/NodeTodo/status/803657321993961472
https://gist.github.com/Trott/864401455d4afa2428cd4814e072bd7c

This pull request simply applies those suggested fixes.

Trott · 2016-11-29T20:13:18Z

LGTM if CI passes.

Nit: Can you update the first line of the commit message to comply with https://github.com/nodejs/node/blob/master/CONTRIBUTING.md#step-3-commit? The first line should probably be something like: test: replace assert.equal with assert.strictEqual

Trott · 2016-11-29T20:13:48Z

CI: https://ci.nodejs.org/job/node-test-pull-request/5025/

brad-decker · 2016-11-29T20:16:46Z

Amended the commit message. Thanks!

Trott · 2016-11-29T20:41:58Z

CI is green. I approved the PR in the GitHub interface. With a tiny number of exceptions, we leave PRs open for at least 48 hours (72 hours on weekends) to make sure people have an opportunity to comment etc. So if nobody expresses any objections, this can land Thursday afternoon Austin time.

brad-decker · 2016-11-29T20:52:16Z

Awesome! Thanks for the introduction and guidance at NodeInteractive / NodeTodo. 👍

gibfahn

LGTM, very good to see CI completely green

lpinca · 2016-11-30T17:31:13Z

test/addons/async-hello-world/test.js

@@ -4,7 +4,7 @@ var assert = require('assert');
 const binding = require(`./build/${common.buildType}/binding`);

 binding(5, common.mustCall(function(err, val) {
-  assert.equal(null, err);
-  assert.equal(10, val);
+  assert.strictEqual(null, err);


Nit: can you please swap the arguments? the first one is actual and the second one expected.

Yeah, sure thing.

@lpinca Swapped this instance and a couple others that were in expected, actual order instead of the desired actual, expected signature. Then rebased and squashed changes into one commit.

@brad-decker perfect, thanks!

Using NodeTodo I learned of a need to swap out the .equal function with .strictEqual in a few test files. https://twitter.com/NodeTodo/status/803657321993961472 https://gist.github.com/Trott/864401455d4afa2428cd4814e072bd7c additional commits squashed: .strictEqual's argument signature is actual, expected, [message]. Previously some statements were listed as expected, actual. As asked in PR i swapped them to match the correct argument signature.

lpinca · 2016-11-30T17:44:16Z

New CI: https://ci.nodejs.org/job/node-test-pull-request/5033/

mhdawson

LGTM

Trott · 2016-12-01T19:52:27Z

Landed in 18016d3 🎉

Using NodeTodo I learned of a need to swap out the .equal function with .strictEqual in a few test files. https://twitter.com/NodeTodo/status/803657321993961472 https://gist.github.com/Trott/864401455d4afa2428cd4814e072bd7c additional commits squashed: .strictEqual's argument signature is actual, expected, [message]. Previously some statements were listed as expected, actual. As asked in PR i swapped them to match the correct argument signature. PR-URL: #9842 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>

Using NodeTodo I learned of a need to swap out the .equal function with .strictEqual in a few test files. https://twitter.com/NodeTodo/status/803657321993961472 https://gist.github.com/Trott/864401455d4afa2428cd4814e072bd7c additional commits squashed: .strictEqual's argument signature is actual, expected, [message]. Previously some statements were listed as expected, actual. As asked in PR i swapped them to match the correct argument signature. PR-URL: nodejs#9842 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>

Using NodeTodo I learned of a need to swap out the .equal function with .strictEqual in a few test files. https://twitter.com/NodeTodo/status/803657321993961472 https://gist.github.com/Trott/864401455d4afa2428cd4814e072bd7c additional commits squashed: .strictEqual's argument signature is actual, expected, [message]. Previously some statements were listed as expected, actual. As asked in PR i swapped them to match the correct argument signature. PR-URL: #9842 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>

nodejs-github-bot added the test Issues and PRs related to the tests. label Nov 29, 2016

brad-decker changed the title ~~replaces assert.equal with asser.strictEqual in some tests~~ test: replaces assert.equal with asser.strictEqual Nov 29, 2016

brad-decker force-pushed the strictequals branch from 94b3a81 to 46b1704 Compare November 29, 2016 20:15

brad-decker changed the title ~~test: replaces assert.equal with asser.strictEqual~~ test: replace assert.equal with assert.strictEqual Nov 29, 2016

mscdex added the addons Issues and PRs related to native addons. label Nov 29, 2016

Trott approved these changes Nov 29, 2016

View reviewed changes

gibfahn approved these changes Nov 29, 2016

View reviewed changes

lpinca reviewed Nov 30, 2016

View reviewed changes

lpinca approved these changes Nov 30, 2016

View reviewed changes

brad-decker force-pushed the strictequals branch from 46b1704 to 530a0c3 Compare November 30, 2016 17:40

mhdawson approved these changes Nov 30, 2016

View reviewed changes

cjihrig approved these changes Dec 1, 2016

View reviewed changes

targos approved these changes Dec 1, 2016

View reviewed changes

Trott closed this Dec 1, 2016

Fishrock123 mentioned this pull request Dec 5, 2016

v7.2.1 proposal #10127

Merged

2 tasks

addaleax added the land-on-v6.x label Dec 8, 2016

MylesBorins added the dont-land-on-v4.x label Dec 20, 2016

MylesBorins mentioned this pull request Dec 21, 2016

V6.9.3 proposal #10394

Merged

snyk-bot mentioned this pull request Apr 24, 2023

[Snyk] Security upgrade eslint from 4.19.1 to 5.0.0 aliscco/alisco-node#97

Open

aliscco mentioned this pull request Apr 24, 2023

[Snyk] Fix for 19 vulnerabilities aliscco/alisco-node#109

Open

This was referenced Apr 24, 2023

[Snyk] Fix for 8 vulnerabilities aliscco/alisco-node#122

Open

[Snyk] Fix for 17 vulnerabilities aliscco/alisco-node#127

Open

aliscco mentioned this pull request Apr 24, 2023

[Snyk] Fix for 19 vulnerabilities aliscco/alisco-node#128

Open

This was referenced Apr 25, 2023

[Snyk] Fix for 8 vulnerabilities aliscco/alisco-node#177

Open

[Snyk] Fix for 7 vulnerabilities aliscco/alisco-node#182

Open

[Snyk] Fix for 8 vulnerabilities aliscco/alisco-node#183

Open

aliscco mentioned this pull request Apr 26, 2023

[Snyk] Fix for 2 vulnerabilities aliscco/alisco-node#195

Open

snyk-bot mentioned this pull request Apr 26, 2023

[Snyk] Security upgrade eslint from 4.19.1 to 5.0.0 aliscco/alisco-node#249

Open

aliscco mentioned this pull request Apr 28, 2023

[Snyk] Fix for 10 vulnerabilities aliscco/alisco-node#309

Open

snyk-bot mentioned this pull request Apr 28, 2023

[Snyk] Security upgrade eslint from 4.19.1 to 5.0.0 aliscco/alisco-node#336

Open

aliscco mentioned this pull request Apr 29, 2023

[Snyk] Fix for 7 vulnerabilities aliscco/alisco-node#356

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: replace assert.equal with assert.strictEqual #9842

test: replace assert.equal with assert.strictEqual #9842

brad-decker commented Nov 29, 2016 •

edited

Loading

Trott commented Nov 29, 2016

Trott commented Nov 29, 2016

brad-decker commented Nov 29, 2016

Trott commented Nov 29, 2016

brad-decker commented Nov 29, 2016

gibfahn left a comment

lpinca Nov 30, 2016

brad-decker Nov 30, 2016

brad-decker Nov 30, 2016

lpinca Nov 30, 2016

lpinca commented Nov 30, 2016

mhdawson left a comment

Trott commented Dec 1, 2016

test: replace assert.equal with assert.strictEqual #9842

test: replace assert.equal with assert.strictEqual #9842

Conversation

brad-decker commented Nov 29, 2016 • edited Loading

Checklist

Affected core subsystem(s)

Description of change

Trott commented Nov 29, 2016

Trott commented Nov 29, 2016

brad-decker commented Nov 29, 2016

Trott commented Nov 29, 2016

brad-decker commented Nov 29, 2016

gibfahn left a comment

Choose a reason for hiding this comment

lpinca Nov 30, 2016

Choose a reason for hiding this comment

brad-decker Nov 30, 2016

Choose a reason for hiding this comment

brad-decker Nov 30, 2016

Choose a reason for hiding this comment

lpinca Nov 30, 2016

Choose a reason for hiding this comment

lpinca commented Nov 30, 2016

mhdawson left a comment

Choose a reason for hiding this comment

Trott commented Dec 1, 2016

brad-decker commented Nov 29, 2016 •

edited

Loading