test: surface skipped and todo tests at the end of every run - #1587
Closed
quietbits wants to merge 2 commits into
Closed
test: surface skipped and todo tests at the end of every run#1587quietbits wants to merge 2 commits into
quietbits wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an advisory Vitest reporter that surfaces skipped and todo tests after each run.
Changes:
- Groups skipped/todo tests by file with notes and project labels.
- Flags entirely skipped files.
- Registers the reporter across Node, browser, e2e, and inherited guide tests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
config/skipped-tests-reporter.ts |
Implements skip/todo reporting. |
config/vitest.config.ts |
Registers the reporter for Node tests. |
config/vitest.config.browser.ts |
Registers it for browser tests. |
config/vitest.config.e2e.ts |
Registers it for e2e tests. |
Suppressed comments (1)
config/skipped-tests-reporter.ts:128
showProjectis derived only from projects that produced a skipped/todo record, rather than all projects active in the run. In a Chromium/Firefox run where only Firefox skips a test, this set has size 1 and the output omits[firefox], making the project-dependent skip ambiguous. Derive the active project set fromtestModulesinstead.
const projects = new Set(
[...skipped, ...todo].map((record) => record.project),
);
const showProject = projects.size > 1;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
config/skipped-tests-reporter.ts:165
- Please add automated coverage for this reporter. Its core behavior (todo-vs-skip classification, whole-file detection across projects, project labels,
ctx.skip()notes, and suppressing output when nothing is skipped) is currently untested, so a Vitest API or formatting regression could silently defeat the exact safeguard this PR introduces. A focused reporter test using representative modules or a small Vitest fixture would make these guarantees enforceable; similarconfig/logic is unit-tested intest/unit/guide-snippets.test.ts.
onTestRunEnd(testModules: ReadonlyArray<TestModule>): void {
Ryang-21
approved these changes
Aug 4, 2026
Contributor
Author
|
Closing in favor of #1589 |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
Vitest reports skipped tests as a single count in the summary. In a run of ~5500 passing tests,
6 skippedscrolls past unnoticed — so a test file that silently stops running doesn't fail anything and doesn't get spotted.Change
A custom reporter (
config/skipped-tests-reporter.ts) that prints an explicit block after each run:ctx.skip()notes when present<-- ENTIRE FILE[chromium]/[firefox]) when more than one project is activeRegistered in the node, browser, and e2e configs. Not added to the guides config — it inherits via
mergeConfig, and adding it there would register it twice.Deliberately advisory: it never fails a run. Whether a given skip is acceptable is a judgement call, so this only makes the cost visible.
Known limitations
it.onlyland in the skipped bucket, so one.onlyin a large generated suite prints a long listing. Local only — CI rejects.onlyviaallowOnly.--coverage, the coverage table prints after the block, since coverage is sequenced afteronTestRunEnd.