Skip to content

feat(git-node): add git node benchmark#1114

Open
aduh95 wants to merge 3 commits into
nodejs:mainfrom
aduh95:git-node-benchmark
Open

feat(git-node): add git node benchmark#1114
aduh95 wants to merge 3 commits into
nodejs:mainfrom
aduh95:git-node-benchmark

Conversation

@aduh95

@aduh95 aduh95 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Add a tool to help spawning GHA benchmark from the CLI. Here's the output from my test run:

$ git node benchmark https://github.com/aduh95/node/pull/43/commits/b511522444cd573cbc17ab50019edb29410f357a
✔  Fetched data for https://github.com/aduh95/node/pull/43
✔  Found 53 benchmark categories
✔  Found 0 benchmark file(s) in the PR changes
   ℹ  Based on the PR labels, this/these benchmark(s) seem relevant:
   ℹ  zlib
   ℹ  If not relevant, do not forget to unselect it/them.
✔ Select the benchmark categories to run: domain
--------------------------------------------------------------------------------
✔ Substring to filter which benchmarks run (leave empty to run all): 
--------------------------------------------------------------------------------
✔ Should the workflow post the results as a comment on the PR? Yes
---------------------------------- Benchmark -----------------------------------
PR:        https://github.com/aduh95/node/pull/43
Commit:    b511522444cd573cbc17ab50019edb29410f357a
Categories:domain
Comment on PR:yes
Workflow:  aduh95/node (benchmark.yml@main)
--------------------------------------------------------------------------------
✔ Trigger the benchmark workflow with these settings? Yes
✔  Benchmark workflow dispatched

N.B.: the tests have been vibe-coded, part of the implementation has also been LLM-generated

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.51515% with 93 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.06%. Comparing base (a22e127) to head (d274ef4).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
components/git/benchmark.js 67.85% 36 Missing ⚠️
lib/request.js 14.63% 35 Missing ⚠️
lib/benchmark.js 90.94% 22 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1114      +/-   ##
==========================================
+ Coverage   62.43%   63.06%   +0.63%     
==========================================
  Files          48       50       +2     
  Lines        7882     8288     +406     
==========================================
+ Hits         4921     5227     +306     
- Misses       2961     3061     +100     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread lib/benchmark.js
Comment on lines +184 to +186
const postComment = await cli.prompt(
'Should the workflow post the results as a comment on the PR?',
{ defaultAnswer: sameRepo });

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This assumes that nodejs/node#64395 will land, I would need to remove it if it doesn't

@richardlau

Copy link
Copy Markdown
Member

In the last few days we have added a GH workflow for benchmark (which this PR is using) but also one for stress runs.

Since these are not Jenkins maybe these don't belong to ncu-ci, but do we want to group all potential GH workflows into a group of commands? Or do we not care about the Jenkins/GH distinction and put these all under ncu-ci?

I'm not saying we need to add the other workflows now, just asking/thinking about whether we need a pattern of where this (and potential other) commands go.

@aduh95

aduh95 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

I agree it would feel weird on ncu-ci. It's unclear to me whether you're suggesting a new binary, or a new git-node component with a more "neutral" name so it can be used for both (and potentially more in the future) workflows. It seems to me we would be better off having them both on git-node on different components (e.g. git node benchmark and git node stress-test), and they can both use the same Request.prototype.dispatchWorkflow, wdyt?

@richardlau

Copy link
Copy Markdown
Member

or a new git-node component with a more "neutral" name so it can be used for both (and potentially more in the future) workflows.

This is what I was thinking of, but it's more of a suggestion and not a blocking issue for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants