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: move tick.js from test/async-hooks to test/common #23551

Closed
wants to merge 3 commits into from
Closed

test: move tick.js from test/async-hooks to test/common #23551

wants to merge 3 commits into from

Conversation

arur
Copy link
Contributor

@arur arur commented Oct 12, 2018

Move tick.js from test/async-hooks to test/common

tick.js provides a helper function that can be used to call a callback after a given number of event loop "ticks". Similar tick functions were used in couple of different tests as well. This commit moves tick.js from test/async-hooks/tick.js to test/common, updates corresponding imports, as well removes the dublicates.

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 async_hooks Issues and PRs related to the async hooks subsystem. test Issues and PRs related to the tests. labels Oct 12, 2018
@addaleax addaleax added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Oct 12, 2018
addaleax
addaleax approved these changes Oct 12, 2018
View reviewed changes
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Nice!

@mcollina
Copy link
Member

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

mcollina
mcollina approved these changes Oct 12, 2018
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM

vsemozhetbyt
vsemozhetbyt reviewed Oct 12, 2018
View reviewed changes
test/common/README.md Outdated Show resolved Hide resolved
lundibundi
lundibundi approved these changes Oct 12, 2018
View reviewed changes
Copy link
Member

@lundibundi lundibundi left a comment

Choose a reason for hiding this comment

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

👍

@Trott
Copy link
Member

Trott commented Oct 12, 2018

Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/17774/

@Trott
Copy link
Member

Trott commented Oct 12, 2018 •
edited

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

addaleax
addaleax approved these changes Oct 13, 2018
View reviewed changes
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Looks good to me!

lundibundi
lundibundi approved these changes Oct 13, 2018
View reviewed changes
Copy link
Member

@lundibundi lundibundi left a comment

Choose a reason for hiding this comment

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

LGTM with as nit.

test/common/README.md Show resolved Hide resolved
@Trott
Copy link
Member

Trott commented Oct 13, 2018

Resume Build: https://ci.nodejs.org/job/node-test-pull-request/17829/

@Trott
Copy link
Member

Trott commented Oct 14, 2018

Collaborators, 👍 to approve fast-tracking.

@Trott Trott added the fast-track PRs that do not need to wait for 48 hours to land. label Oct 14, 2018
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Oct 15, 2018
@arur @BridgeAR
test: move tick.js from test/async-hooks to test/common
52a1ef7 
PR-URL: nodejs#23551
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@danbev
Copy link
Contributor

danbev commented Oct 15, 2018

Landed in e096df3, and 2a1492a 🎉

@danbev danbev closed this Oct 15, 2018
danbev pushed a commit that referenced this pull request Oct 15, 2018
@arur @danbev
test: move tick.js from test/async-hooks to test/common
2a1492a 
PR-URL: #23551
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
danbev pushed a commit that referenced this pull request Oct 15, 2018
@arur @danbev
doc: add "tick" function name and argument description
e096df3 
PR-URL: #23551
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
jasnell pushed a commit that referenced this pull request Oct 17, 2018
@arur @jasnell
test: move tick.js from test/async-hooks to test/common
1471023 
PR-URL: #23551
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
jasnell pushed a commit that referenced this pull request Oct 17, 2018
@arur @jasnell
doc: add "tick" function name and argument description
3ad2267 
PR-URL: #23551
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@addaleax
Copy link
Member

@arur These patches land cleanly on v10.x, but it seem there are other tests that need a small fixup to work fully after this PR.

It would be great if this could be this be backported to the v10.x-staging branch here. If you want to do that yourself, please follow the guide and raise a backport PR – if you prefer for somebody else to take care of that, just let us know. :)

@arur arur mentioned this pull request Oct 20, 2018
Closed
4 tasks
MylesBorins pushed a commit to arur/node that referenced this pull request May 16, 2019
@arur @MylesBorins
test: move tick.js from test/async-hooks to test/common
5d86e63 
PR-URL: nodejs#23551
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
MylesBorins pushed a commit to arur/node that referenced this pull request May 16, 2019
@arur @MylesBorins
doc: add "tick" function name and argument description
bc53d17 
PR-URL: nodejs#23551
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
@arur @MylesBorins
test: move tick.js from test/async-hooks to test/common
f5c0b84 
Backport-PR-URL: #23780
PR-URL: #23551
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
@arur @MylesBorins
doc: add "tick" function name and argument description
bc4d717 
Backport-PR-URL: #23780
PR-URL: #23551
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
@arur @MylesBorins
test: move tick.js from test/async-hooks to test/common
4733a56 
Backport-PR-URL: #23780
PR-URL: #23551
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
@arur @MylesBorins
doc: add "tick" function name and argument description
2a98b9c 
Backport-PR-URL: #23780
PR-URL: #23551
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@BethGriggs BethGriggs mentioned this pull request May 16, 2019
Merged
This was referenced May 29, 2019
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

@lundibundi lundibundi lundibundi approved these changes

@vsemozhetbyt vsemozhetbyt vsemozhetbyt left review comments

@mcollina mcollina mcollina approved these changes

@jasnell jasnell jasnell approved these changes

@thefourtheye thefourtheye thefourtheye approved these changes

@addaleax addaleax addaleax approved these changes

@cjihrig cjihrig cjihrig approved these changes

@tniessen tniessen tniessen approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

@trivikr trivikr trivikr approved these changes

Assignees
No one assigned
Labels
async_hooks Issues and PRs related to the async hooks subsystem. code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. fast-track PRs that do not need to wait for 48 hours to land. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet