Skip to content

test: change var to const/let - change assert.equal to assert.strictE…#10018

Closed
aileenvl wants to merge 1 commit intonodejs:masterfrom
aileenvl:issue-test-crypto-cipheriv-decipheriv_aileen
Closed

test: change var to const/let - change assert.equal to assert.strictE…#10018
aileenvl wants to merge 1 commit intonodejs:masterfrom
aileenvl:issue-test-crypto-cipheriv-decipheriv_aileen

Conversation

@aileenvl
Copy link

@aileenvl aileenvl commented Dec 1, 2016 •
edited
Loading

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

test

Description of change

…qual

Make change in test file per issue received in NINA 2016 involving the change from var to const/let and the change of assert.equal to assert.strictEqual

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Dec 1, 2016
@imyller imyller added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Dec 1, 2016
@mscdex mscdex added the crypto Issues and PRs related to the crypto subsystem. label Dec 1, 2016
@mscdex
Copy link
Contributor

mscdex commented Dec 1, 2016

Please see the commit message guidelines here.

@Trott
Copy link
Member

Trott commented Dec 4, 2016

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

@Trott
Copy link
Member

Trott commented Dec 4, 2016

Thanks for opening a pull request. The make -j8 test box is checked, but this branch fails linting, so I don't think it was actually run.. Can you run it and fix any issues that arise?

@Trott
Copy link
Member

Trott commented Dec 4, 2016

Whoops! Ignore the previous comment. It looks like the lint failure in CI is an infrastructure glitch and not a lint problem. Sorry about that.

Trott
Trott reviewed Dec 4, 2016
View reviewed changes
test/parallel/test-crypto-cipheriv-decipheriv.js Outdated
Copy link
Member

@Trott Trott Dec 4, 2016

Choose a reason for hiding this comment

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

It seems like this could be const.

Trott
Trott reviewed Dec 4, 2016
View reviewed changes
test/parallel/test-crypto-cipheriv-decipheriv.js Outdated
Copy link
Member

@Trott Trott Dec 4, 2016

Choose a reason for hiding this comment

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

Also could be const, it would seem.

@Trott
Copy link
Member

Trott commented Dec 4, 2016

Actually, the lint failures were real after all. The assert.strictEqual() lines were longer than 80 chars. I'll fix that as I rebase to resolve a conflict with another PR that just landed. I'll also use const where let isn't necessary.

@Trott Trott force-pushed the issue-test-crypto-cipheriv-decipheriv_aileen branch from 6be766a to 8f46bcc Compare December 4, 2016 06:44
@Trott
Copy link
Member

Trott commented Dec 4, 2016

(Those let instances that could be const are also called out by the linter.)

@Trott
test: refactor test-crypto-cipheriv-decipheriv
0b8800d 
Make change in test file from var to const/let.
@Trott Trott force-pushed the issue-test-crypto-cipheriv-decipheriv_aileen branch from 8f46bcc to 0b8800d Compare December 4, 2016 06:49
@Trott
Copy link
Member

Trott commented Dec 4, 2016

Pushed changes. CI: https://ci.nodejs.org/job/node-test-pull-request/5169/

Trott
Trott approved these changes Dec 4, 2016
View reviewed changes
Copy link
Member

@Trott Trott 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 CI is ✅

@Trott
Copy link
Member

Trott commented Dec 4, 2016

CI is good except for one unrelated AIX build failure.

Trott pushed a commit to Trott/io.js that referenced this pull request Dec 4, 2016
@Trott
test: refactor test-crypto-cipheriv-decipheriv
07ef682 
Make change in test file from var to const/let.

PR-URL: nodejs#10018
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Trott
Copy link
Member

Trott commented Dec 4, 2016

Landed in 07ef682. Thanks for the contribution! 🎉

@Trott Trott closed this Dec 4, 2016
addaleax pushed a commit that referenced this pull request Dec 5, 2016
@addaleax
test: refactor test-crypto-cipheriv-decipheriv
3e37673 
Make change in test file from var to const/let.

PR-URL: #10018
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Fishrock123 Fishrock123 mentioned this pull request Dec 5, 2016
Merged
2 tasks
addaleax pushed a commit to addaleax/node that referenced this pull request Dec 8, 2016
@addaleax
test: refactor test-crypto-cipheriv-decipheriv
8954724 
Make change in test file from var to const/let.

PR-URL: nodejs#10018
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
jmdarling pushed a commit to jmdarling/node that referenced this pull request Dec 8, 2016
test: refactor test-crypto-cipheriv-decipheriv
3daa715 
Make change in test file from var to const/let.

PR-URL: nodejs#10018
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 20, 2016
@MylesBorins
test: refactor test-crypto-cipheriv-decipheriv
9473fc6 
Make change in test file from var to const/let.

PR-URL: #10018
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
@MylesBorins
test: refactor test-crypto-cipheriv-decipheriv
a7a8a4c 
Make change in test file from var to const/let.

PR-URL: #10018
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
@MylesBorins
test: refactor test-crypto-cipheriv-decipheriv
87dafd5 
Make change in test file from var to const/let.

PR-URL: #10018
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This was referenced Dec 21, 2016
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Trott Trott Trott approved these changes

@cjihrig cjihrig cjihrig approved these changes

Assignees

No one assigned

Labels

code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. crypto Issues and PRs related to the crypto subsystem. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@aileenvl @mscdex @Trott @cjihrig @MylesBorins @addaleax @imyller @nodejs-github-bot