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

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

Closed

Conversation

sagirk
Copy link
Contributor

@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

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Nov 19, 2018
@gireeshpunathil 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
Copy link
Member

@sagirk
Copy link
Contributor Author

sagirk commented Nov 20, 2018

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

image

@sagirk
Copy link
Contributor Author

sagirk commented Nov 21, 2018

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 November 21, 2018 08:18
@sagirk
Copy link
Contributor Author

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.

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 sagirk force-pushed the refactor/test-cluster-send-deadlock branch from d841dee to fed582b Compare November 22, 2018 02:38
@sagirk
Copy link
Contributor Author

sagirk commented Nov 22, 2018

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

@gireeshpunathil
Copy link
Member

@gireeshpunathil
Copy link
Member

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

@gireeshpunathil 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
Copy link
Member

landed as a67b22a , thanks!

pull bot pushed a commit to shakir-abdo/node that referenced this pull request Nov 22, 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.

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 November 23, 2018 17:40
targos pushed a commit that referenced this pull request Nov 24, 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.

PR-URL: #24479
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
rvagg pushed a commit that referenced this pull request Nov 28, 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.

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
4 tasks
codebytere pushed a commit that referenced this pull request Jan 13, 2019
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 pushed a commit to refack/node that referenced this pull request Jan 14, 2019
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
codebytere pushed a commit that referenced this pull request Jan 29, 2019
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 pushed a commit that referenced this pull request Jan 29, 2019
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants