Skip to content

Conversation

AdamMajer
Copy link
Contributor

In previous version of this fix, I've simply added a check if the tested tool is available or not. Unfortuntelly, this fails when only the first tool is to be run as part of the test-benchmark-misc, and it doesn't exist.

benchmark/test-benchmark-misc
...
AssertionError [ERR_ASSERTION]: benchmark file not running exactly one configuration in test:
...
misc/startup-cli-version.js
...

The solution is to move the tool that is not present in a tarball down the list.

Fixes: #51146
Refs: #50684

@nodejs-github-bot nodejs-github-bot added the benchmark Issues and PRs related to the benchmark subsystem. label Feb 13, 2024
@AdamMajer AdamMajer force-pushed the benchmark-fixes branch 2 times, most recently from 396f5e4 to 893ac74 Compare February 13, 2024 12:44
In previous version of this fix, I've simply added a check if the tested
tool is available or not. Unfortuntelly, this fails when only the first
tool is to be run as part of the test-benchmark-misc, and it doesn't
exist.

benchmark/test-benchmark-misc
...
AssertionError [ERR_ASSERTION]: benchmark file not running exactly one
configuration in test:
...
misc/startup-cli-version.js
...

One solution is to check if the cli tool is actually available before
using it in a benchmark

Refs: nodejs#51146
@joyeecheung joyeecheung added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 1, 2024
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Mar 1, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/51746
✔  Done loading data for nodejs/node/pull/51746
----------------------------------- PR info ------------------------------------
Title      benchmark: move non-present deps down the list (#51746)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     AdamMajer:benchmark-fixes -> nodejs:main
Labels     benchmark
Commits    1
 - benchmark: move non-present deps down the list
Committers 1
 - Adam Majer 
PR-URL: https://github.com/nodejs/node/pull/51746
Fixes: https://github.com/nodejs/node/pull/51146
Refs: https://github.com/nodejs/node/pull/50684
Reviewed-By: Richard Lau 
Reviewed-By: Luigi Pinca 
Reviewed-By: Marco Ippolito 
Reviewed-By: Joyee Cheung 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/51746
Fixes: https://github.com/nodejs/node/pull/51146
Refs: https://github.com/nodejs/node/pull/50684
Reviewed-By: Richard Lau 
Reviewed-By: Luigi Pinca 
Reviewed-By: Marco Ippolito 
Reviewed-By: Joyee Cheung 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Tue, 13 Feb 2024 12:31:27 GMT
   ✔  Approvals: 4
   ✔  - Richard Lau (@richardlau) (TSC): https://github.com/nodejs/node/pull/51746#pullrequestreview-1877961858
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/51746#pullrequestreview-1881265306
   ✔  - Marco Ippolito (@marco-ippolito): https://github.com/nodejs/node/pull/51746#pullrequestreview-1909418797
   ✔  - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/51746#pullrequestreview-1909400852
   ✘  GitHub CI is still running
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/8113019460

@aduh95 aduh95 merged commit b1515c7 into nodejs:main May 11, 2024
@aduh95
Copy link
Contributor

aduh95 commented May 11, 2024

Landed in b1515c7

targos pushed a commit that referenced this pull request May 11, 2024
PR-URL: #51746
Refs: #51146
Refs: #50684
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
marco-ippolito pushed a commit that referenced this pull request Jun 17, 2024
PR-URL: #51746
Refs: #51146
Refs: #50684
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
marco-ippolito pushed a commit that referenced this pull request Jun 17, 2024
PR-URL: #51746
Refs: #51146
Refs: #50684
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
marco-ippolito pushed a commit that referenced this pull request Jun 17, 2024
PR-URL: #51746
Refs: #51146
Refs: #50684
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
eliphazbouye pushed a commit to eliphazbouye/node that referenced this pull request Jun 20, 2024
PR-URL: nodejs#51746
Refs: nodejs#51146
Refs: nodejs#50684
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
bmeck pushed a commit to bmeck/node that referenced this pull request Jun 22, 2024
PR-URL: nodejs#51746
Refs: nodejs#51146
Refs: nodejs#50684
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmark Issues and PRs related to the benchmark subsystem. commit-queue-failed An error occurred while landing this pull request using GitHub Actions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants