test: refactor test-cluster-send-deadlock to use arrow functions by sagirk · Pull Request #24479 · nodejs/node

sagirk · 2018-11-19T09:32:46Z

In test/parallel/test-cluster-send-deadlock.js, callbacks use
anonymous closure functions. It is safe to replace them with arrow
functions since these callbacks don't contain references to this,
super or arguments. This results in shorter functions.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

test/parallel/test-cluster-send-deadlock.js

gireeshpunathil · 2018-11-19T11:31:54Z

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

test/parallel/test-cluster-send-deadlock.js

sagirk · 2018-11-20T10:20:37Z

@thefourtheye Build failure — common.mustCall seems to be the culprit here as well.

sagirk · 2018-11-21T08:17:27Z

Like @Trott pointed out, the build failure here too is caused because of wrapping an exit event handler within common.mustCall.

Update: had to remove the common.mustCall wrapper from both change-sites to get the build to pass.

sagirk · 2018-11-21T16:02:55Z

Build failed because the GitHub API rate limit was exceeded! 🙀

$ if [ "${TRAVIS_PULL_REQUEST}" != "false" ]; then bash -x tools/lint-pr-commit-message.sh ${TRAVIS_PULL_REQUEST}; fi
+GH_API_URL=https://api.github.com
+PR_ID=24479
+'[' -z 24479 ']'
+'[' -z 24479 ']'
++curl -s https://api.github.com/repos/nodejs/node/pulls/24479/commits
+PR_COMMITS='{
  "message": "API rate limit exceeded for 104.154.255.220. (But here'\''s the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)",
  "documentation_url": "https://developer.github.com/v3/#rate-limiting"
}'
++node -p 'JSON.parse(process.argv[1])[0].url' '{
  "message": "API rate limit exceeded for 104.154.255.220. (But here'\''s the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)",
  "documentation_url": "https://developer.github.com/v3/#rate-limiting"
}'
+FIRST_COMMIT=
+echo 'Unable to determine the first commit for pull request 24479.'
Unable to determine the first commit for pull request 24479.
+exit 1
The command "if [ "${TRAVIS_PULL_REQUEST}" != "false" ]; then bash -x tools/lint-pr-commit-message.sh ${TRAVIS_PULL_REQUEST}; fi" exited with 1.

In `test/parallel/test-cluster-send-deadlock.js`, callbacks use anonymous closure functions. It is safe to replace them with arrow functions since these callbacks don't contain references to `this`, `super` or `arguments`. This results in shorter functions.

…ustCall`" This reverts commit 6eab88a71332e79825cd719dbc907564a2bfaec5.

sagirk · 2018-11-22T03:58:27Z

Rebased against master and force-pushed, triggering a new PR Build. Passed. PTAL.

gireeshpunathil · 2018-11-22T04:21:30Z

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

gireeshpunathil · 2018-11-22T08:27:07Z

all good; just it needs to grow 72 hours old.

gireeshpunathil · 2018-11-22T09:19:13Z

landed as a67b22a , thanks!

In `test/parallel/test-cluster-send-deadlock.js`, callbacks use anonymous closure functions. It is safe to replace them with arrow functions since these callbacks don't contain references to `this`, `super` or `arguments`. This results in shorter functions. PR-URL: nodejs#24479 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>

In `test/parallel/test-cluster-send-deadlock.js`, callbacks use anonymous closure functions. It is safe to replace them with arrow functions since these callbacks don't contain references to `this`, `super` or `arguments`. This results in shorter functions. PR-URL: #24479 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>

In `test/parallel/test-cluster-send-deadlock.js`, callbacks use anonymous closure functions. It is safe to replace them with arrow functions since these callbacks don't contain references to `this`, `super` or `arguments`. This results in shorter functions. PR-URL: nodejs#24479 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>

In `test/parallel/test-cluster-send-deadlock.js`, callbacks use anonymous closure functions. It is safe to replace them with arrow functions since these callbacks don't contain references to `this`, `super` or `arguments`. This results in shorter functions. PR-URL: #24479 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>

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

gireeshpunathil added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Nov 19, 2018

gireeshpunathil reviewed Nov 19, 2018

View reviewed changes

test/parallel/test-cluster-send-deadlock.js Outdated Show resolved Hide resolved

gireeshpunathil approved these changes Nov 19, 2018

View reviewed changes

sagirk commented Nov 20, 2018

View reviewed changes

test/parallel/test-cluster-send-deadlock.js Outdated Show resolved Hide resolved

sagirk force-pushed the refactor/test-cluster-send-deadlock branch from f46d511 to ec0646d Compare November 21, 2018 08:18

jasnell approved these changes Nov 21, 2018

View reviewed changes

sagirk added 4 commits November 22, 2018 08:08

test: wrap functions containing assertions inside a common.mustCall

0d08ce2

test: remove common.mustCall wrapper over exit event handler

28a3b93

Revert "test: wrap functions containing assertions inside a `common.m…

fed582b

…ustCall`" This reverts commit 6eab88a71332e79825cd719dbc907564a2bfaec5.

sagirk force-pushed the refactor/test-cluster-send-deadlock branch from d841dee to fed582b Compare November 22, 2018 02:38

gireeshpunathil added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 22, 2018

gireeshpunathil closed this Nov 22, 2018

sagirk deleted the refactor/test-cluster-send-deadlock branch November 23, 2018 17:40

BridgeAR mentioned this pull request Dec 5, 2018

v11.4.0 proposal #24854

Merged

4 tasks

This was referenced Dec 8, 2018

2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#264

Closed

2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#265

Closed

codebytere mentioned this pull request Jan 15, 2019

v10.15.1 proposal #25346

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

test: refactor test-cluster-send-deadlock to use arrow functions#24479

test: refactor test-cluster-send-deadlock to use arrow functions#24479
sagirk wants to merge 4 commits intonodejs:masterfrom
sagirk:refactor/test-cluster-send-deadlock

sagirk commented Nov 19, 2018

Uh oh!

Uh oh!

gireeshpunathil commented Nov 19, 2018

Uh oh!

Uh oh!

sagirk commented Nov 20, 2018

Uh oh!

sagirk commented Nov 21, 2018 •

edited

Loading

Uh oh!

sagirk commented Nov 21, 2018

Uh oh!

sagirk commented Nov 22, 2018

Uh oh!

gireeshpunathil commented Nov 22, 2018

Uh oh!

gireeshpunathil commented Nov 22, 2018

Uh oh!

gireeshpunathil commented Nov 22, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

sagirk commented Nov 19, 2018

Checklist

Uh oh!

Uh oh!

gireeshpunathil commented Nov 19, 2018

Uh oh!

Uh oh!

sagirk commented Nov 20, 2018

Uh oh!

sagirk commented Nov 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sagirk commented Nov 21, 2018

Uh oh!

sagirk commented Nov 22, 2018

Uh oh!

gireeshpunathil commented Nov 22, 2018

Uh oh!

gireeshpunathil commented Nov 22, 2018

Uh oh!

gireeshpunathil commented Nov 22, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sagirk commented Nov 21, 2018 •

edited

Loading