Skip to content

Commit

Permalink
fix: do not throw when merging into blob report (#26355)
Browse files Browse the repository at this point in the history
We cannot import a Symbol to isomorphic code from config. Instead,
__projectId property is used.
  • Loading branch information
yury-s authored Aug 8, 2023
1 parent 0f0045b commit bc2c794
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 14 deletions.
11 changes: 5 additions & 6 deletions packages/playwright-test/src/common/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ export class FullConfigInternal {
if (usedNames.has(candidate))
continue;
p.id = candidate;
(p.project as any).__projectId = p.id;
usedNames.add(candidate);
break;
}
Expand All @@ -160,10 +161,6 @@ export class FullProjectInternal {
deps: FullProjectInternal[] = [];
teardown: FullProjectInternal | undefined;

static from(config: FullProject): FullProjectInternal {
return (config as any)[projectInternalSymbol];
}

constructor(configDir: string, config: Config, fullConfig: FullConfigInternal, projectConfig: Project, configCLIOverrides: ConfigCLIOverrides, throwawayArtifactsPath: string) {
this.fullConfig = fullConfig;
const testDir = takeFirst(pathResolve(configDir, projectConfig.testDir), pathResolve(configDir, config.testDir), fullConfig.configDir);
Expand All @@ -189,7 +186,6 @@ export class FullProjectInternal {
dependencies: projectConfig.dependencies || [],
teardown: projectConfig.teardown,
};
(this.project as any)[projectInternalSymbol] = this;
this.fullyParallel = takeFirst(configCLIOverrides.fullyParallel, projectConfig.fullyParallel, config.fullyParallel, undefined);
this.expect = takeFirst(projectConfig.expect, config.expect, {});
this.respectGitIgnore = !projectConfig.testDir && !config.testDir;
Expand Down Expand Up @@ -278,4 +274,7 @@ export const defaultGrep = /.*/;
export const defaultReporter = process.env.CI ? 'dot' : 'list';

const configInternalSymbol = Symbol('configInternalSymbol');
const projectInternalSymbol = Symbol('projectInternalSymbol');

export function getProjectId(project: FullProject): string {
return (project as any).__projectId!;
}
6 changes: 3 additions & 3 deletions packages/playwright-test/src/isomorphic/teleReceiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ export class TeleReporterReceiver {
}

private _onProject(project: JsonProject) {
let projectSuite = this._rootSuite.suites.find(suite => suite.project()!.id === project.id);
let projectSuite = this._rootSuite.suites.find(suite => suite.project()!.__projectId === project.id);
if (!projectSuite) {
projectSuite = new TeleSuite(project.name, 'project');
this._rootSuite.suites.push(projectSuite);
Expand Down Expand Up @@ -323,7 +323,7 @@ export class TeleReporterReceiver {

private _parseProject(project: JsonProject): TeleFullProject {
return {
id: project.id,
__projectId: project.id,
metadata: project.metadata,
name: project.name,
outputDir: this._absolutePath(project.outputDir),
Expand Down Expand Up @@ -577,7 +577,7 @@ class TeleTestResult implements reporterTypes.TestResult {
}
}

export type TeleFullProject = FullProject & { id: string };
export type TeleFullProject = FullProject & { __projectId: string };

export const baseFullConfig: FullConfig = {
forbidOnly: false,
Expand Down
6 changes: 3 additions & 3 deletions packages/playwright-test/src/reporters/json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import type { FullConfig, TestCase, Suite, TestResult, TestError, TestStep, Full
import { formatError, prepareErrorStack } from './base';
import { MultiMap } from 'playwright-core/lib/utils';
import { assert } from 'playwright-core/lib/utils';
import { FullProjectInternal } from '../common/config';
import { getProjectId } from '../common/config';
import EmptyReporter from './empty';

export function toPosixPath(aPath: string): string {
Expand Down Expand Up @@ -69,7 +69,7 @@ class JSONReporter extends EmptyReporter {
repeatEach: project.repeatEach,
retries: project.retries,
metadata: project.metadata,
id: FullProjectInternal.from(project).id,
id: getProjectId(project),
name: project.name,
testDir: toPosixPath(project.testDir),
testIgnore: serializePatterns(project.testIgnore),
Expand All @@ -86,7 +86,7 @@ class JSONReporter extends EmptyReporter {
private _mergeSuites(suites: Suite[]): JSONReportSuite[] {
const fileSuites = new MultiMap<string, JSONReportSuite>();
for (const projectSuite of suites) {
const projectId = FullProjectInternal.from(projectSuite.project()!).id;
const projectId = getProjectId(projectSuite.project()!);
const projectName = projectSuite.project()!.name;
for (const fileSuite of projectSuite.suites) {
const file = fileSuite.location!.file;
Expand Down
4 changes: 2 additions & 2 deletions packages/playwright-test/src/reporters/teleEmitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import path from 'path';
import { createGuid } from 'playwright-core/lib/utils';
import type { SuitePrivate } from '../../types/reporterPrivate';
import type { FullConfig, FullResult, Location, TestCase, TestError, TestResult, TestStep } from '../../types/testReporter';
import { FullConfigInternal, FullProjectInternal } from '../common/config';
import { FullConfigInternal, getProjectId } from '../common/config';
import type { Suite } from '../common/test';
import type { JsonAttachment, JsonConfig, JsonEvent, JsonProject, JsonStdIOType, JsonSuite, JsonTestCase, JsonTestEnd, JsonTestResultEnd, JsonTestResultStart, JsonTestStepEnd, JsonTestStepStart } from '../isomorphic/teleReceiver';
import { serializeRegexPatterns } from '../isomorphic/teleReceiver';
Expand Down Expand Up @@ -151,7 +151,7 @@ export class TeleReporterEmitter implements ReporterV2 {
private _serializeProject(suite: Suite): JsonProject {
const project = suite.project()!;
const report: JsonProject = {
id: FullProjectInternal.from(project).id,
id: getProjectId(project),
metadata: project.metadata,
name: project.name,
outputDir: this._relativePath(project.outputDir),
Expand Down
56 changes: 56 additions & 0 deletions tests/playwright-test/reporter-blob.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,62 @@ test('should merge into html with dependencies', async ({ runInlineTest, mergeRe
}
});

test('should merge blob into blob', async ({ runInlineTest, mergeReports, showReport, page }) => {
const reportDir = test.info().outputPath('blob-report-orig');
const files = {
'playwright.config.ts': `
module.exports = {
retries: 1,
reporter: [['blob', { outputDir: '${reportDir.replace(/\\/g, '/')}' }]]
};
`,
'a.test.js': `
import { test, expect } from '@playwright/test';
test('math 1', async ({}) => {
expect(1 + 1).toBe(2);
});
test('failing 1', async ({}) => {
expect(1).toBe(2);
});
test('flaky 1', async ({}) => {
expect(test.info().retry).toBe(1);
});
test.skip('skipped 1', async ({}) => {});
`,
'b.test.js': `
import { test, expect } from '@playwright/test';
test('math 2', async ({}) => {
expect(1 + 1).toBe(2);
});
test('failing 2', async ({}) => {
expect(1).toBe(2);
});
test.skip('skipped 2', async ({}) => {});
`
};
await runInlineTest(files, { shard: `1/2` }, { PWTEST_BLOB_DO_NOT_REMOVE: '1' });
await runInlineTest(files, { shard: `2/2` }, { PWTEST_BLOB_DO_NOT_REMOVE: '1' });
{
const reportFiles = await fs.promises.readdir(reportDir);
reportFiles.sort();
expect(reportFiles).toEqual(['report-1.zip', 'report-2.zip']);
const { exitCode } = await mergeReports(reportDir, undefined, { additionalArgs: ['--reporter', 'blob'] });
expect(exitCode).toBe(0);
}
{
const compinedBlobReportDir = test.info().outputPath('blob-report');
const { exitCode } = await mergeReports(compinedBlobReportDir, { 'PW_TEST_HTML_REPORT_OPEN': 'never' }, { additionalArgs: ['--reporter', 'html,json'] });
expect(exitCode).toBe(0);
expect(fs.existsSync(test.info().outputPath('report.json'))).toBe(true);
await showReport();
await expect(page.locator('.subnav-item:has-text("All") .counter')).toHaveText('7');
await expect(page.locator('.subnav-item:has-text("Passed") .counter')).toHaveText('2');
await expect(page.locator('.subnav-item:has-text("Failed") .counter')).toHaveText('2');
await expect(page.locator('.subnav-item:has-text("Flaky") .counter')).toHaveText('1');
await expect(page.locator('.subnav-item:has-text("Skipped") .counter')).toHaveText('2');
}
});

test('be able to merge incomplete shards', async ({ runInlineTest, mergeReports, showReport, page }) => {
const reportDir = test.info().outputPath('blob-report');
const files = {
Expand Down

0 comments on commit bc2c794

Please sign in to comment.