Skip to content

Stop including API benchmarks in validate - #64337

Merged
Wesley Wigham (weswigham) merged 1 commit into
microsoft:mainfrom
weswigham:extract-benchmarks-from-tests
Sep 21, 2026
Merged

Wesley Wigham (weswigham) merged 1 commit into
microsoft:mainfrom
weswigham:extract-benchmarks-from-tests

Conversation

@weswigham

@weswigham Wesley Wigham (weswigham) commented Sep 19, 2026

Copy link
Copy Markdown
Member

They get their own test subtask executed in CI instead of being run directly in the main API tests, similar to how the go benchmarks already work.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The broader Go benchmark exclusion and conflicting contributor guidance need clarification or correction.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity · 1 Low severity

Open (2)
What changed in this PR

Separates API benchmark smoke runs from validation and executes them through dedicated tasks.

Changes:

  • Removes benchmarks from API tests.
  • Adds a standalone API benchmark task and CI step.
  • Excludes benchmark tasks from validate.
File Description
packages/​typescript/​test/​sync/​api.test.ts Removes sync benchmark execution.
packages/​typescript/​test/​sync/​api-generators.test.ts Removes generator benchmark execution.
packages/​typescript/​test/​async/​api.test.ts Removes async benchmark execution.
Herebyfile.mjs Adds API benchmark orchestration and changes validation behavior.
.github/​workflows/​ci.yml Runs API benchmarks as a dedicated CI step.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread Herebyfile.mjs
Comment thread Herebyfile.mjs
Comment thread Herebyfile.mjs

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I had the same thought last week. I agree that the benchmarks shouldn't add any actual coverage over the rest of the test suite.

@github-project-automation github-project-automation Bot moved this from Not started to Needs merge in PR Backlog Sep 21, 2026
Comment thread .github/workflows/ci.yml

@jakebailey Jake Bailey (jakebailey) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So long as CI still checks it, I think it's fine.

@weswigham
Wesley Wigham (weswigham) added this pull request to the merge queue Sep 21, 2026
Merged via the queue into microsoft:main with commit 4323071 Sep 21, 2026
26 checks passed
@weswigham
Wesley Wigham (weswigham) deleted the extract-benchmarks-from-tests branch September 21, 2026 16:11
@github-project-automation github-project-automation Bot moved this from Needs merge to Done in PR Backlog Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants