From 06c7f1fb6c0a595e215858a6fc6de1230d48e1ae Mon Sep 17 00:00:00 2001 From: Ross Wollman Date: Thu, 22 Dec 2022 18:57:55 -0500 Subject: [PATCH] fix(html): test and fix reporter timing display (#19670) #19576 introduced a regression where the CLI reporters displayed some times with way too many decimals. e.g. 7.123456789ms. Prior to #19576, there were two monotonicTime implementations; that PR updated the reporters to use the common definition that had existed in utils.ts. However, that introduced a regression in the base.ts reporters which used the ms duration humanizing package which did not account for the more precise decimals used by the shared monotonicTime function. This fix removes the dependency on the third-party ms package and now consistently uses Pavel's humanize function which the HTML reporter had been using and proved to have better defaults for decimals. Additionally, we add more test coverage to limit future regressions since this was caught in passing. Closes #19556. Relates #19576. --- .../bundles/utils/package-lock.json | 2 -- .../bundles/utils/package.json | 2 -- .../bundles/utils/src/utilsBundleImpl.ts | 3 --- packages/playwright-core/src/utilsBundle.ts | 27 ++++++++++++++++++- packages/web/src/uiUtils.ts | 2 +- tests/playwright-test/reporter-base.spec.ts | 12 +++++++++ tests/playwright-test/reporter-html.spec.ts | 2 +- 7 files changed, 40 insertions(+), 10 deletions(-) diff --git a/packages/playwright-core/bundles/utils/package-lock.json b/packages/playwright-core/bundles/utils/package-lock.json index dedbc4fd01..dce6ac47e1 100644 --- a/packages/playwright-core/bundles/utils/package-lock.json +++ b/packages/playwright-core/bundles/utils/package-lock.json @@ -16,7 +16,6 @@ "jpeg-js": "0.4.4", "mime": "^3.0.0", "minimatch": "^3.1.2", - "ms": "^2.1.2", "pngjs": "6.0.0", "progress": "2.0.3", "proper-lockfile": "4.1.2", @@ -31,7 +30,6 @@ "@types/glob": "^7.2.0", "@types/mime": "^2.0.3", "@types/minimatch": "^3.0.5", - "@types/ms": "^0.7.31", "@types/pngjs": "^6.0.1", "@types/progress": "^2.0.5", "@types/proper-lockfile": "^4.1.2", diff --git a/packages/playwright-core/bundles/utils/package.json b/packages/playwright-core/bundles/utils/package.json index 82ebb7714c..98bb30e8af 100644 --- a/packages/playwright-core/bundles/utils/package.json +++ b/packages/playwright-core/bundles/utils/package.json @@ -17,7 +17,6 @@ "jpeg-js": "0.4.4", "mime": "^3.0.0", "minimatch": "^3.1.2", - "ms": "^2.1.2", "pngjs": "6.0.0", "progress": "2.0.3", "proper-lockfile": "4.1.2", @@ -32,7 +31,6 @@ "@types/glob": "^7.2.0", "@types/mime": "^2.0.3", "@types/minimatch": "^3.0.5", - "@types/ms": "^0.7.31", "@types/pngjs": "^6.0.1", "@types/progress": "^2.0.5", "@types/proper-lockfile": "^4.1.2", diff --git a/packages/playwright-core/bundles/utils/src/utilsBundleImpl.ts b/packages/playwright-core/bundles/utils/src/utilsBundleImpl.ts index 1ec3c810a7..1e8123888a 100644 --- a/packages/playwright-core/bundles/utils/src/utilsBundleImpl.ts +++ b/packages/playwright-core/bundles/utils/src/utilsBundleImpl.ts @@ -36,9 +36,6 @@ export const mime = mimeLibrary; import minimatchLibrary from 'minimatch'; export const minimatch = minimatchLibrary; -import msLibrary from 'ms'; -export const ms = msLibrary; - export { PNG } from 'pngjs'; export { program } from 'commander'; diff --git a/packages/playwright-core/src/utilsBundle.ts b/packages/playwright-core/src/utilsBundle.ts index 9bab42fb64..718ca796f7 100644 --- a/packages/playwright-core/src/utilsBundle.ts +++ b/packages/playwright-core/src/utilsBundle.ts @@ -25,7 +25,6 @@ export const jpegjs: typeof import('../bundles/utils/node_modules/jpeg-js') = re export const lockfile: typeof import('../bundles/utils/node_modules/@types/proper-lockfile') = require('./utilsBundleImpl').lockfile; export const mime: typeof import('../bundles/utils/node_modules/@types/mime') = require('./utilsBundleImpl').mime; export const minimatch: typeof import('../bundles/utils/node_modules/@types/minimatch') = require('./utilsBundleImpl').minimatch; -export const ms: typeof import('../bundles/utils/node_modules/@types/ms') = require('./utilsBundleImpl').ms; export const PNG: typeof import('../bundles/utils/node_modules/@types/pngjs').PNG = require('./utilsBundleImpl').PNG; export const program: typeof import('../bundles/utils/node_modules/commander').program = require('./utilsBundleImpl').program; export const progress: typeof import('../bundles/utils/node_modules/@types/progress') = require('./utilsBundleImpl').progress; @@ -55,3 +54,29 @@ export function parseStackTraceLine(line: string): { frame: import('../bundles/u fileName, }; } + +export function ms(ms: number): string { + if (!isFinite(ms)) + return '-'; + + if (ms === 0) + return '0ms'; + + if (ms < 1000) + return ms.toFixed(0) + 'ms'; + + const seconds = ms / 1000; + if (seconds < 60) + return seconds.toFixed(1) + 's'; + + const minutes = seconds / 60; + if (minutes < 60) + return minutes.toFixed(1) + 'm'; + + const hours = minutes / 60; + if (hours < 24) + return hours.toFixed(1) + 'h'; + + const days = hours / 24; + return days.toFixed(1) + 'd'; +} diff --git a/packages/web/src/uiUtils.ts b/packages/web/src/uiUtils.ts index 9af12be9cd..0bbd672de7 100644 --- a/packages/web/src/uiUtils.ts +++ b/packages/web/src/uiUtils.ts @@ -19,7 +19,7 @@ export function msToString(ms: number): string { return '-'; if (ms === 0) - return '0'; + return '0ms'; if (ms < 1000) return ms.toFixed(0) + 'ms'; diff --git a/tests/playwright-test/reporter-base.spec.ts b/tests/playwright-test/reporter-base.spec.ts index 3377a6ffa2..76e1d32821 100644 --- a/tests/playwright-test/reporter-base.spec.ts +++ b/tests/playwright-test/reporter-base.spec.ts @@ -347,3 +347,15 @@ test('should report fatal errors at the end', async ({ runInlineTest }) => { expect(result.passed).toBe(2); expect(stripAnsi(result.output)).toContain('2 errors were not a part of any test, see above for details'); }); + +test('should contain at most 1 decimal for humanized timing', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.spec.ts': ` + const { test } = pwt; + test('should work', () => {}); + ` + }); + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(1); + expect(stripAnsi(result.output)).toMatch(/\d+ passed \(\d+(\.\d)?(ms|s)\)/); +}); \ No newline at end of file diff --git a/tests/playwright-test/reporter-html.spec.ts b/tests/playwright-test/reporter-html.spec.ts index 194d8981eb..a758aabf19 100644 --- a/tests/playwright-test/reporter-html.spec.ts +++ b/tests/playwright-test/reporter-html.spec.ts @@ -70,7 +70,7 @@ test('should generate report', async ({ runInlineTest, showReport, page }) => { await expect(page.locator('.test-file-test-outcome-expected >> text=passes')).toBeVisible(); await expect(page.locator('.test-file-test-outcome-skipped >> text=skipped')).toBeVisible(); - await expect(page.getByTestId('overall-duration')).toContainText(/^Total time: \d+(\.\d+)?(ms|s|m)$/); // e.g. 1.2s + await expect(page.getByTestId('overall-duration'), 'should contain humanized total time with at most 1 decimal place').toContainText(/^Total time: \d+(\.\d)?(ms|s|m)$/); await expect(page.locator('.metadata-view')).not.toBeVisible(); });