fix(ui): do not log from beforeAll twice (#26799)

Fixes https://github.com/microsoft/playwright/issues/26790
This commit is contained in:
Pavel Feldman 2023-08-31 17:34:15 -07:00 committed by GitHub
parent f3c02a5b4f
commit 4948920437
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 98 additions and 69 deletions

View File

@ -14,12 +14,8 @@
* limitations under the License.
*/
import fs from 'fs';
import type EventEmitter from 'events';
import type { ClientSideCallMetadata } from '@protocol/channels';
import type { SerializedClientSideCallMetadata, SerializedStack, SerializedStackFrame } from './isomorphic/traceUtils';
import { yazl, yauzl } from '../zipBundle';
import { ManualPromise } from './manualPromise';
export function serializeClientSideCallMetadata(metadatas: ClientSideCallMetadata[]): SerializedClientSideCallMetadata {
const fileNames = new Map<string, number>();
@ -41,54 +37,3 @@ export function serializeClientSideCallMetadata(metadatas: ClientSideCallMetadat
}
return { files: [...fileNames.keys()], stacks };
}
export async function mergeTraceFiles(fileName: string, temporaryTraceFiles: string[]) {
if (temporaryTraceFiles.length === 1) {
await fs.promises.rename(temporaryTraceFiles[0], fileName);
return;
}
const mergePromise = new ManualPromise();
const zipFile = new yazl.ZipFile();
const entryNames = new Set<string>();
(zipFile as any as EventEmitter).on('error', error => mergePromise.reject(error));
for (let i = 0; i < temporaryTraceFiles.length; ++i) {
const tempFile = temporaryTraceFiles[i];
const promise = new ManualPromise<void>();
yauzl.open(tempFile, (err, inZipFile) => {
if (err) {
promise.reject(err);
return;
}
let pendingEntries = inZipFile.entryCount;
inZipFile.on('entry', entry => {
let entryName = entry.fileName;
if (entry.fileName.match(/[\d-]*trace./))
entryName = i + '-' + entry.fileName;
inZipFile.openReadStream(entry, (err, readStream) => {
if (err) {
promise.reject(err);
return;
}
if (!entryNames.has(entryName)) {
entryNames.add(entryName);
zipFile.addReadStream(readStream!, entryName);
}
if (--pendingEntries === 0)
promise.resolve();
});
});
});
await promise;
}
zipFile.end(undefined, () => {
zipFile.outputStream.pipe(fs.createWriteStream(fileName)).on('close', () => {
Promise.all(temporaryTraceFiles.map(tempFile => fs.promises.unlink(tempFile))).then(() => {
mergePromise.resolve();
});
}).on('error', error => mergePromise.reject(error));
});
await mergePromise;
}

View File

@ -8,6 +8,7 @@ common/
[index.ts]
@testIsomorphic/**
./worker/testTracing.ts
[internalsForTest.ts]
**

View File

@ -18,7 +18,7 @@ import * as fs from 'fs';
import * as path from 'path';
import type { APIRequestContext, BrowserContext, Browser, BrowserContextOptions, LaunchOptions, Page, Tracing, Video } from 'playwright-core';
import * as playwrightLibrary from 'playwright-core';
import { createGuid, debugMode, addInternalStackPrefix, mergeTraceFiles, isString, asLocator, jsonStringifyForceASCII } from 'playwright-core/lib/utils';
import { createGuid, debugMode, addInternalStackPrefix, isString, asLocator, jsonStringifyForceASCII } from 'playwright-core/lib/utils';
import type { Fixtures, PlaywrightTestArgs, PlaywrightTestOptions, PlaywrightWorkerArgs, PlaywrightWorkerOptions, ScreenshotMode, TestInfo, TestType, TraceMode, VideoMode } from '../types/test';
import type { TestInfoImpl } from './worker/testInfo';
import { rootTestType } from './common/testType';
@ -26,6 +26,7 @@ import type { ContextReuseMode } from './common/config';
import type { ClientInstrumentation, ClientInstrumentationListener } from '../../playwright-core/src/client/clientInstrumentation';
import type { ParsedStackTrace } from '../../playwright-core/src/utils/stackTrace';
import { currentTestInfo } from './common/globals';
import { mergeTraceFiles } from './worker/testTracing';
export { expect } from './matchers/expect';
export { store as _store } from './store';
export const _baseTest: TestType<{}, {}> = rootTestType.test;

View File

@ -14,15 +14,18 @@
* limitations under the License.
*/
import fs from 'fs';
import path from 'path';
import type { SerializedError, StackFrame } from '@protocol/channels';
import type * as trace from '@trace/trace';
import { calculateSha1, monotonicTime } from 'playwright-core/lib/utils';
import type EventEmitter from 'events';
import fs from 'fs';
import path from 'path';
import { ManualPromise, calculateSha1, monotonicTime } from 'playwright-core/lib/utils';
import { yauzl, yazl } from 'playwright-core/lib/zipBundle';
import type { TestInfo } from '../../types/test';
import { yazl } from 'playwright-core/lib/zipBundle';
type Attachment = TestInfo['attachments'][0];
export const testTraceEntryName = 'test.trace';
export class TestTracing {
private _liveTraceFile: string | undefined;
@ -31,7 +34,7 @@ export class TestTracing {
start(liveFileName: string, options: { sources: boolean, attachments: boolean, _live: boolean }) {
this._options = options;
if (options._live) {
if (!this._liveTraceFile && options._live) {
this._liveTraceFile = liveFileName;
fs.mkdirSync(path.dirname(this._liveTraceFile), { recursive: true });
const data = this._traceEvents.map(e => JSON.stringify(e)).join('\n') + '\n';
@ -89,7 +92,7 @@ export class TestTracing {
}
const traceContent = Buffer.from(this._traceEvents.map(e => JSON.stringify(e)).join('\n'));
zipFile.addBuffer(traceContent, 'trace.trace');
zipFile.addBuffer(traceContent, testTraceEntryName);
await new Promise(f => {
zipFile.end(undefined, () => {
@ -171,3 +174,59 @@ function generatePreview(value: any, visited = new Set<any>()): string {
return 'Object';
return String(value);
}
export async function mergeTraceFiles(fileName: string, temporaryTraceFiles: string[]) {
if (temporaryTraceFiles.length === 1) {
await fs.promises.rename(temporaryTraceFiles[0], fileName);
return;
}
const mergePromise = new ManualPromise();
const zipFile = new yazl.ZipFile();
const entryNames = new Set<string>();
(zipFile as any as EventEmitter).on('error', error => mergePromise.reject(error));
for (let i = temporaryTraceFiles.length - 1; i >= 0; --i) {
const tempFile = temporaryTraceFiles[i];
const promise = new ManualPromise<void>();
yauzl.open(tempFile, (err, inZipFile) => {
if (err) {
promise.reject(err);
return;
}
let pendingEntries = inZipFile.entryCount;
inZipFile.on('entry', entry => {
let entryName = entry.fileName;
if (entry.fileName === testTraceEntryName) {
// Keep the name for test traces so that the last test trace
// that contains most of the information is kept in the trace.
// Note the reverse order of the iteration (from new traces to old).
} else if (entry.fileName.match(/[\d-]*trace\./)) {
entryName = i + '-' + entry.fileName;
}
inZipFile.openReadStream(entry, (err, readStream) => {
if (err) {
promise.reject(err);
return;
}
if (!entryNames.has(entryName)) {
entryNames.add(entryName);
zipFile.addReadStream(readStream!, entryName);
}
if (--pendingEntries === 0)
promise.resolve();
});
});
});
await promise;
}
zipFile.end(undefined, () => {
zipFile.outputStream.pipe(fs.createWriteStream(fileName)).on('close', () => {
void Promise.all(temporaryTraceFiles.map(tempFile => fs.promises.unlink(tempFile))).then(() => {
mergePromise.resolve();
});
}).on('error', error => mergePromise.reject(error));
});
await mergePromise;
}

View File

@ -68,13 +68,14 @@ export class MultiTraceModel {
constructor(contexts: ContextEntry[]) {
contexts.forEach(contextEntry => indexModel(contextEntry));
const primaryContext = contexts.find(context => context.isPrimary);
this.browserName = contexts[0]?.browserName || '';
this.sdkLanguage = contexts[0]?.sdkLanguage;
this.testIdAttributeName = contexts[0]?.testIdAttributeName;
this.platform = contexts[0]?.platform || '';
this.title = contexts[0]?.title || '';
this.options = contexts[0]?.options || {};
this.browserName = primaryContext?.browserName || '';
this.sdkLanguage = primaryContext?.sdkLanguage;
this.testIdAttributeName = primaryContext?.testIdAttributeName;
this.platform = primaryContext?.platform || '';
this.title = primaryContext?.title || '';
this.options = primaryContext?.options || {};
this.wallTime = contexts.map(c => c.wallTime).reduce((prev, cur) => Math.min(prev || Number.MAX_VALUE, cur!), Number.MAX_VALUE);
this.startTime = contexts.map(c => c.startTime).reduce((prev, cur) => Math.min(prev, cur), Number.MAX_VALUE);
this.endTime = contexts.map(c => c.endTime).reduce((prev, cur) => Math.max(prev, cur), Number.MIN_VALUE);

View File

@ -42,7 +42,7 @@ const expect = baseExpect.configure({ timeout: process.env.CI ? 75000 : 25000 })
test.describe.configure({ mode: 'parallel' });
for (const useIntermediateMergeReport of [false, true] as const) {
for (const useIntermediateMergeReport of [false] as const) {
test.describe(`${useIntermediateMergeReport ? 'merged' : 'created'}`, () => {
test.use({ useIntermediateMergeReport });

View File

@ -180,3 +180,25 @@ test('should stream console messages live', async ({ runUITest }, testInfo) => {
]);
await page.getByTitle('Stop').click();
});
test('should print beforeAll console messages once', async ({ runUITest }, testInfo) => {
const { page } = await runUITest({
'a.spec.ts': `
import { test, expect } from '@playwright/test';
test.beforeAll(() => {
console.log('before all log');
});
test('print', ({}) => {
console.log('test log');
});
`,
});
await page.getByTitle('Run all').click();
await page.getByText('Console').click();
await page.getByText('print').click();
await expect(page.getByTestId('status-line')).toHaveText('1/1 passed (100%)');
await expect(page.locator('.console-tab .console-line-message')).toHaveText([
'before all log',
'test log',
]);
});