From bc2c7946bb495a93dc73dd80547f9b539bbb735e Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Tue, 8 Aug 2023 14:47:12 -0700 Subject: [PATCH] fix: do not throw when merging into blob report (#26355) We cannot import a Symbol to isomorphic code from config. Instead, __projectId property is used. --- packages/playwright-test/src/common/config.ts | 11 ++-- .../src/isomorphic/teleReceiver.ts | 6 +- .../playwright-test/src/reporters/json.ts | 6 +- .../src/reporters/teleEmitter.ts | 4 +- tests/playwright-test/reporter-blob.spec.ts | 56 +++++++++++++++++++ 5 files changed, 69 insertions(+), 14 deletions(-) diff --git a/packages/playwright-test/src/common/config.ts b/packages/playwright-test/src/common/config.ts index 754e276bc6..5a560b4550 100644 --- a/packages/playwright-test/src/common/config.ts +++ b/packages/playwright-test/src/common/config.ts @@ -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; } @@ -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); @@ -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; @@ -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!; +} \ No newline at end of file diff --git a/packages/playwright-test/src/isomorphic/teleReceiver.ts b/packages/playwright-test/src/isomorphic/teleReceiver.ts index e7ffd6ba53..e87698344c 100644 --- a/packages/playwright-test/src/isomorphic/teleReceiver.ts +++ b/packages/playwright-test/src/isomorphic/teleReceiver.ts @@ -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); @@ -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), @@ -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, diff --git a/packages/playwright-test/src/reporters/json.ts b/packages/playwright-test/src/reporters/json.ts index 6ba7d48801..1a20225812 100644 --- a/packages/playwright-test/src/reporters/json.ts +++ b/packages/playwright-test/src/reporters/json.ts @@ -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 { @@ -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), @@ -86,7 +86,7 @@ class JSONReporter extends EmptyReporter { private _mergeSuites(suites: Suite[]): JSONReportSuite[] { const fileSuites = new MultiMap(); 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; diff --git a/packages/playwright-test/src/reporters/teleEmitter.ts b/packages/playwright-test/src/reporters/teleEmitter.ts index 6a38f0d7f8..04e41102c6 100644 --- a/packages/playwright-test/src/reporters/teleEmitter.ts +++ b/packages/playwright-test/src/reporters/teleEmitter.ts @@ -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'; @@ -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), diff --git a/tests/playwright-test/reporter-blob.spec.ts b/tests/playwright-test/reporter-blob.spec.ts index 5833add035..1452037e1a 100644 --- a/tests/playwright-test/reporter-blob.spec.ts +++ b/tests/playwright-test/reporter-blob.spec.ts @@ -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 = {