-
-
Notifications
You must be signed in to change notification settings - Fork 113
feat: use reporters in the test results panel #707
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
c80d6b9
bcac665
6751de7
4451d63
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,7 @@ export function createRpcOptions() { | |
| onCollected: createHandler<ExtensionWorkerEvents['onCollected']>(), | ||
| onTestRunStart: createHandler<ExtensionWorkerEvents['onTestRunStart']>(), | ||
| onTestRunEnd: createHandler<ExtensionWorkerEvents['onTestRunEnd']>(), | ||
| onProcessLog: createHandler<ExtensionWorkerEvents['onProcessLog']>(), | ||
| } | ||
|
|
||
| const events: Omit<ExtensionWorkerEvents, 'onReady' | 'onError'> = { | ||
|
|
@@ -42,6 +43,7 @@ export function createRpcOptions() { | |
| onCollected: handlers.onCollected.trigger, | ||
| onTestRunStart: handlers.onTestRunStart.trigger, | ||
| onProcessLog(type, message) { | ||
| handlers.onProcessLog.trigger(type, message) | ||
| log.worker(type === 'stderr' ? 'error' : 'info', stripVTControlCharacters(message)) | ||
| }, | ||
| } | ||
|
|
@@ -54,12 +56,18 @@ export function createRpcOptions() { | |
| onTestRunEnd: handlers.onTestRunEnd.register, | ||
| onCollected: handlers.onCollected.register, | ||
| onTestRunStart: handlers.onTestRunStart.register, | ||
| onProcessLog: handlers.onProcessLog.register, | ||
| removeListener(name: string, listener: any) { | ||
| handlers[name as 'onCollected']?.remove(listener) | ||
| }, | ||
| clearListeners() { | ||
| for (const name in handlers) | ||
| handlers[name as 'onCollected']?.clear() | ||
| // Clear all handlers except onProcessLog, which needs to persist | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do this? the handlers are not cleaned after the test run, they are cleaned when the fork is destroyed, |
||
| // across test runs to forward stdout from Vitest to the extension | ||
| handlers.onConsoleLog.clear() | ||
| handlers.onTaskUpdate.clear() | ||
| handlers.onCollected.clear() | ||
| handlers.onTestRunStart.clear() | ||
| handlers.onTestRunEnd.clear() | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -160,37 +160,16 @@ export class TestRunner extends vscode.Disposable { | |
| if (unhandledError) | ||
| testRun.appendOutput(formatTestOutput(unhandledError)) | ||
|
|
||
| if (!collecting) | ||
| this.endTestRun() | ||
| }) | ||
|
|
||
| api.onConsoleLog((consoleLog) => { | ||
| const testItem = consoleLog.taskId ? tree.getTestItemByTaskId(consoleLog.taskId) : undefined | ||
| const testRun = this.testRun | ||
| if (testRun) { | ||
| // Create location from parsed console log for inline display | ||
| // Only set location if inline console logs are enabled | ||
| let location: vscode.Location | undefined | ||
| if (consoleLog.parsedLocation && this.showInlineConsoleLog) { | ||
| const uri = vscode.Uri.file(consoleLog.parsedLocation.file) | ||
| const position = new vscode.Position( | ||
| consoleLog.parsedLocation.line, | ||
| consoleLog.parsedLocation.column, | ||
| ) | ||
| location = new vscode.Location(uri, position) | ||
| } | ||
|
|
||
| testRun.appendOutput( | ||
| formatTestOutput(consoleLog.content) + (consoleLog.browser ? '\r\n' : ''), | ||
| location, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With this change logs are no longer attributed to tests and are not shown inline in UI |
||
| testItem, | ||
| ) | ||
| } | ||
| else { | ||
| log.info('[TEST]', consoleLog.content) | ||
| // Signal that the test run is complete, but DON'T set this.testRun to undefined yet | ||
| // The testRun will be properly ended in the finally block of startTestRun | ||
| if (!collecting) { | ||
| this.testRunDefer?.resolve() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why replace |
||
| } | ||
| }) | ||
|
|
||
| // Skip individual console logs since DefaultReporter already includes them | ||
| // api.onConsoleLog((consoleLog) => { ... } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this? |
||
|
|
||
| // Listen to configuration changes | ||
| this.disposables.push( | ||
| vscode.workspace.onDidChangeConfiguration((event) => { | ||
|
|
@@ -449,9 +428,26 @@ export class TestRunner extends vscode.Disposable { | |
| const run = this.testRun = this.controller.createTestRun(request, name) | ||
| this.testRunRequest = request | ||
| this.testRunDefer = Promise.withResolvers() | ||
|
|
||
| // Show the equivalent CLI command for debugging/reproducibility | ||
| const fileList = files.map(f => this.relative(f)).join(' ') | ||
| const pattern = formatTestPattern(request.include || []) | ||
| let vitestCmd = 'vitest' | ||
| if (fileList) { | ||
| vitestCmd += ` ${fileList}` | ||
| } | ||
| if (pattern) { | ||
| vitestCmd += ` -t "${pattern}"` | ||
| } | ||
| run.appendOutput(`\x1B[36m\x1B[1m[command]\x1B[0m ${vitestCmd}\r\n\r\n`) | ||
|
|
||
|
Comment on lines
+431
to
+443
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not how the extension works, we don't create a new process for every test run. Even more, I am planning to change to the new |
||
| // run the next test when this one finished, or cancell or test runs if they were cancelled | ||
| this.testRunDefer.promise = this.testRunDefer.promise.finally(() => { | ||
| run.end() | ||
| this.testRun = undefined | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why? |
||
| this.testRunDefer = undefined | ||
| this.testRunRequest = undefined | ||
|
|
||
| if (this.cancelled) { | ||
| log.verbose?.('Not starting a new test run because the previous one was cancelled manually.') | ||
| this.scheduleTestRunsQueue.forEach(item => item.resolveWithoutRunning()) | ||
|
|
@@ -797,7 +793,7 @@ function formatTestPattern(tests: readonly vscode.TestItem[]) { | |
| } | ||
|
|
||
| function formatTestOutput(output: string) { | ||
| return stripVTControlCharacters(output.replace(/(?<!\r)\n/g, '\r\n')) | ||
| return output.replace(/(?<!\r)\n/g, '\r\n') | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test results panel in VSCode / Cursor supports colors. No need to remove them.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Colors are not supported in inline view, at least |
||
| } | ||
|
|
||
| function labelTestItems(items: readonly vscode.TestItem[] | undefined) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,7 +68,6 @@ export async function initVitest( | |
| api: false, | ||
| // @ts-expect-error private property | ||
| reporter: undefined, | ||
| reporters: [reporter], | ||
| ui: false, | ||
| includeTaskLocation: true, | ||
| execArgv: meta.pnpApi && meta.pnpLoader | ||
|
|
@@ -105,13 +104,23 @@ export async function initVitest( | |
| coverageOptions.reporter = [ | ||
| ['json', { ...jsonReporterOptions, file: meta.finalCoverageFileName }], | ||
| ] | ||
|
|
||
| const rawReporters = testConfig.reporters | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We intentionally don't show the report since vscode already has the UI to show everything. It is a waste of resources to duplicate it The extension also doesn't support the same hooks order, so custom reporters might break If the intention is to help AI, then it is better to have a custom reporter designed for that (to reduce the amount of tokens, at least) or something like this: #676 |
||
| const userReporters = (Array.isArray(rawReporters) ? rawReporters : (rawReporters ? [rawReporters] : [])) | ||
| .filter((r: string) => r !== 'html') | ||
| const hasReporters = userReporters.length > 0 | ||
|
|
||
| return { | ||
| test: { | ||
| printConsoleTrace: true, | ||
| coverage: { | ||
| reportOnFailure: true, | ||
| reportsDirectory: join(tmpdir(), `vitest-coverage-${randomUUID()}`), | ||
| }, | ||
| // If user already has reporters, we only return ours and let Vitest merge it. | ||
| // This prevents duplication since Vite merges arrays by appending. | ||
| reporters: hasReporters | ||
| ? [reporter] | ||
| : ['default', reporter], | ||
| }, | ||
| // TODO: type is not augmented | ||
| } as any | ||
|
|
@@ -136,10 +145,6 @@ export async function initVitest( | |
| }, | ||
| ], | ||
| }, | ||
| { | ||
| stderr, | ||
| stdout, | ||
| }, | ||
| ) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why? This makes it so unhandled errors outside of tests are not reporter (failed global setup, for example) |
||
| await (vitest as any).report('onInit', vitest) | ||
| const configs = [ | ||
|
|
||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why?