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.

Sign up for GitHub

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

[v10.x backport] test: move tick.js from test/async-hooks to test/common #23780

Closed
wants to merge 3 commits into from
Closed

[v10.x backport] test: move tick.js from test/async-hooks to test/common #23780

wants to merge 3 commits into from

Conversation

arur
Copy link
Contributor

@arur arur commented Oct 20, 2018

Backport: #23551

cc: @addaleax

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. v10.x labels Oct 20, 2018
addaleax
addaleax approved these changes Oct 20, 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.

Thanks!

@addaleax
Copy link
Member

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

@arur arur changed the title [v10.x-staging] test: move tick.js from test/async-hooks to test/common [v10.x backport] test: move tick.js from test/async-hooks to test/common Oct 20, 2018
@MylesBorins
Copy link
Contributor

@arur would you be able to rebase this against v10.x-staging please

@arur
Copy link
Contributor Author

arur commented Nov 5, 2018

Hey @MylesBorins, I might need a bit guidance on this. If my understanding is correct here are the steps I need to perform (being on local branch arur:v10.x-staging ):

$ git fetch upstream
$ git checkout backport-23551-v10.x
$ git rebase origin/v10.x-staging

I guess I may have conflicts here, so resolve, run tests

$ git push --force

Please let me know if it is the right way to do. Also, one of the my commits did not pass CI. Does it mean I need to address it somehow? Thanks!

@MylesBorins
Copy link
Contributor

@arur that looks mostly right although you likely want to rebase against upstream/v10.x-staging

@Trott Trott force-pushed the backport-23551-v10.x branch from d978183 to 6f77b5c Compare November 20, 2018 02:45
@Trott
Copy link
Member

Trott commented Nov 20, 2018 •
edited
Loading

@arur that looks mostly right although you likely want to rebase against upstream/v10.x-staging

@MylesBorins I rebased it and pushed up to @arur's branch. Is this landable? Still seems to be a stray commit in there to me, but I could be mistaken about it being stray?

@MylesBorins
Copy link
Contributor

MylesBorins commented Nov 20, 2018 •
edited
Loading

it seems like ffb14e8 had been removed from staging by @addaleax in #23795

maybe they can chime in as to why

@addaleax
Copy link
Member

@MylesBorins I don’t remember the details, but I think there were test failures from that commit?

@MylesBorins
Copy link
Contributor

Another go at CI: https://ci.nodejs.org/job/node-test-pull-request/18966/

@MylesBorins MylesBorins force-pushed the v10.x-staging branch 2 times, most recently from 4864cb0 to 4035ca1 Compare December 3, 2018 18:33
@codebytere codebytere force-pushed the v10.x-staging branch 3 times, most recently from 2d6e145 to 7840f71 Compare January 29, 2019 18:12
arur added 3 commits May 16, 2019 00:05
@arur @MylesBorins
test: move tick.js from test/async-hooks to test/common
5d86e63 
PR-URL: nodejs#23551
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@arur @MylesBorins
doc: add "tick" function name and argument description
bc53d17 
PR-URL: nodejs#23551
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@arur @MylesBorins
test: fix require path to tick module
afeeb14
@MylesBorins MylesBorins force-pushed the backport-23551-v10.x branch from 6f77b5c to afeeb14 Compare May 16, 2019 04:08
@MylesBorins
Copy link
Contributor

rebased and kicked off CI.

@nodejs-github-bot
Copy link
Collaborator

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

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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@MylesBorins
Copy link
Contributor

MylesBorins commented May 16, 2019 •
edited
Loading

landed in bc4d717...915ea2a

test failure unrelated

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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@addaleax addaleax addaleax approved these changes

Assignees
No one assigned
Labels
async_hooks Issues and PRs related to the async hooks 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.
5 participants
@arur @addaleax @MylesBorins @Trott @nodejs-github-bot