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.

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",

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",

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';

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';
+}

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';

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)\)/);
+});

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();
 });