Skip to content

Conversation

@Skn0tt
Copy link
Member

@Skn0tt Skn0tt commented Jan 8, 2026

No description provided.

@Skn0tt Skn0tt requested a review from dgozman January 8, 2026 12:29
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@Skn0tt Skn0tt marked this pull request as ready for review January 9, 2026 10:45
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

this.screen.stdout.write(`\u001B[1A\u001B[2K`);
}
this.writeLine();
for (const error of formatResultFailure(this.screen, test, { ...result, errors: newErrors }, ' '))
Copy link
Contributor

Choose a reason for hiding this comment

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

  • I don't understand why this.epilogue() won't print all the errors anyway.
  • (just an idea) we can mark errors with some symbol to avoid double-printing them.

Copy link
Member Author

Choose a reason for hiding this comment

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

we're passing false to this.epilogue(), so it doesn't print any errors at all. All errors are printed during onTestEnd.

this._printedErrorCounts.set(result, result.errors.length);

if (result.errors.length > 0) {
const errorHeader = this.formatTestHeader(test, { indent: ' ', index: ++this._failures });
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can use a helper in base.ts, something like formatSingleResult(), that will include everything except for Paused .... Press Ctrl+C to end line.

Co-authored-by: Dmitry Gozman <dgozman@gmail.com>
Signed-off-by: Simon Knott <info@simonknott.de>
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@Skn0tt Skn0tt requested a review from dgozman January 14, 2026 09:14
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

updateSourceMethod: takeFirst(configCLIOverrides.updateSourceMethod, userConfig.updateSourceMethod, 'patch'),
version: require('../../package.json').version,
workers: resolveWorkers(takeFirst(configCLIOverrides.debug ? 1 : undefined, configCLIOverrides.workers, userConfig.workers, '50%')),
workers: resolveWorkers(takeFirst((configCLIOverrides.debug || configCLIOverrides.pause) ? 1 : undefined, configCLIOverrides.workers, userConfig.workers, '50%')),
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that debug implies pause, we can just check for pause here?

private _failures = 0;
private _lastTest: TestCase | undefined;
private _didBegin = false;
private _printedErrorCounts = new Map<TestResult, number>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this one.

}

for (const error of result.errors) {
const reportedIndex = (result as any)[kReportedSymbol] || 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make this an explicit call, something like markErrorsAsReported(result), to avoid surprises in the future.

@github-actions
Copy link
Contributor

Test results for "tests 1"

7 failed
❌ [default] › debug-tests.spec.ts:380 › should run global setup before debugging @vscode-extension
❌ [default] › debug-tests.spec.ts:516 › should not pause at the end of a setup test @vscode-extension
❌ [default-reuse] › debug-tests.spec.ts:380 › should run global setup before debugging @vscode-extension
❌ [default-reuse] › debug-tests.spec.ts:516 › should not pause at the end of a setup test @vscode-extension
❌ [default-trace] › debug-tests.spec.ts:380 › should run global setup before debugging @vscode-extension
❌ [default-trace] › debug-tests.spec.ts:516 › should not pause at the end of a setup test @vscode-extension
❌ [playwright-test] › runner.spec.ts:124 › should ignore subprocess creation error because of SIGINT @macos-latest-node20

4 flaky ⚠️ [chromium-library] › library/popup.spec.ts:258 › should not throw when click closes popup `@ubuntu-22.04-chromium-tip-of-tree`
⚠️ [firefox-library] › library/inspector/cli-codegen-1.spec.ts:1082 › cli codegen › should not throw csp directive violation errors `@firefox-ubuntu-22.04-node20`
⚠️ [playwright-test] › ui-mode-trace.spec.ts:426 › should work behind reverse proxy `@macos-latest-node20`
⚠️ [playwright-test] › ui-mode-trace.spec.ts:688 › should indicate current test status `@macos-latest-node20`

34458 passed, 699 skipped


Merge workflow run.

@Skn0tt Skn0tt merged commit ce4f077 into microsoft:main Jan 14, 2026
29 of 32 checks passed
@github-actions
Copy link
Contributor

Test results for "MCP"

1 failed
❌ [chrome] › mcp/autowait.spec.ts:19 › racy navigation destroys context @mcp-windows-latest

2821 passed, 116 skipped


Merge workflow run.

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