FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: change var declarations, add mustCall check by danielgsims · Pull Request #9962 · nodejs/node · GitHub

/ node Public

test: change var declarations, add mustCall check - #9962

Closed
danielgsims wants to merge 3 commits into
nodejs:masterfrom
danielgsims:test-cluster-net-send
ConversationCommits (3)ChecksFiles changed
Closed

test: change var declarations, add mustCall check#9962
danielgsims wants to merge 3 commits into
nodejs:masterfrom
danielgsims:test-cluster-net-send

Conversation

danielgsims commented Dec 1, 2016

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

test

Description of change

In this test, I changed the var declarations to be either a let or a
const. For some of the callbacks, I added a mustCall check to ensure
that the functions have run.

test: change var declarations, add mustCall check
3b7ece6 
In this test, I changed the var declarations to be either a let or a
const. For some of the callbacks, I added a mustCall check to ensure
that the functions have run.
nodejs-github-bot added the test Issues and PRs related to the tests. label Dec 1, 2016

danielgsims commented Dec 1, 2016

Copy link
Copy Markdown
Author

This is from the code and learn at NINA

imyller added code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. cluster Issues and PRs related to the cluster subsystem. labels Dec 1, 2016
mscdex added the net Issues and PRs related to the net subsystem. label Dec 1, 2016
julianduque reviewed Dec 1, 2016
View reviewed changes
Comment thread test/parallel/test-cluster-net-send.js Outdated
}));

process.once('exit', function() {
console.log('runs');

julianduque Dec 1, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can you please remove this console.log statement here?

danielgsims Dec 1, 2016

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

whoops, sure.

danielgsims commented Dec 1, 2016

Copy link
Copy Markdown
Author

@julianduque - all set

Trott commented Dec 4, 2016

Copy link
Copy Markdown
Member

Thanks for the contribution! If it's not too much trouble, can you switch the occurrences of assert.equal() in the test to assert.strictEqual()?

cjihrig requested changes Dec 5, 2016
View reviewed changes
Comment thread test/parallel/test-cluster-net-send.js Outdated

var server = net.createServer(function(c) {
process.once('message', function(msg) {
let server = net.createServer(function(c) {

cjihrig Dec 5, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can't this be const?

test: change let declaration, assertions
f139445 
I changed the let to a const declaration, and edited the equal
assertions to a strict assertion.

danielgsims commented Dec 6, 2016

Copy link
Copy Markdown
Author

Here you go, let me know if there's anything else.

Trott commented Dec 22, 2016

Copy link
Copy Markdown
Member

Ping @cjihrig: Looks like your comment has been addressed. If so, can you update your review?

Trott commented Dec 22, 2016

Copy link
Copy Markdown
Member

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

Trott pushed a commit to Trott/io.js that referenced this pull request Dec 23, 2016
test: change var declarations, add mustCall check
9fd79c9 
In this test, I changed the var declarations to be either a let or a
const. For some of the callbacks, I added a mustCall check to ensure
that the functions have run. I also changed assert.equal() to
assert.strictEqual().

PR-URL: nodejs#9962
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Trott commented Dec 23, 2016

Copy link
Copy Markdown
Member

Landed in 9fd79c9.
Thanks for the contribution! 🎉

If you or anyone else wants a good second commit, this can possibly be further refactored to get rid of the called variable and the process.on('exit', ...) handler by using common.mustCall().

Trott closed this Dec 23, 2016
evanlucas mentioned this pull request Jan 3, 2017
Merged
evanlucas pushed a commit that referenced this pull request Jan 3, 2017
test: change var declarations, add mustCall check
78376e6 
In this test, I changed the var declarations to be either a let or a
const. For some of the callbacks, I added a mustCall check to ensure
that the functions have run. I also changed assert.equal() to
assert.strictEqual().

PR-URL: #9962
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
evanlucas pushed a commit that referenced this pull request Jan 4, 2017
test: change var declarations, add mustCall check
bee7d7e 
In this test, I changed the var declarations to be either a let or a
const. For some of the callbacks, I added a mustCall check to ensure
that the functions have run. I also changed assert.equal() to
assert.strictEqual().

PR-URL: #9962
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 23, 2017
test: change var declarations, add mustCall check
8eb8c0f 
In this test, I changed the var declarations to be either a let or a
const. For some of the callbacks, I added a mustCall check to ensure
that the functions have run. I also changed assert.equal() to
assert.strictEqual().

PR-URL: #9962
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 23, 2017
test: change var declarations, add mustCall check
cd08da8 
In this test, I changed the var declarations to be either a let or a
const. For some of the callbacks, I added a mustCall check to ensure
that the functions have run. I also changed assert.equal() to
assert.strictEqual().

PR-URL: #9962
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
test: change var declarations, add mustCall check
4960a3e 
In this test, I changed the var declarations to be either a let or a
const. For some of the callbacks, I added a mustCall check to ensure
that the functions have run. I also changed assert.equal() to
assert.strictEqual().

PR-URL: #9962
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
test: change var declarations, add mustCall check
1c21166 
In this test, I changed the var declarations to be either a let or a
const. For some of the callbacks, I added a mustCall check to ensure
that the functions have run. I also changed assert.equal() to
assert.strictEqual().

PR-URL: #9962
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This was referenced Jan 24, 2017
Merged
Merged
MylesBorins pushed a commit that referenced this pull request Jan 31, 2017
test: change var declarations, add mustCall check
390cab8 
In this test, I changed the var declarations to be either a let or a
const. For some of the callbacks, I added a mustCall check to ensure
that the functions have run. I also changed assert.equal() to
assert.strictEqual().

PR-URL: #9962
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 1, 2017
test: change var declarations, add mustCall check
122917d 
In this test, I changed the var declarations to be either a let or a
const. For some of the callbacks, I added a mustCall check to ensure
that the functions have run. I also changed assert.equal() to
assert.strictEqual().

PR-URL: #9962
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
aliscco mentioned this pull request Jun 21, 2023
Open
This was referenced Jul 1, 2023
Open
Open
This was referenced Jul 8, 2023
Open
Open
leonardoadame mentioned this pull request Sep 30, 2023
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
cjihrig cjihrig approved these changes
+1 more reviewer
julianduque julianduque left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cluster Issues and PRs related to the cluster subsystem. code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. net Issues and PRs related to the net 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.

9 participants

Back | FazBrowse Home | New Git URL