Skip to content

Commit

Permalink
fix(merge): preserve original "rootDir" by default (#27963)
Browse files Browse the repository at this point in the history
Fixes #27877
  • Loading branch information
yury-s authored Nov 3, 2023
1 parent 738cbfc commit 53a78a3
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 9 deletions.
3 changes: 3 additions & 0 deletions docs/src/test-sharding-js.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ You can now see the reports have been merged and a combined HTML report is avail

`npx playwright merge-reports path/to/blob-reports-dir` reads all blob reports from the passed directory and merges them into a single report.

When merging reports from different OS'es you'll have to provide an explicit merge config to disambiguate which directory should be used as tests root.

Supported options:
- `--reporter reporter-to-use`

Expand All @@ -166,6 +168,7 @@ Supported options:

```ts title="merge.config.ts"
export default {
testDir: 'e2e',
reporter: [['html', { open: 'never' }]],
};
```
3 changes: 2 additions & 1 deletion packages/playwright/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,8 @@ async function mergeReports(reportDir: string | undefined, opts: { [key: string]
reporterDescriptions = config.config.reporter;
if (!reporterDescriptions)
reporterDescriptions = [[defaultReporter]];
await createMergedReport(config, dir, reporterDescriptions!);
const rootDirOverride = configFile ? config.config.rootDir : undefined;
await createMergedReport(config, dir, reporterDescriptions!, rootDirOverride);
}

function overridesFromOptions(options: { [key: string]: any }): ConfigCLIOverrides {
Expand Down
3 changes: 1 addition & 2 deletions packages/playwright/src/isomorphic/teleReceiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ export class TeleReporterReceiver {
}

private _onConfigure(config: JsonConfig) {
this._rootDir = this._reportConfig?.rootDir || config.rootDir;
this._rootDir = config.rootDir;
this._listOnly = config.listOnly;
this._config = this._parseConfig(config);
this._reporter.onConfigure?.(this._config);
Expand Down Expand Up @@ -324,7 +324,6 @@ export class TeleReporterReceiver {
const result = { ...baseFullConfig, ...config };
if (this._reportConfig) {
result.configFile = this._reportConfig.configFile;
result.rootDir = this._reportConfig.rootDir;
result.reportSlowTests = this._reportConfig.reportSlowTests;
result.quiet = this._reportConfig.quiet;
result.reporter = [...this._reportConfig.reporter];
Expand Down
32 changes: 27 additions & 5 deletions packages/playwright/src/reporters/merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ type ReportData = {
reportFile: string;
};

export async function createMergedReport(config: FullConfigInternal, dir: string, reporterDescriptions: ReporterDescription[]) {
export async function createMergedReport(config: FullConfigInternal, dir: string, reporterDescriptions: ReporterDescription[], rootDirOverride: string | undefined) {
const reporters = await createReporters(config, 'merge', reporterDescriptions);
const multiplexer = new Multiplexer(reporters);
const receiver = new TeleReporterReceiver(path.sep, multiplexer, false, config.config);
Expand All @@ -49,7 +49,7 @@ export async function createMergedReport(config: FullConfigInternal, dir: string
const shardFiles = await sortedShardFiles(dir);
if (shardFiles.length === 0)
throw new Error(`No report files found in ${dir}`);
const eventData = await mergeEvents(dir, shardFiles, stringPool, printStatus);
const eventData = await mergeEvents(dir, shardFiles, stringPool, printStatus, rootDirOverride);
printStatus(`processing test events`);

const dispatchEvents = async (events: JsonEvent[]) => {
Expand Down Expand Up @@ -148,7 +148,7 @@ function findMetadata(events: JsonEvent[], file: string): BlobReportMetadata {
return metadata;
}

async function mergeEvents(dir: string, shardReportFiles: string[], stringPool: StringInternPool, printStatus: StatusCallback) {
async function mergeEvents(dir: string, shardReportFiles: string[], stringPool: StringInternPool, printStatus: StatusCallback, rootDirOverride: string | undefined) {
const internalizer = new JsonStringInternalizer(stringPool);

const configureEvents: JsonEvent[] = [];
Expand Down Expand Up @@ -207,7 +207,7 @@ async function mergeEvents(dir: string, shardReportFiles: string[], stringPool:

return {
prologue: [
mergeConfigureEvents(configureEvents),
mergeConfigureEvents(configureEvents, rootDirOverride),
...projectEvents,
{ method: 'onBegin', params: undefined },
],
Expand All @@ -219,7 +219,7 @@ async function mergeEvents(dir: string, shardReportFiles: string[], stringPool:
};
}

function mergeConfigureEvents(configureEvents: JsonEvent[]): JsonEvent {
function mergeConfigureEvents(configureEvents: JsonEvent[], rootDirOverride: string | undefined): JsonEvent {
if (!configureEvents.length)
throw new Error('No configure events found');
let config: JsonConfig = {
Expand All @@ -235,6 +235,28 @@ function mergeConfigureEvents(configureEvents: JsonEvent[]): JsonEvent {
};
for (const event of configureEvents)
config = mergeConfigs(config, event.params.config);

if (rootDirOverride) {
config.rootDir = rootDirOverride;
} else {
const rootDirs = new Set(configureEvents.map(e => e.params.config.rootDir));
if (rootDirs.size > 1) {
throw new Error([
`Blob reports being merged were recorded with different test directories, and`,
`merging cannot proceed. This may happen if you are merging reports from`,
`machines with different environments, like different operating systems or`,
`if the tests ran with different playwright configs.`,
``,
`You can force merge by specifying a merge config file with "-c" option. If`,
`you'd like all test paths to be correct, make sure 'testDir' in the merge config`,
`file points to the actual tests location.`,
``,
`Found directories:`,
...rootDirs
].join('\n'));
}
}

return {
method: 'onConfigure',
params: {
Expand Down
98 changes: 97 additions & 1 deletion tests/playwright-test/reporter-blob.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ test('generate html with attachment urls', async ({ runInlineTest, mergeReports,

const htmlReportDir = test.info().outputPath('playwright-report');
for (const entry of await fs.promises.readdir(htmlReportDir))
await (fs.promises as any).cp(path.join(htmlReportDir, entry), path.join(reportDir, entry), { recursive: true });
await fs.promises.cp(path.join(htmlReportDir, entry), path.join(reportDir, entry), { recursive: true });

const oldSeveFile = server.serveFile;
server.serveFile = async (req, res) => {
Expand Down Expand Up @@ -1416,3 +1416,99 @@ test('reporter list in the custom config', async ({ runInlineTest, mergeReports
expect(text).toContain('testReporter.js');
expect(text).toContain('{ myOpt: 1 }');
});

test('merge reports with different rootDirs', async ({ runInlineTest, mergeReports }) => {
test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/27877' });
const files1 = {
'echo-reporter.js': `
export default class EchoReporter {
onBegin(config, suite) {
console.log('rootDir:', config.rootDir);
}
onTestBegin(test) {
console.log('test:', test.location.file);
}
};
`,
'merge.config.ts': `module.exports = {
testDir: 'mergeRoot',
reporter: './echo-reporter.js'
};`,
'dir1/playwright.config.ts': `module.exports = {
testDir: 'tests1',
reporter: [['blob', { outputDir: 'blob-report' }]]
};`,
'dir1/tests1/a.test.js': `
import { test, expect } from '@playwright/test';
test('math 1', async ({}) => { });
`,
};
await runInlineTest(files1, { workers: 1 }, undefined, { additionalArgs: ['--config', test.info().outputPath('dir1/playwright.config.ts')] });

const files2 = {
'dir2/playwright.config.ts': `module.exports = {
reporter: [['blob', { outputDir: 'blob-report' }]]
};`,
'dir2/tests2/b.test.js': `
import { test, expect } from '@playwright/test';
test('math 2', async ({}) => { });
`,
};
await runInlineTest(files2, { workers: 1 }, undefined, { additionalArgs: ['--config', test.info().outputPath('dir2/playwright.config.ts')] });

const allReportsDir = test.info().outputPath('all-blob-reports');
await fs.promises.cp(test.info().outputPath('dir1', 'blob-report', 'report.zip'), path.join(allReportsDir, 'report-1.zip'));
await fs.promises.cp(test.info().outputPath('dir2', 'blob-report', 'report.zip'), path.join(allReportsDir, 'report-2.zip'));

{
const { exitCode, output } = await mergeReports(allReportsDir);
expect(exitCode).toBe(1);
expect(output).toContain(`Blob reports being merged were recorded with different test directories`);
}

{
const { exitCode, output } = await mergeReports(allReportsDir, undefined, { additionalArgs: ['--config', 'merge.config.ts'] });
expect(exitCode).toBe(0);
expect(output).toContain(`rootDir: ${test.info().outputPath('mergeRoot')}`);
expect(output).toContain(`test: ${test.info().outputPath('mergeRoot', 'a.test.js')}`);
expect(output).toContain(`test: ${test.info().outputPath('mergeRoot', 'tests2', 'b.test.js')}`);
}
});

test('merge reports same rootDirs', async ({ runInlineTest, mergeReports }) => {
test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/27877' });
const files = {
'echo-reporter.js': `
export default class EchoReporter {
onBegin(config, suite) {
console.log('rootDir:', config.rootDir);
}
onTestBegin(test) {
console.log('test:', test.location.file);
}
};
`,
'playwright.config.ts': `module.exports = {
testDir: 'tests',
reporter: [['blob', { outputDir: 'blob-report' }]]
};`,
'tests/dir1/a.test.js': `
import { test, expect } from '@playwright/test';
test('math 1', async ({}) => { });
`,
'tests/dir2/b.test.js': `
import { test, expect } from '@playwright/test';
test('math 2', async ({}) => { });
`,
};
await runInlineTest(files, { shard: `1/2` });
await runInlineTest(files, { shard: `2/2` }, { PWTEST_BLOB_DO_NOT_REMOVE: '1' });

const allReportsDir = test.info().outputPath('blob-report');

const { exitCode, output } = await mergeReports(allReportsDir, undefined, { additionalArgs: ['--reporter', './echo-reporter.js'] });
expect(exitCode).toBe(0);
expect(output).toContain(`rootDir: ${test.info().outputPath('tests')}`);
expect(output).toContain(`test: ${test.info().outputPath('tests', 'dir1', 'a.test.js')}`);
expect(output).toContain(`test: ${test.info().outputPath('tests', 'dir2', 'b.test.js')}`);
});

0 comments on commit 53a78a3

Please sign in to comment.