Skip to content

feat: support sharding in HTML Reporter #19691

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.

Sign up for GitHub

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

Closed
kevin940726 wants to merge 2 commits into from
Closed

feat: support sharding in HTML Reporter #19691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5364c0a
feat: supports sharding in HTML Reporter
kevin940726 Dec 23, 2022
8e29b22
fix: file:// protocol warning
kevin940726 Dec 24, 2022
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
7 changes: 7 additions & 0 deletions packages/html-reporter/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,12 @@
<body>
<div id='root'></div>
<script type='module' src='/src/index.tsx'></script>
<dialog id="fallback-error">
Copy link
Member

@pavelfeldman pavelfeldman Jan 10, 2023

Choose a reason for hiding this comment

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

Let's remove this: we should be able to open it from file:// - it was a strong requirement from the community.

<p>The Playwright Test Report must be loaded over the <code>http://</code> or <code>https://</code> protocols.</p>
</dialog>
<script>
if (!/^https?:/.test(window.location.protocol))
document.getElementById("fallback-error").show();
</script>
</body>
</html>
26 changes: 22 additions & 4 deletions packages/html-reporter/src/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import * as ReactDOM from 'react-dom';
import './colors.css';
import type { LoadedReport } from './loadedReport';
import { ReportView } from './reportView';
import { mergeReports } from './mergeReports';
// @ts-ignore
const zipjs = zipImport as typeof zip;

Expand All @@ -46,10 +47,27 @@ class ZipReport implements LoadedReport {
private _json!: HTMLReport;

async load() {
const zipReader = new zipjs.ZipReader(new zipjs.Data64URIReader((window as any).playwrightReportBase64), { useWebWorkers: false }) as zip.ZipReader;
for (const entry of await zipReader.getEntries())
this._entries.set(entry.filename, entry);
this._json = await this.entry('report.json') as HTMLReport;
const metadata = (window as any).playwrightMetadata;
if (metadata.shard) {
const reports: HTMLReport[] = [];
for (let index = 1; index <= metadata.shard.total; index += 1) {
try {
const zipReader = new zipjs.ZipReader(new zipjs.HttpReader(`/report/report-${index}.zip`), { useWebWorkers: false }) as zip.ZipReader;
Copy link
Member

@pavelfeldman pavelfeldman Jan 10, 2023

Choose a reason for hiding this comment

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

Let's keep those files siblings in order to be less intrusive, hard-code less names. So it'll be

new zipjs.HttpReader(`report-${index}.zip`)

for (const entry of await zipReader.getEntries())
Copy link
Member

@pavelfeldman pavelfeldman Jan 10, 2023

Choose a reason for hiding this comment

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

Looks like you could benefit from fetching them all concurrently instead.

this._entries.set(entry.filename, entry);
const currentJson = await this.entry(`report-${index}.json`) as HTMLReport;
reports.push(currentJson);
} catch (error) {
// Ignore not found error for viewing individual shard report.
Copy link
Member

@pavelfeldman pavelfeldman Jan 10, 2023

Choose a reason for hiding this comment

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

Let's make sure these errors end up in devtools so that it was clear what is going on.

}
}
this._json = mergeReports(reports);
} else {
const zipReader = new zipjs.ZipReader(new zipjs.HttpReader('/report/report.zip'), { useWebWorkers: false }) as zip.ZipReader;
Copy link
Member

@pavelfeldman pavelfeldman Jan 10, 2023

Choose a reason for hiding this comment

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

Having it all in one file is a strong requirement, let's keep it as is. Loading shards from http(s) is Ok though.

for (const entry of await zipReader.getEntries())
this._entries.set(entry.filename, entry);
this._json = await this.entry('report.json') as HTMLReport;
}
}

json(): HTMLReport {
Expand Down
47 changes: 47 additions & 0 deletions packages/html-reporter/src/mergeReports.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* Copyright (c) Microsoft Corporation.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import type { HTMLReport, Stats } from './types';

export function mergeReports(reports: HTMLReport[]): HTMLReport {
const [report, ...rest] = reports;

for (const currentReport of rest) {
currentReport.files.forEach(file => {
const existingGroup = report.files.find(({ fileId }) => fileId === file.fileId);

if (existingGroup) {
existingGroup.tests.push(...file.tests);
mergeStats(existingGroup.stats, file.stats);
} else {
report.files.push(file);
}
});

mergeStats(report.stats, currentReport.stats);
}

return report;
}

function mergeStats(stats: Stats, sourceStats: Stats) {
stats.total += sourceStats.total;
stats.expected += sourceStats.expected;
stats.unexpected += sourceStats.unexpected;
stats.flaky += sourceStats.flaky;
stats.skipped += sourceStats.skipped;
stats.duration += sourceStats.duration;
stats.ok = stats.ok && sourceStats.ok;
}
47 changes: 12 additions & 35 deletions packages/playwright-test/src/reporters/html.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ import { colors } from 'playwright-core/lib/utilsBundle';
import fs from 'fs';
import { open } from '../utilsBundle';
import path from 'path';
import type { TransformCallback } from 'stream';
import { Transform } from 'stream';
import type { FullConfig, Suite } from '../../types/testReporter';
import { HttpServer } from 'playwright-core/lib/utils/httpServer';
import { assert, calculateSha1, monotonicTime } from 'playwright-core/lib/utils';
Expand Down Expand Up @@ -103,6 +101,7 @@ class HtmlReporter implements ReporterInternal {

async onEnd() {
const duration = monotonicTime() - this._montonicStartTime;
const shard = this.config.shard;
const projectSuites = this.suite.suites;
const reports = projectSuites.map(suite => {
const rawReporter = new RawReporter();
Expand All @@ -111,7 +110,7 @@ class HtmlReporter implements ReporterInternal {
});
await removeFolders([this._outputFolder]);
const builder = new HtmlBuilder(this._outputFolder);
this._buildResult = await builder.build({ ...this.config.metadata, duration }, reports);
this._buildResult = await builder.build({ ...this.config.metadata, duration, shard }, reports);
}

async _onExit() {
Expand Down Expand Up @@ -204,7 +203,7 @@ class HtmlBuilder {
this._dataZipFile = new yazl.ZipFile();
}

async build(metadata: Metadata & { duration: number }, rawReports: JsonReport[]): Promise<{ ok: boolean, singleTestId: string | undefined }> {
async build(metadata: Metadata & { duration: number, shard: FullConfigInternal['shard'] }, rawReports: JsonReport[]): Promise<{ ok: boolean, singleTestId: string | undefined }> {

const data = new Map<string, { testFile: TestFile, testFileSummary: TestFileSummary }>();
for (const projectJson of rawReports) {
Expand Down Expand Up @@ -271,7 +270,8 @@ class HtmlBuilder {
return w2 - w1;
});

this._addDataFile('report.json', htmlReport);
const reportName = metadata.shard ? `report-${metadata.shard.current}` : 'report';
this._addDataFile(`${reportName}.json`, htmlReport);

// Copy app.
const appFolder = path.join(require.resolve('playwright-core'), '..', 'lib', 'webpack', 'htmlReport');
Expand All @@ -289,17 +289,19 @@ class HtmlBuilder {
}
}

// Inline report data.
// Inline metadata.
const indexFile = path.join(this._reportFolder, 'index.html');
fs.appendFileSync(indexFile, '<script>\nwindow.playwrightReportBase64 = "data:application/zip;base64,');
fs.appendFileSync(indexFile, `<script>\nwindow.playwrightMetadata = ${JSON.stringify(metadata)}\n</script>`);
Copy link
Member

@pavelfeldman pavelfeldman Jan 10, 2023

Choose a reason for hiding this comment

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

We would need both, the new data and the base64 placeholder.


// Write report data.
const reportFolder = path.join(this._reportFolder, 'report');
fs.mkdirSync(reportFolder, { recursive: true });
await new Promise(f => {
this._dataZipFile!.end(undefined, () => {
this._dataZipFile!.outputStream
.pipe(new Base64Encoder())
.pipe(fs.createWriteStream(indexFile, { flags: 'a' })).on('close', f);
.pipe(fs.createWriteStream(path.join(reportFolder, `${reportName}.zip`))).on('close', f);
});
});
fs.appendFileSync(indexFile, '";</script>');

let singleTestId: string | undefined;
if (htmlReport.stats.total === 1) {
Expand Down Expand Up @@ -485,31 +487,6 @@ const addStats = (stats: Stats, delta: Stats): Stats => {
return stats;
};

class Base64Encoder extends Transform {
private _remainder: Buffer | undefined;

override _transform(chunk: any, encoding: BufferEncoding, callback: TransformCallback): void {
if (this._remainder) {
chunk = Buffer.concat([this._remainder, chunk]);
this._remainder = undefined;
}

const remaining = chunk.length % 3;
if (remaining) {
this._remainder = chunk.slice(chunk.length - remaining);
chunk = chunk.slice(0, chunk.length - remaining);
}
chunk = chunk.toString('base64');
this.push(Buffer.from(chunk));
callback();
}

override _flush(callback: TransformCallback): void {
if (this._remainder)
this.push(Buffer.from(this._remainder.toString('base64')));
callback();
}
}

function isTextContentType(contentType: string) {
return contentType.startsWith('text/') || contentType.startsWith('application/json');
Expand Down
1 change: 0 additions & 1 deletion tests/playwright-test/reporter-html.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,6 @@ test('should warn user when viewing via file:// protocol', async ({ runInlineTes
await test.step('view via local file://', async () => {
const reportFolder = testInfo.outputPath('playwright-report');
await page.goto(url.pathToFileURL(path.join(reportFolder, 'index.html')).toString());
await page.locator('[title="View trace"]').click();
await expect(page.locator('dialog')).toBeVisible();
await expect(page.locator('dialog')).toContainText('must be loaded over');
});
Expand Down