★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/24479Nouvelle recherche | Portail wanayoo
Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
wants to merge 4 commits into from
Closed

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

wants to merge 4 commits into from

Conversation

sagirk
Copy link
Member

@sagirk sagirk commented Nov 19, 2018

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

@gireeshpunathil
Copy link
Member

@gireeshpunathil gireeshpunathil commented Nov 19, 2018

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

@sagirk
Copy link
Member Author

@sagirk sagirk commented Nov 20, 2018

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

image

@sagirk
Copy link
Member Author

@sagirk sagirk commented Nov 21, 2018 •
edited

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 sagirk force-pushed the refactor/test-cluster-send-deadlock branch from f46d511 to ec0646d Compare Nov 21, 2018
@sagirk
Copy link
Member Author

@sagirk sagirk commented Nov 21, 2018

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.

sagirk added 4 commits Nov 22, 2018
@sagirk
test: refactor test-cluster-send-deadlock to use arrow functions
acb4e1d 
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.
@sagirk
test: wrap functions containing assertions inside a common.mustCall
0d08ce2
@sagirk
test: remove common.mustCall wrapper over exit event handler
28a3b93
@sagirk
Revert "test: wrap functions containing assertions inside a `common.m…
fed582b 
…ustCall`"

This reverts commit 6eab88a71332e79825cd719dbc907564a2bfaec5.
@sagirk sagirk force-pushed the refactor/test-cluster-send-deadlock branch from d841dee to fed582b Compare Nov 22, 2018
@sagirk
Copy link
Member Author

@sagirk sagirk commented Nov 22, 2018

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

@gireeshpunathil
Copy link
Member

@gireeshpunathil gireeshpunathil commented Nov 22, 2018

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

@gireeshpunathil
Copy link
Member

@gireeshpunathil gireeshpunathil commented Nov 22, 2018

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

@gireeshpunathil
Copy link
Member

@gireeshpunathil gireeshpunathil commented Nov 22, 2018

landed as a67b22a , thanks!

pull bot pushed a commit to shakir-abdo/node that referenced this issue Nov 22, 2018
@sagirk @gireeshpunathil
test: refactor test to use arrow functions
a67b22a 
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>
@sagirk sagirk deleted the refactor/test-cluster-send-deadlock branch Nov 23, 2018
targos added a commit that referenced this issue Nov 24, 2018
@sagirk @targos
test: refactor test to use arrow functions
b355997 
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>
rvagg added a commit that referenced this issue Nov 28, 2018
@sagirk @rvagg
test: refactor test to use arrow functions
46b5df0 
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>
@BridgeAR BridgeAR mentioned this pull request Dec 5, 2018
Merged
4 tasks
@beevelop beevelop mentioned this pull request Dec 7, 2018
Closed
@beevelop beevelop mentioned this pull request Dec 7, 2018
Closed
@beevelop beevelop mentioned this pull request Dec 7, 2018
Closed
@beevelop beevelop mentioned this pull request Dec 8, 2018
Closed
@beevelop beevelop mentioned this pull request Dec 8, 2018
Closed
codebytere added a commit that referenced this issue Jan 13, 2019
@sagirk @codebytere
test: refactor test to use arrow functions
0d11e83 
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>
refack added a commit to refack/node that referenced this issue Jan 14, 2019
@sagirk @refack
test: refactor test to use arrow functions
4a82a56 
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>
@codebytere codebytere mentioned this pull request Jan 15, 2019
Merged
codebytere added a commit that referenced this issue Jan 29, 2019
@sagirk @codebytere
test: refactor test to use arrow functions
9ccc878 
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>
codebytere added a commit that referenced this issue Jan 29, 2019
@sagirk @codebytere
test: refactor test to use arrow functions
952842b 
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>
@beevelop beevelop mentioned this pull request Jan 29, 2019
Closed
@beevelop beevelop mentioned this pull request Jan 29, 2019
Closed
@beevelop beevelop mentioned this pull request Jan 29, 2019
Closed
@beevelop beevelop mentioned this pull request Jan 29, 2019
Closed
@beevelop beevelop mentioned this pull request Jan 29, 2019
Closed
@beevelop beevelop mentioned this pull request Jan 29, 2019
Closed
@beevelop beevelop mentioned this pull request Jan 30, 2019
Closed
@beevelop beevelop mentioned this pull request Jan 30, 2019
Closed
@beevelop beevelop mentioned this pull request Jan 30, 2019
Closed
@beevelop beevelop mentioned this pull request Jan 30, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell

@gireeshpunathil gireeshpunathil

Assignees
No one assigned
Labels
author ready code-and-learn test
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet
4 participants
@sagirk @gireeshpunathil @jasnell @nodejs-github-bot