feat(vscode): report test run progress #720
Conversation
✅ Deploy Preview for rstest-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
eb7033a to
fc55505
Compare
There was a problem hiding this comment.
Pull request overview
This PR implements real-time test run progress reporting for the VSCode extension, enabling features like live test results, error location highlighting, and snapshot updating. The changes span both the core testing framework (adding new reporter hooks onTestSuiteStart and onTestSuiteResult) and the VSCode extension (implementing a comprehensive test run reporter with error formatting and stack trace parsing).
Key changes include:
- Added suite-level reporter hooks to track test suite execution lifecycle
- Implemented exact test name pattern matching with configurable delimiter
- Refactored VSCode extension to report progress in real-time via new
TestRunReporterclass - Enhanced error reporting with actual/expected value formatting using
pretty-format - Added support for updating snapshots directly from VSCode test results
Reviewed changes
Copilot reviewed 43 out of 44 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/types/reporter.ts | Added onTestSuiteStart and onTestSuiteResult hooks, logger option for default reporter |
| packages/core/src/types/testSuite.ts | Extracted TestSuiteInfo type, added expected/actual fields to FormattedError |
| packages/core/src/types/config.ts | Added exact flag for test name pattern matching |
| packages/core/src/runtime/runner/runner.ts | Implemented suite result tracking and hook invocations |
| packages/core/src/runtime/runner/task.ts | Added exact matching logic for test name patterns |
| packages/core/src/utils/error.ts | Made getSourcemap parameter optional |
| packages/vscode/src/testRunReporter.ts | New class implementing real-time test progress reporting with error formatting |
| packages/vscode/src/worker/reporter.ts | Refactored to use new progress-based reporter architecture |
| packages/vscode/src/worker/index.ts | Updated to pass reporter and logger instances to rstest |
| packages/vscode/src/master.ts | Added onTestProgress method to handle worker progress events |
| packages/vscode/src/testTree.ts | Simplified test tree parsing with exit callbacks, removed old result handling |
| packages/vscode/src/extension.ts | Added snapshot update command and improved test run workflow |
| packages/vscode/package.json | Added pretty-format dependency and snapshot update command configuration |
| packages/vscode/tests/suite/progress.test.ts | New integration test for progress reporting functionality |
| e2e/reporter/rstest.customReporterConfig.ts | Added suite hooks to test reporter |
| pnpm-lock.yaml | Updated dependencies (@rslib/core, @rsbuild/core, @rspack versions) |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "strict": true, | ||
| "skipLibCheck": true | ||
| "skipLibCheck": true, | ||
| "types": ["../core/src/env.d.ts"] |
There was a problem hiding this comment.
The types compiler option expects an array of package names or paths to type definition files, not a path to a .d.ts file. This should likely be removed and handled differently, or you should use typeRoots instead. The current configuration will likely not work as intended.
| import { ROOT_SUITE_NAME } from '../../core/src/utils/constants'; | ||
| import { parseErrorStacktrace } from '../../core/src/utils/error'; |
There was a problem hiding this comment.
This import uses a relative path '../../core/src/utils/constants' to access core package source directly. While the PR description mentions this as a known concern ("Is it ok to import core source code from vscode extension directly?"), this creates tight coupling and build order dependencies. Consider either:
- Exporting these constants from
@rstest/corepackage's main entry point - Duplicating the constant in the vscode extension
- Creating a shared constants package
The same applies to the import on line 12 (parseErrorStacktrace).
| async onTestProgress<T extends keyof TestRunReporter>( | ||
| runId: string, | ||
| event: T, | ||
| param: Parameters<NonNullable<TestRunReporter[T]>>[0], | ||
| ) { | ||
| const reporter = this.testRunReporters.get(runId); | ||
| // @ts-expect-error |
There was a problem hiding this comment.
The @ts-expect-error comment suppresses type checking without explanation. Consider adding a comment explaining why this is necessary, or better yet, fixing the type issue. The problem likely stems from TypeScript's inability to ensure param matches the expected parameter type for the specific event method.
| async onTestProgress<T extends keyof TestRunReporter>( | |
| runId: string, | |
| event: T, | |
| param: Parameters<NonNullable<TestRunReporter[T]>>[0], | |
| ) { | |
| const reporter = this.testRunReporters.get(runId); | |
| // @ts-expect-error | |
| // Map each method name to its first parameter type | |
| type TestRunReporterEventParams = { | |
| [K in keyof TestRunReporter]: TestRunReporter[K] extends (arg: infer P, ...args: any[]) => any ? P : never; | |
| }; | |
| async onTestProgress<T extends keyof TestRunReporter>( | |
| runId: string, | |
| event: T, | |
| param: TestRunReporterEventParams[T], | |
| ) { | |
| const reporter = this.testRunReporters.get(runId); |
| export default defineConfig({ | ||
| setupFiles: ['../scripts/rstest.setup.ts'], | ||
| testTimeout: process.env.CI ? 10_000 : 5_000, | ||
| testTimeout: 10_000, |
There was a problem hiding this comment.
The change removes the conditional timeout based on CI environment (process.env.CI ? 10_000 : 5_000) and sets a fixed 10-second timeout. This may slow down local development since tests will now wait up to 10 seconds even in non-CI environments. Consider keeping the conditional logic to maintain faster feedback in local development.
| testTimeout: 10_000, | |
| testTimeout: process.env.CI ? 10_000 : 5_000, |
6a0f62f to
2023755
Compare
Changes Summary
onTestSuiteStartandonTestSuiteResulttestNamePatternexactly (This is still a temporary solution that cannot handle cases where the name contains>. I plan to support matching by arrays instead of strings in the future.)actualandexpectedvalue inerrorobjectupdateFromContents, now ancestors stack is poped when exit astnodeNew Features
20251127-095853.mp4
Problems
Related Links
Checklist