Refactor test-readline-async-iterators into a benchmark #49237
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
Running the benchmark on my system produces following results: |
Collaborator
Contributor
Author
|
I see two tests failed. For commit message should I change commit and push or it is ok and will be changed while merging? |
35 tasks
21 tasks
Contributor
Author
|
Hi @mcollina, sorry you may be busy, just wanted to know if there are any changes needed from my side on this. |
27 tasks
Member
|
Hello, your first commit message has an incorrect prefix; it can be formatted as follows: test: refactor test-readline-async-iterators into a benchmark |
6a7950d to
429e78d
Compare
Contributor
Author
|
@mertcanaltin thanks, I have updated |
27 tasks
Contributor
Author
|
I am not sure why 3 of the tests are stuck in InProgress state. Can someone help? |
Member
Collaborator
Collaborator
8000
Contributor
Author
|
Logs from |
22 tasks
Member
|
@nodejs/build @nodejs/testing could somebody take a look at this? It keeps failing but there are less tests, not more tests, so they should not be. |
Collaborator
Collaborator
17 tasks
Member
I started a new CI -- the only failure was a temporary infra issue while we've been adding gcc 10 to the test machines (this particular failure fixed in nodejs/build#3495). One resume CI later and the Jenkins CI has passed for this PR. |
Collaborator
Commit Queue failed- Loading data for nodejs/node/pull/49237 ✔ Done loading data for nodejs/node/pull/49237 ----------------------------------- PR info ------------------------------------ Title Refactor test-readline-async-iterators into a benchmark (#49237) Author Shubham Pandey (@shubham9411) Branch shubham9411:shubham9411/readline-iterable-benchmark -> nodejs:main Labels readline, test, benchmark, author ready, commit-queue-squash Commits 2 - test: refactor test-readline-async-iterators into a benchmark - empty commit Committers 1 - Shubham Pandey PR-URL: https://github.com/nodejs/node/pull/49237 Fixes: https://github.com/nodejs/node/issues/49224 Reviewed-By: Matteo Collina ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/49237 Fixes: https://github.com/nodejs/node/issues/49224 Reviewed-By: Matteo Collina -------------------------------------------------------------------------------- ⚠ Commits were pushed since the last approving review: ⚠ - test: refactor test-readline-async-iterators into a benchmark ⚠ - empty commit ℹ This PR was created on Sat, 19 Aug 2023 05:18:29 GMT ✔ Approvals: 1 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/49237#pullrequestreview-1609041171 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2023-09-21T17:12:52Z: https://ci.nodejs.org/job/node-test-pull-request/54135/ ℹ Last Benchmark CI on 2023-09-07T09:20:49Z: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1365/ - Querying data for job/node-test-pull-request/54135/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/6274851210 |
16 tasks
Contributor
Author
|
Hey @mcollina, I pushed one empty commit just to trigger the CI pipeline. Do I have to remove it as commit queue is failing. Thanks |
Member
|
No need |
Collaborator
|
Landed in c0b4208 |
20 tasks
alexfernandez
pushed a commit
to alexfernandez/node
that referenced
this pull request
Nov 1, 2023
PR-URL: nodejs#49237 Fixes: nodejs#49224 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor test-readline-async-iterators into a benchmark
Fixes: #49224
Added old way of readline async iterator to benchmark.