Skip to content

feat: Display warnings only on test failure #35464

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

Merged
merged 18 commits into from
Apr 8, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/playwright/src/common/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export class FullConfigInternal {
cliLastFailed?: boolean;
testIdMatcher?: Matcher;
lastFailedTestIdMatcher?: Matcher;
warningTestIdMatcher?: Matcher;
defineConfigWasUsed = false;

globalSetups: string[] = [];
Expand Down
13 changes: 12 additions & 1 deletion packages/playwright/src/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { Runner } from './runner/runner';
import * as testServer from './runner/testServer';
import { runWatchModeLoop } from './runner/watchMode';
import { serializeError } from './util';
import { LastRunReporter } from './runner/lastRun';

import type { TestError } from '../types/testReporter';
import type { ConfigCLIOverrides } from './common/ipc';
Expand Down Expand Up @@ -218,7 +219,16 @@ async function runTests(args: string[], opts: { [key: string]: any }) {
return;
}

const runner = new Runner(config);
const lastRun = new LastRunReporter(config);
const runner = new Runner(config, lastRun);

if (opts.showWarnings) {
await lastRun.filterWarnings();
const { status } = await runner.printWarnings(lastRun);
const exitCode = status === 'interrupted' ? 130 : (status === 'passed' ? 0 : 1);
gracefullyProcessExitDoNotHang(exitCode);
}

const status = await runner.runAllTests();
await stopProfiling('runner');
const exitCode = status === 'interrupted' ? 130 : (status === 'passed' ? 0 : 1);
Expand Down Expand Up @@ -415,6 +425,7 @@ const testOptions: [string, string][] = [
['--reporter <reporter>', `Reporter to use, comma-separated, can be ${builtInReporters.map(name => `"${name}"`).join(', ')} (default: "${defaultReporter}")`],
['--retries <retries>', `Maximum retry count for flaky tests, zero for no retries (default: no retries)`],
['--shard <shard>', `Shard tests and execute only the selected shard, specify in the form "current/all", 1-based, for example "3/5"`],
['--show-warnings', `Show all warning messages collected in the last test run. Does not execute tests.`],
['--timeout <timeout>', `Specify test timeout threshold in milliseconds, zero for unlimited (default: ${defaultTimeout})`],
['--trace <mode>', `Force tracing mode, can be ${kTraceModes.map(mode => `"${mode}"`).join(', ')}`],
['--tsconfig <path>', `Path to a single tsconfig applicable to all imported files (default: look up tsconfig for each imported file separately)`],
Expand Down
25 changes: 4 additions & 21 deletions packages/playwright/src/prompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ import * as path from 'path';

import { parseErrorStack } from 'playwright-core/lib/utils';

import { stripAnsiEscapes } from './util';
import { codeFrameColumns } from './transform/babelBundle';
import { loadCodeFrame, stripAnsiEscapes } from './util';

import type { TestInfo } from '../types/test';
import type { MetadataWithCommitInfo } from './isomorphic/types';
Expand Down Expand Up @@ -87,15 +86,9 @@ export async function attachErrorPrompts(testInfo: TestInfo, sourceCache: Map<st
const parsedError = error.stack ? parseErrorStack(error.stack, path.sep) : undefined;
const inlineMessage = stripAnsiEscapes(parsedError?.message || error.message || '').split('\n')[0];
const location = parsedError?.location || { file: testInfo.file, line: testInfo.line, column: testInfo.column };
const source = await loadSource(location.file, sourceCache);
const codeFrame = codeFrameColumns(
source,
{
start: {
line: location.line,
column: location.column
},
},
const codeFrame = await loadCodeFrame(
location,
sourceCache,
{
highlightCode: false,
linesAbove: 100,
Expand Down Expand Up @@ -146,13 +139,3 @@ export async function attachErrorContext(testInfo: TestInfo, ariaSnapshot: strin
})),
}, undefined);
}

async function loadSource(file: string, sourceCache: Map<string, string>) {
let source = sourceCache.get(file);
if (!source) {
// A mild race is Ok here.
source = await fs.readFile(file, 'utf8');
sourceCache.set(file, source);
}
return source;
}
41 changes: 13 additions & 28 deletions packages/playwright/src/reporters/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { getEastAsianWidth } from '../utilsBundle';
import type { ReporterV2 } from './reporterV2';
import type { FullConfig, FullResult, Location, Suite, TestCase, TestError, TestResult, TestStep } from '../../types/testReporter';
import type { Colors } from '@isomorphic/colors';
import type { TestAnnotation } from '../../types/test';

export type TestResultOutput = { chunk: string | Buffer, type: 'stdout' | 'stderr' };
export const kOutputSymbol = Symbol('output');
Expand All @@ -44,6 +45,7 @@ type TestSummary = {
flaky: TestCase[];
failuresToPrint: TestCase[];
fatalErrors: TestError[];
warnings: TestAnnotation[];
};

export type Screen = {
Expand Down Expand Up @@ -192,7 +194,7 @@ export class TerminalReporter implements ReporterV2 {
return fileDurations.filter(([, duration]) => duration > threshold).slice(0, count);
}

protected generateSummaryMessage({ didNotRun, skipped, expected, interrupted, unexpected, flaky, fatalErrors }: TestSummary) {
protected generateSummaryMessage({ didNotRun, skipped, expected, interrupted, unexpected, flaky, fatalErrors, warnings }: TestSummary) {
const tokens: string[] = [];
if (unexpected.length) {
tokens.push(this.screen.colors.red(` ${unexpected.length} failed`));
Expand All @@ -209,6 +211,8 @@ export class TerminalReporter implements ReporterV2 {
for (const test of flaky)
tokens.push(this.screen.colors.yellow(this.formatTestHeader(test, { indent: ' ' })));
}
if (warnings.length)
tokens.push(this.screen.colors.yellow(` ${warnings.length} warnings. Run "npx playwright test --show-warnings" for more information.`));
if (skipped)
tokens.push(this.screen.colors.yellow(` ${skipped} skipped`));
if (didNotRun)
Expand All @@ -229,8 +233,14 @@ export class TerminalReporter implements ReporterV2 {
const interruptedToPrint: TestCase[] = [];
const unexpected: TestCase[] = [];
const flaky: TestCase[] = [];
const warnings: TestAnnotation[] = [];

this.suite.allTests().forEach(test => {
[...test.annotations, ...test.results.flatMap(r => r.annotations)].forEach(annotation => {
if (annotation.type === 'warning')
warnings.push(annotation);
});

switch (test.outcome()) {
case 'skipped': {
if (test.results.some(result => result.status === 'interrupted')) {
Expand Down Expand Up @@ -260,6 +270,7 @@ export class TerminalReporter implements ReporterV2 {
flaky,
failuresToPrint,
fatalErrors: this._fatalErrors,
warnings,
};
}

Expand All @@ -269,7 +280,6 @@ export class TerminalReporter implements ReporterV2 {
if (full && summary.failuresToPrint.length && !this._omitFailures)
this._printFailures(summary.failuresToPrint);
this._printSlowTests();
this._printWarnings();
this._printSummary(summaryMessage);
}

Expand All @@ -289,31 +299,6 @@ export class TerminalReporter implements ReporterV2 {
console.log(this.screen.colors.yellow(' Consider running tests from slow files in parallel, see https://playwright.dev/docs/test-parallel.'));
}

private _printWarnings() {
const warningTests = this.suite.allTests().filter(test => {
const annotations = [...test.annotations, ...test.results.flatMap(r => r.annotations)];
return annotations.some(a => a.type === 'warning');
});
const encounteredWarnings = new Map<string, Array<TestCase>>();
for (const test of warningTests) {
for (const annotation of [...test.annotations, ...test.results.flatMap(r => r.annotations)]) {
if (annotation.type !== 'warning' || annotation.description === undefined)
continue;
let tests = encounteredWarnings.get(annotation.description);
if (!tests) {
tests = [];
encounteredWarnings.set(annotation.description, tests);
}
tests.push(test);
}
}
for (const [description, tests] of encounteredWarnings) {
console.log(this.screen.colors.yellow(' Warning: ') + description);
for (const test of tests)
console.log(this.formatTestHeader(test, { indent: ' ', mode: 'default' }));
}
}

private _printSummary(summary: string) {
if (summary.trim())
console.log(summary);
Expand Down Expand Up @@ -464,7 +449,7 @@ function formatTestTitle(screen: Screen, config: FullConfig, test: TestCase, ste
return `${testTitle}${stepSuffix(step)}${extraTags.length ? ' ' + extraTags.join(' ') : ''}`;
}

function formatTestHeader(screen: Screen, config: FullConfig, test: TestCase, options: { indent?: string, index?: number, mode?: 'default' | 'error' } = {}): string {
export function formatTestHeader(screen: Screen, config: FullConfig, test: TestCase, options: { indent?: string, index?: number, mode?: 'default' | 'error' } = {}): string {
const title = formatTestTitle(screen, config, test);
const header = `${options.indent || ''}${options.index ? options.index + ') ' : ''}${title}`;
let fullHeader = header;
Expand Down
40 changes: 35 additions & 5 deletions packages/playwright/src/runner/lastRun.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,22 @@ import { filterProjects } from './projectUtils';
import type { FullResult, Suite } from '../../types/testReporter';
import type { FullConfigInternal } from '../common/config';
import type { ReporterV2 } from '../reporters/reporterV2';
import type { TestAnnotation } from 'packages/playwright-test';

type WarningAnnotation = Omit<TestAnnotation, 'type'> & { type: 'warning' };

type LastRunInfo = {
status: FullResult['status'];
failedTests: string[];
warningTests: {
[id: string]: WarningAnnotation[];
};
};

export class LastRunReporter implements ReporterV2 {
private _config: FullConfigInternal;
private _lastRunFile: string | undefined;
private _lastRunInfo: LastRunInfo | undefined;
private _suite: Suite | undefined;

constructor(config: FullConfigInternal) {
Expand All @@ -40,16 +47,33 @@ export class LastRunReporter implements ReporterV2 {
this._lastRunFile = path.join(project.project.outputDir, '.last-run.json');
}

async filterLastFailed() {
async runInfo() {
if (this._lastRunInfo)
return this._lastRunInfo;

if (!this._lastRunFile)
return;
return undefined;

try {
const lastRunInfo = JSON.parse(await fs.promises.readFile(this._lastRunFile, 'utf8')) as LastRunInfo;
this._config.lastFailedTestIdMatcher = id => lastRunInfo.failedTests.includes(id);
return JSON.parse(await fs.promises.readFile(this._lastRunFile, 'utf8')) as LastRunInfo;
} catch {
}
}

async filterLastFailed() {
const runInfo = await this.runInfo();
if (!runInfo)
return;
this._config.lastFailedTestIdMatcher = id => runInfo.failedTests.includes(id);
}

async filterWarnings() {
const runInfo = await this.runInfo();
if (!runInfo)
return;
this._config.warningTestIdMatcher = id => id in runInfo.warningTests;
}

version(): 'v2' {
return 'v2';
}
Expand All @@ -67,7 +91,13 @@ export class LastRunReporter implements ReporterV2 {
return;
await fs.promises.mkdir(path.dirname(this._lastRunFile), { recursive: true });
const failedTests = this._suite?.allTests().filter(t => !t.ok()).map(t => t.id);
const lastRunReport = JSON.stringify({ status: result.status, failedTests }, undefined, 2);
const warningTests: LastRunInfo['warningTests'] = {};
for (const test of this._suite?.allTests() ?? []) {
const warningAnnotations = [...test.annotations, ...test.results.flatMap(r => r.annotations)].filter(a => a.type === 'warning');
if (warningAnnotations.length > 0)
warningTests[test.id] = warningAnnotations as WarningAnnotation[];
}
const lastRunReport = JSON.stringify({ status: result.status, failedTests, warningTests }, undefined, 2);
await fs.promises.writeFile(this._lastRunFile, lastRunReport);
}
}
2 changes: 2 additions & 0 deletions packages/playwright/src/runner/loadUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,8 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho
// Explicitly apply --last-failed filter after sharding.
if (config.lastFailedTestIdMatcher)
filterByTestIds(rootSuite, config.lastFailedTestIdMatcher);
if (config.warningTestIdMatcher)
filterByTestIds(rootSuite, config.warningTestIdMatcher);

// Now prepend dependency projects without filtration.
{
Expand Down
88 changes: 86 additions & 2 deletions packages/playwright/src/runner/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ import { webServerPluginsForConfig } from '../plugins/webServerPlugin';
import { terminalScreen } from '../reporters/base';
import { InternalReporter } from '../reporters/internalReporter';
import { affectedTestFiles } from '../transform/compilationCache';
import { formatTestHeader } from '../reporters/base';
import { loadCodeFrame } from '../util';

import type { TestAnnotation, Location } from '../../types/test';
import type { TestCase } from '../common/test';
import type { FullResult, TestError } from '../../types/testReporter';
import type { FullConfigInternal } from '../common/config';

Expand All @@ -48,9 +52,11 @@ export type FindRelatedTestFilesReport = {

export class Runner {
private _config: FullConfigInternal;
private _lastRun?: LastRunReporter;

constructor(config: FullConfigInternal) {
constructor(config: FullConfigInternal, lastRun?: LastRunReporter) {
this._config = config;
this._lastRun = lastRun;
}

async listTestFiles(projectNames?: string[]): Promise<ConfigListFilesReport> {
Expand Down Expand Up @@ -79,7 +85,7 @@ export class Runner {
webServerPluginsForConfig(config).forEach(p => config.plugins.push({ factory: p }));

const reporters = await createReporters(config, listOnly ? 'list' : 'test', false);
const lastRun = new LastRunReporter(config);
const lastRun = this._lastRun ?? new LastRunReporter(config);
if (config.cliLastFailed)
await lastRun.filterLastFailed();

Expand Down Expand Up @@ -134,4 +140,82 @@ export class Runner {
]);
return { status };
}

async printWarnings(lastRun: LastRunReporter) {
const reporter = new InternalReporter([createErrorCollectingReporter(terminalScreen, true)]);
const testRun = new TestRun(this._config, reporter);
const status = await runTasks(testRun, [
...createPluginSetupTasks(this._config),
createLoadTask('in-process', { failOnLoadErrors: true, filterOnly: false })
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm... If we store warnings in .last-run.json, why do we need to load any tests? I'd think we have all the information needed for the warning in the json, so we can just print it? If so, I'd put all this logic into a separate file warnings.ts and let it manage both the "warnings data in last-run.json" and the terminal output.

That said, I think we should make warnings conveniently available to other reporters. For example, do we want to show warning per test result in the html reporter, or do we want a similar summary? If we want a summary, perhaps generate it and attach? Needs brainstorming.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Storing all of the necessary test information is valid, but I didn't want to deviate significantly from the current patterns. If we did store more information than just test ID, would we store code frames? Or would we just store the test name and still fetch the code frame at runtime?


You don't consider the annotation reporter API to be sufficient? A reporter can read all warning annotations onEnd.

]);

const tests = testRun.rootSuite?.allTests() ?? [];
const testsMap = new Map(tests.map(test => [test.id, test]));

const lastRunInfo = await lastRun.runInfo();
const knownWarnings = lastRunInfo?.warningTests ?? {};

const testToWarnings = Object.entries(knownWarnings).flatMap(([id, warnings]) => {
const test = testsMap.get(id);
if (!test)
return [];

return { test, warnings };
});

const sourceCache = new Map<string, string>();

const warningMessages = await Promise.all(testToWarnings.map(({ test, warnings }, i) => this._buildWarning(test, warnings, i + 1, sourceCache)));
if (warningMessages.length > 0) {
// eslint-disable-next-line no-console
console.log(`${warningMessages.join('\n')}\n`);
}

return { status };
}

private async _buildWarning(test: TestCase, warnings: TestAnnotation[], renderIndex: number, sourceCache: Map<string, string>): Promise<string> {
const encounteredWarnings = new Map<string, Array<Location | undefined>>();
for (const annotation of warnings) {
if (annotation.description === undefined)
continue;
let matchingWarnings = encounteredWarnings.get(annotation.description);
if (!matchingWarnings) {
matchingWarnings = [];
encounteredWarnings.set(annotation.description, matchingWarnings);
}
matchingWarnings.push(annotation.location);
}

// Sort warnings by location inside of each category
for (const locations of encounteredWarnings.values()) {
locations.sort((a, b) => {
if (!a)
return 1;
if (!b)
return -1;
if (a.line !== b.line)
return a.line - b.line;
if (a.column !== b.column)
return a.column - b.column;
return 0;
});
}

const testHeader = formatTestHeader(terminalScreen, this._config.config, test, { indent: ' ', index: renderIndex });

const codeFrameIndent = ' ';

const warningMessages = await Promise.all(encounteredWarnings.entries().map(async ([description, locations]) => {
const renderedCodeFrames = await Promise.all(locations.flatMap(location => !!location ? loadCodeFrame(location, sourceCache, { highlightCode: true }) : []));
const indentedCodeFrames = renderedCodeFrames.map(f => f.split('\n').map(line => `${codeFrameIndent}${line}`).join('\n'));

const warningCount = locations.length > 1 ? ` (x${locations.length})` : '';
const allFrames = renderedCodeFrames.length > 0 ? `\n\n${indentedCodeFrames.join('\n\n')}` : '';

return ` ${terminalScreen.colors.yellow(`Warning${warningCount}: ${description}`)}${allFrames}`;
}));

return `\n${testHeader}\n\n${warningMessages.join('\n\n')}`;
}
}
Loading
Loading