microsoft
/
playwright
зеркало из https://github.com/microsoft/playwright.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

feat(test runner): on beforeAll failure, precisely skip the tests (#12417) 

Previously, we used to skip all the tests from the same file when
any `beforeAll` fails in the file.

Now, we only skip the rest of the tests affected by this particular
`beforeAll` and continue with other tests in the new worker.
This commit is contained in:
Dmitry Gozman 2022-03-08 20:29:31 -08:00 коммит произвёл GitHub
Родитель 4a768294b4
Коммит e8ce5d0258
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
5 изменённых файлов: 104 добавлений и 60 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

67
packages/playwright-test/src/dispatcher.ts
Просмотреть файл

@ -18,7 +18,7 @@ import child_process from 'child_process';
import path from 'path';
import { EventEmitter } from 'events';
import { RunPayload, TestBeginPayload, TestEndPayload, DonePayload, TestOutputPayload, WorkerInitParams, StepBeginPayload, StepEndPayload, SerializedLoaderData, TeardownErrorsPayload } from './ipc';
import type { TestResult, Reporter, TestStep } from '../types/testReporter';
import type { TestResult, Reporter, TestStep, TestError } from '../types/testReporter';
import { Suite, TestCase } from './test';
import { Loader } from './loader';
import { ManualPromise } from 'playwright-core/lib/utils/async';
@ -278,7 +278,7 @@ export class Dispatcher {
// - there are no remaining
// - we are here not because something failed
// - no unrecoverable worker error
if (!remaining.length && !failedTestIds.size && !params.fatalErrors.length && !params.skipRemaining) {
if (!remaining.length && !failedTestIds.size && !params.fatalErrors.length && !params.skipTestsDueToSetupFailure.length) {
if (this._isWorkerRedundant(worker))
worker.stop();
doneWithJob();
@ -288,41 +288,46 @@ export class Dispatcher {
// When worker encounters error, we will stop it and create a new one.
worker.stop(true /* didFail */);
// In case of fatal error, report first remaining test as failing with this error,
// and all others as skipped.
if (params.fatalErrors.length || params.skipRemaining) {
let shouldAddFatalErrorsToNextTest = params.fatalErrors.length > 0;
for (const test of remaining) {
if (this._hasReachedMaxFailures())
break;
const data = this._testById.get(test._id)!;
const runData = data.resultByWorkerIndex.get(worker.workerIndex);
// There might be a single test that has started but has not finished yet.
let result: TestResult;
if (runData) {
result = runData.result;
} else {
result = data.test._appendTestResult();
this._reporter.onTestBegin?.(test, result);
const massSkipTestsFromRemaining = (testIds: Set<string>, errors: TestError[]) => {
remaining = remaining.filter(test => {
if (!testIds.has(test._id))
return true;
if (!this._hasReachedMaxFailures()) {
const data = this._testById.get(test._id)!;
const runData = data.resultByWorkerIndex.get(worker.workerIndex);
// There might be a single test that has started but has not finished yet.
let result: TestResult;
if (runData) {
result = runData.result;
} else {
result = data.test._appendTestResult();
this._reporter.onTestBegin?.(test, result);
}
result.errors = [...errors];
result.error = result.errors[0];
result.status = errors.length ? 'failed' : 'skipped';
this._reportTestEnd(test, result);
failedTestIds.add(test._id);
errors = []; // Only report errors for the first test.
}
result.errors = shouldAddFatalErrorsToNextTest ? [...params.fatalErrors] : [];
result.error = result.errors[0];
result.status = shouldAddFatalErrorsToNextTest ? 'failed' : 'skipped';
this._reportTestEnd(test, result);
failedTestIds.add(test._id);
shouldAddFatalErrorsToNextTest = false;
}
if (shouldAddFatalErrorsToNextTest) {
// We had a fatal error after all tests have passed - most likely in the afterAll hook.
return false;
});
if (errors.length) {
// We had fatal errors after all tests have passed - most likely in some teardown.
// Let's just fail the test run.
this._hasWorkerErrors = true;
for (const error of params.fatalErrors)
this._reporter.onError?.(error);
}
// Since we pretend that all remaining tests failed/skipped, there is nothing else to run,
// except for possible retries.
remaining = [];
};
if (params.fatalErrors.length) {
// In case of fatal errors, report first remaining test as failing with these errors,
// and all others as skipped.
massSkipTestsFromRemaining(new Set(remaining.map(test => test._id)), params.fatalErrors);
}
// Handle tests that should be skipped because of the setup failure.
massSkipTestsFromRemaining(new Set(params.skipTestsDueToSetupFailure), []);
const retryCandidates = new Set<string>();
const serialSuitesWithFailures = new Set<Suite>();
@ -384,7 +389,7 @@ export class Dispatcher {
worker.on('done', onDone);
const onExit = (expectedly: boolean) => {
onDone({ skipRemaining: false, fatalErrors: expectedly ? [] : [{ value: 'Worker process exited unexpectedly' }] });
onDone({ skipTestsDueToSetupFailure: [], fatalErrors: expectedly ? [] : [{ value: 'Worker process exited unexpectedly' }] });
};
worker.on('exit', onExit);

2
packages/playwright-test/src/ipc.ts
Просмотреть файл

@ -76,7 +76,7 @@ export type RunPayload = {
export type DonePayload = {
fatalErrors: TestError[];
skipRemaining: boolean;
skipTestsDueToSetupFailure: string[]; // test ids
};
export type TestOutputPayload = {

58
packages/playwright-test/src/workerRunner.ts
Просмотреть файл

@ -39,9 +39,9 @@ export class WorkerRunner extends EventEmitter {
// Accumulated fatal errors that cannot be attributed to a test.
private _fatalErrors: TestError[] = [];
// Whether we should skip running remaining tests in the group because
// Whether we should skip running remaining tests in this suite because
// of a setup error, usually beforeAll hook.
private _skipRemainingTests = false;
private _skipRemainingTestsInSuite: Suite | undefined;
// The stage of the full cleanup. Once "finished", we can safely stop running anything.
private _didRunFullCleanup = false;
// Whether the worker was requested to stop.
@ -134,8 +134,8 @@ export class WorkerRunner extends EventEmitter {
async runTestGroup(runPayload: RunPayload) {
this._runFinished = new ManualPromise<void>();
const entries = new Map(runPayload.entries.map(e => [ e.testId, e ]));
try {
const entries = new Map(runPayload.entries.map(e => [ e.testId, e ]));
await this._loadIfNeeded();
const fileSuite = await this._loader.loadTestFile(runPayload.file, 'worker');
const suite = this._project.cloneFileSuite(fileSuite, this._params.repeatEachIndex, test => {
@ -148,8 +148,14 @@ export class WorkerRunner extends EventEmitter {
this._activeSuites = new Set();
this._didRunFullCleanup = false;
const tests = suite.allTests().filter(test => entries.has(test._id));
for (let i = 0; i < tests.length; i++)
await this._runTest(tests[i], entries.get(tests[i]._id)!.retry, tests[i + 1]);
for (let i = 0; i < tests.length; i++) {
// Do not run tests after full cleanup, because we are entirely done.
if (this._isStopped && this._didRunFullCleanup)
break;
const entry = entries.get(tests[i]._id)!;
entries.delete(tests[i]._id);
await this._runTest(tests[i], entry.retry, tests[i + 1]);
}
}
} catch (e) {
// In theory, we should run above code without any errors.
@ -157,16 +163,22 @@ export class WorkerRunner extends EventEmitter {
// but not in the runner, let's do a fatal error.
this.unhandledError(e);
} finally {
this._reportDone();
const donePayload: DonePayload = {
fatalErrors: this._fatalErrors,
skipTestsDueToSetupFailure: [],
};
for (const test of this._skipRemainingTestsInSuite?.allTests() || []) {
if (entries.has(test._id))
donePayload.skipTestsDueToSetupFailure.push(test._id);
}
this.emit('done', donePayload);
this._fatalErrors = [];
this._skipRemainingTestsInSuite = undefined;
this._runFinished.resolve();
}
}
private async _runTest(test: TestCase, retry: number, nextTest: TestCase | undefined) {
// Do not run tests after full cleanup, because we are entirely done.
if (this._isStopped && this._didRunFullCleanup)
return;
let lastStepId = 0;
const testInfo = new TestInfoImpl(this._loader, this._params, test, retry, data => {
const stepId = `${data.category}@${data.title}@${++lastStepId}`;
@ -254,8 +266,7 @@ export class WorkerRunner extends EventEmitter {
return;
}
// Assume beforeAll failed until we actually finish it successfully.
let didFailBeforeAll = true;
let didFailBeforeAllForSuite: Suite | undefined;
let shouldRunAfterEachHooks = false;
await testInfo._runWithTimeout(async () => {
@ -263,7 +274,7 @@ export class WorkerRunner extends EventEmitter {
// Getting here means that worker is requested to stop, but was not able to
// run full cleanup yet. Skip the test, but run the cleanup.
testInfo.status = 'skipped';
didFailBeforeAll = false;
didFailBeforeAllForSuite = undefined;
return;
}
@ -283,14 +294,18 @@ export class WorkerRunner extends EventEmitter {
continue;
const extraAnnotations: Annotation[] = [];
this._extraSuiteAnnotations.set(suite, extraAnnotations);
didFailBeforeAllForSuite = suite; // Assume failure, unless reset below.
await this._runModifiersForSuite(suite, testInfo, 'worker', extraAnnotations);
}
// Run "beforeAll" hooks, unless already run during previous tests.
for (const suite of suites)
for (const suite of suites) {
didFailBeforeAllForSuite = suite; // Assume failure, unless reset below.
await this._runBeforeAllHooksForSuite(suite, testInfo);
// Running "beforeAll" succeeded!
didFailBeforeAll = false;
}
// Running "beforeAll" succeeded for all suites!
didFailBeforeAllForSuite = undefined;
// Run "beforeEach" modifiers.
for (const suite of suites)
@ -313,11 +328,11 @@ export class WorkerRunner extends EventEmitter {
beforeHooksStep.complete(maybeError); // Second complete is a no-op.
});
if (didFailBeforeAll) {
if (didFailBeforeAllForSuite) {
// This will inform dispatcher that we should not run more tests from this group
// because we had a beforeAll error.
// This behavior avoids getting the same common error for each test.
this._skipRemainingTests = true;
this._skipRemainingTestsInSuite = didFailBeforeAllForSuite;
}
const afterHooksStep = testInfo._addStep({
@ -457,13 +472,6 @@ export class WorkerRunner extends EventEmitter {
if (error)
throw error;
}
private _reportDone() {
const donePayload: DonePayload = { fatalErrors: this._fatalErrors, skipRemaining: this._skipRemainingTests };
this.emit('done', donePayload);
this._fatalErrors = [];
this._skipRemainingTests = false;
}
}
function buildTestBeginPayload(testInfo: TestInfoImpl): TestBeginPayload {

31
tests/playwright-test/hooks.spec.ts
Просмотреть файл

@ -752,3 +752,34 @@ test('test.setTimeout should work separately in afterAll', async ({ runInlineTes
'%%afterAll',
]);
});
test('beforeAll failure should only prevent tests that are affected', async ({ runInlineTest }) => {
const result = await runInlineTest({
'a.test.js': `
const { test } = pwt;
test.describe('suite', () => {
test.beforeAll(async () => {
console.log('\\n%%beforeAll');
throw new Error('oh my');
});
test('failed', () => {
console.log('\\n%%test1');
});
test('skipped', () => {
console.log('\\n%%test2');
});
});
test('passed', () => {
console.log('\\n%%test3');
});
`,
});
expect(result.exitCode).toBe(1);
expect(result.failed).toBe(1);
expect(result.skipped).toBe(1);
expect(result.passed).toBe(1);
expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([
'%%beforeAll',
'%%test3',
]);
});

6
tests/playwright-test/playwright-test-fixtures.ts
Просмотреть файл

@ -284,10 +284,10 @@ export function createWhiteImage(width: number, height: number) {
}
export function paintBlackPixels(image: Buffer, blackPixelsCount: number): Buffer {
image = PNG.sync.read(image);
const png = PNG.sync.read(image);
for (let i = 0; i < blackPixelsCount; ++i) {
for (let j = 0; j < 3; ++j)
image.data[i * 4 + j] = 0;
png.data[i * 4 + j] = 0;
}
return PNG.sync.write(image);
return PNG.sync.write(png);
}