[ENG-1888] fix(cmd): replace globby with tinyglobby to drop braces

packages/cmd/package.json

@@ -67,14 +67,14 @@
     "execa": "^9.5.1",
     "fs-extra": "^11.3.4",
     "getos": "^3.2.1",
-    "globby": "^11.1.0",
     "https-proxy-agent": "^7.0.4",
     "lodash": "^4.18.1",
     "nanoid": "^3.3.8",
     "pretty-ms": "^7.0.1",
     "proxy-from-env": "^1.1.0",
     "semver": "^7.8.5",
     "source-map-support": "^0.5.21",
+    "tinyglobby": "^0.2.17",
     "tmp": "^0.2.7",
     "tmp-promise": "^3.0.3",
     "ts-pattern": "^5.8.0",

packages/cmd/src/commands/convert/options.ts

@@ -1,5 +1,5 @@
 import { InvalidArgumentError, Option } from '@commander-js/extra-typings';
-import * as globby from 'globby';
+import { globPaths } from '@lib/glob';
 
 export enum REPORT_INPUT_FORMATS {
   junit = 'junit',
@@ -44,7 +44,7 @@ export const frameworkVersionOption = new Option(
 function validateGlobPattern(value: string) {
   const patterns = value.split(',').map((pattern) => pattern.trim());
 
-  const allResults = globby.sync(patterns);
+  const allResults = globPaths(patterns);
 
   if (allResults.length === 0) {
     throw new InvalidArgumentError('No files found with the provided patterns');

packages/cmd/src/lib/__tests__/glob.test.ts

@@ -0,0 +1,180 @@
+import fs from 'fs-extra';
+import os from 'os';
+import path from 'path';
+import { globSync } from 'tinyglobby';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { posixJoin } from '../../test-utils/path';
+import { globPaths } from '../glob';
+
+// Wrapped so one test can return files out of order: macOS lists folders
+// sorted, so only a fixed tinyglobby result shows that globPaths sorts
+vi.mock('tinyglobby', async (importActual) => {
+  const actual = await importActual<typeof import('tinyglobby')>();
+  return { ...actual, globSync: vi.fn(actual.globSync) };
+});
+
+describe('globPaths', () => {
+  let root: string;
+  let cwd: string;
+
+  beforeEach(() => {
+    root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'glob-')));
+    cwd = path.join(root, 'project');
+    for (const file of [
+      'reports/a.xml',
+      'reports/b.xml',
+      'reports/c.json',
+      'reports/skip.xml',
+      'reports/.hidden.xml',
+      'reports/nested/d.xml',
+      'reports/1.xml',
+      'reports/2.xml',
+      'reports/10.xml',
+      'reports/01.xml',
+      'reports/03.xml',
+      'dist/a.js',
+      'dist/sub/b.js',
+    ]) {
+      fs.outputFileSync(path.join(cwd, file), file);
+    }
+    fs.outputFileSync(path.join(root, 'outside/r.xml'), 'r');
+    vi.spyOn(process, 'cwd').mockReturnValue(cwd);
+  });
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+    fs.removeSync(root);
+  });
+
+  it('sorts the matches of a pattern', () => {
+    vi.mocked(globSync).mockReturnValueOnce(['reports/b.xml', 'reports/a.xml']);
+    expect(globPaths(['reports/*.xml'])).toEqual([
+      'reports/a.xml',
+      'reports/b.xml',
+    ]);
+  });
+
+  it('returns the matches sorted, relative to the current folder', () => {
+    expect(globPaths(['reports/**/[a-z].xml'])).toEqual([
+      'reports/a.xml',
+      'reports/b.xml',
+      'reports/nested/d.xml',
+    ]);
+  });
+
+  it('returns the matches of each pattern in pattern order, once', () => {
+    expect(globPaths(['reports/b.xml', 'reports/[ab].xml'])).toEqual([
+      'reports/b.xml',
+      'reports/a.xml',
+    ]);
+  });
+
+  it('returns the files of a folder', () => {
+    expect(globPaths(['dist'])).toEqual(['dist/a.js', 'dist/sub/b.js']);
+  });
+
+  it('matches a dotfile only by its name', () => {
+    expect(globPaths(['reports/*.xml'])).not.toContain('reports/.hidden.xml');
+    expect(globPaths(['reports/.hidden.xml'])).toEqual(['reports/.hidden.xml']);
+  });
+
+  it('returns the matches of absolute patterns as absolute paths', () => {
+    expect(globPaths([posixJoin(cwd, 'dist')])).toEqual([
+      posixJoin(cwd, 'dist/a.js'),
+      posixJoin(cwd, 'dist/sub/b.js'),
+    ]);
+    expect(globPaths([posixJoin(root, 'outside/*.xml')])).toEqual([
+      posixJoin(root, 'outside/r.xml'),
+    ]);
+  });
+
+  it('expands brace lists', () => {
+    expect(globPaths(['reports/{a,c}.{xml,json}'])).toEqual([
+      'reports/a.xml',
+      'reports/c.json',
+    ]);
+  });
+
+  it.each([
+    ['reports/{1..10}.xml', ['1', '10', '2']],
+    ['reports/{2..1}.xml', ['1', '2']],
+    ['reports/{01..03}.xml', ['01', '03']],
+    ['reports/{1..10..9}.xml', ['1', '10']],
+    ['reports/{a,{1..2}}.xml', ['1', '2', 'a']],
+    ['reports/{0..1}{0..3}.xml', ['01', '03', '10']],
+    ['reports/{1..1000}.xml', ['1', '10', '2']],
+  ])('expands the numeric range in %s', (pattern, names) => {
+    expect(globPaths([pattern])).toEqual(
+      names.map((name) => `reports/${name}.xml`)
+    );
+  });
+
+  it('expands a numeric range with negative values', () => {
+    fs.outputFileSync(path.join(cwd, 'reports/-1.xml'), '-1');
+    expect(globPaths(['reports/{-1..1}.xml'])).toEqual([
+      'reports/-1.xml',
+      'reports/1.xml',
+    ]);
+  });
+
+  it('leaves an escaped range as written', () => {
+    fs.outputFileSync(path.join(cwd, 'reports/{1..2}.xml'), 'literal');
+    expect(globPaths(['reports/\\{1..2\\}.xml'])).toEqual([
+      'reports/{1..2}.xml',
+    ]);
+  });
+
+  it('expands a numeric range in a negated pattern', () => {
+    expect(globPaths(['reports/*.xml', '!reports/{1..10}.xml'])).toEqual([
+      'reports/01.xml',
+      'reports/03.xml',
+      'reports/a.xml',
+      'reports/b.xml',
+      'reports/skip.xml',
+    ]);
+  });
+
+  it.each(['{1..2000}', '{2000..1}', '{1..4000..2}'])(
+    'refuses the numeric range %s, which has more than 1000 values',
+    (range) => {
+      expect(() => globPaths([`reports/${range}.xml`])).toThrow(
+        `${range} expands to more than 1000 values`
+      );
+    }
+  );
+
+  it('leaves out the matches of a negated pattern from every pattern', () => {
+    expect(
+      globPaths(['reports/[as]*.xml', '!reports/skip.xml', 'reports/s*.xml'])
+    ).toEqual(['reports/a.xml']);
+  });
+
+  it.each([
+    ['relative', () => '!reports/skip.xml'],
+    ['absolute', () => `!${posixJoin(cwd, 'reports/skip.xml')}`],
+  ])('applies a %s negated pattern to an absolute one', (_, negation) => {
+    expect(
+      globPaths([posixJoin(cwd, 'reports/[as]*.xml'), negation()])
+    ).toEqual([posixJoin(cwd, 'reports/a.xml')]);
+  });
+
+  it('leaves out the files of a negated folder', () => {
+    expect(globPaths(['reports/**/*.xml', '!reports/nested'])).not.toContain(
+      'reports/nested/d.xml'
+    );
+  });
+
+  // An empty pattern is what a trailing comma in --input-file leaves
+  it('matches nothing for an empty pattern or one with no match', () => {
+    expect(globPaths(['reports/a.xml', '', 'nope/*.xml'])).toEqual([
+      'reports/a.xml',
+    ]);
+  });
+
+  // GHSA-vfj7-8cjw-p6xm: braces overflowed the stack on deeply nested groups
+  it('handles deeply nested braces', () => {
+    const depth = 5000;
+    const pattern = '{'.repeat(depth) + 'a' + '}'.repeat(depth) + '.xml';
+    expect(globPaths([pattern])).toEqual([]);
+  });
+});

packages/cmd/src/lib/glob.ts

@@ -0,0 +1,58 @@
+import { isAbsolute } from 'path';
+import { globSync } from 'tinyglobby';
+
+/**
+ * Returns the files that match the glob patterns, pattern by pattern and
+ * sorted within each, without repeating a path. A `!pattern` leaves its
+ * matches out of every pattern.
+ */
+export function globPaths(globs: string[]): string[] {
+  const patterns = globs.map(expandNumericRanges);
+  const ignore = patterns
+    .filter((pattern) => pattern.startsWith('!'))
+    .map((pattern) => pattern.slice(1));
+  const files = patterns
+    .filter((pattern) => !pattern.startsWith('!'))
+    .flatMap((pattern) =>
+      // Without `absolute`, tinyglobby returns /tmp/r.xml as ../../tmp/r.xml
+      globSync(pattern, { absolute: isAbsolute(pattern), ignore }).sort()
+    );
+  return [...new Set(files)];
+}
+
+const NUMERIC_RANGE = /(?<!\\)\{(-?\d+)\.\.(-?\d+)(?:\.\.(-?\d+))?\}/g;
+// globby (through braces) threw on ascending ranges with more values; this
+// throws on descending ones too, so a typo like {10000..1} doesn't hang the CLI
+const MAX_RANGE_VALUES = 1000;
+
+// tinyglobby reads {1..10} as the character class [1-10], which matches only
+// 0 and 1, and matches nothing for {01..03}
+function expandNumericRanges(pattern: string) {
+  return pattern.replace(NUMERIC_RANGE, (_, from, to, step) => {
+    const values = numericRange(from, to, step);
+    return values.length === 1 ? values[0] : `{${values.join(',')}}`;
+  });
+}
+
+// Same values as braces: a step of 0 counts as 1, and a bound written with a
+// leading zero pads every value to the longest bound
+function numericRange(from: string, to: string, step = '1') {
+  const start = Number(from);
+  const end = Number(to);
+  const size = Math.abs(Number(step)) || 1;
+  const count = Math.floor(Math.abs(end - start) / size) + 1;
+  if (count > MAX_RANGE_VALUES) {
+    const range = [from, to, ...(step === '1' ? [] : [step])].join('..');
+    throw new Error(
+      `{${range}} expands to more than ${MAX_RANGE_VALUES} values`
+    );
+  }
+  const direction = start <= end ? 1 : -1;
+  const padded = [from, to].some((bound) => /^-?0\d/.test(bound));
+  const width = padded ? Math.max(from.length, to.length) : 0;
+  return Array.from({ length: count }, (_, i) => {
+    const value = start + i * direction * size;
+    const sign = value < 0 ? '-' : '';
+    return sign + String(Math.abs(value)).padStart(width - sign.length, '0');
+  });
+}

packages/cmd/src/services/cache/__tests__/path.spec.ts

@@ -2,7 +2,8 @@ import fs from 'fs-extra';
 import os from 'os';
 import path from 'path';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
-import { filterPaths, getUploadPaths } from '../path';
+import { posixJoin } from '../../../test-utils/path';
+import { filterPaths, getLastRunFilePaths, getUploadPaths } from '../path';
 
 vi.mock('../lib', () => ({ warn: vi.fn() }));
 
@@ -32,7 +33,26 @@ describe('cache paths', () => {
       'dist/../../outside.txt',
     ]);
 
-    expect(paths).toEqual([path.join(cwd, 'dist/a.js')]);
+    expect(paths).toEqual([posixJoin(cwd, 'dist/a.js')]);
+  });
+
+  it('finds every .last-run.json under the Playwright output folder', async () => {
+    fs.outputFileSync(path.join(cwd, 'test-results/.last-run.json'), '{}');
+    fs.outputFileSync(path.join(cwd, 'test-results/s1/.last-run.json'), '{}');
+    fs.outputFileSync(path.join(cwd, 'test-results/s1/trace.zip'), 'zip');
+
+    expect((await getLastRunFilePaths()).sort()).toEqual([
+      posixJoin(cwd, 'test-results/.last-run.json'),
+      posixJoin(cwd, 'test-results/s1/.last-run.json'),
+    ]);
+  });
+
+  it('finds .last-run.json in an output folder outside the current folder', async () => {
+    fs.outputFileSync(path.join(root, 'pw-out/.last-run.json'), '{}');
+
+    expect(await getLastRunFilePaths(path.join(root, 'pw-out'))).toEqual([
+      posixJoin(root, 'pw-out/.last-run.json'),
+    ]);
   });
 
   it.each(['../outside.txt', '/etc/hosts', 'dist/../../outside.txt', '..'])(

packages/cmd/src/services/cache/path.ts

@@ -1,5 +1,5 @@
 import path from 'path';
-import * as globby from 'globby';
+import { globPaths } from '@lib/glob';
 import { MetaFile, warn } from './lib';
 
 export const getLastRunFilePaths = async (outputPath?: string) => {
@@ -10,15 +10,15 @@ export const getLastRunFilePaths = async (outputPath?: string) => {
     path.posix.join(prefix.replace(/\\/g, '/'), '.last-run.json'),
   ];
 
-  return globby.sync(patterns);
+  return globPaths(patterns);
 };
 
 export const getUploadPaths = async (pathPatterns: string[] = []) => {
   const insidePaths = filterPaths(pathPatterns);
   if (insidePaths.length === 0) {
     return [];
   }
-  return globby.sync(insidePaths.map((p) => p.replace(/\\/g, '/')));
+  return globPaths(insidePaths.map((p) => p.replace(/\\/g, '/')));
 };
 
 /** Keeps the paths inside the current folder, with a warning for the others. */

packages/cmd/src/test-utils/path.ts

@@ -0,0 +1,5 @@
+import path from 'path';
+
+/** Joins with / on every platform, the way glob patterns and matches are written. */
+export const posixJoin = (...parts: string[]) =>
+  path.join(...parts).replace(/\\/g, '/');