code review feedback

change/monosize-190af040-06c8-4534-88ab-f97941ef976d.json

@@ -1,6 +1,6 @@
 {
   "type": "minor",
-  "comment": "Add arg to measure specific fixture files",
+  "comment": "feat: implement --fixtures argument for measure CLI command",
   "packageName": "monosize",
   "email": "[email protected]",
   "dependentChangeType": "patch"

packages/monosize/src/commands/measure.mts

@@ -15,11 +15,11 @@ import type { BuildResult } from '../types.mjs';
 export type MeasureOptions = CliOptions & {
   debug: boolean;
   'artifacts-location': string;
-  fixtures?: string;
+  fixtures: string;
 };
 
 async function measure(options: MeasureOptions) {
-  const { debug = false, quiet, 'artifacts-location': artifactsLocation, fixtures: fixturesGlob } = options;
+  const { debug = false, quiet, 'artifacts-location': artifactsLocation, fixtures: fixturesGlob = '*.fixture.js' } = options;
 
   const startTime = process.hrtime();
   const artifactsDir = path.resolve(process.cwd(), artifactsLocation);
@@ -40,14 +40,13 @@ async function measure(options: MeasureOptions) {
     console.log(`${pc.blue('[i]')} artifacts dir is cleared`);
   }
 
-  const fixtureFilesGlob = fixturesGlob ? fixturesGlob : '*.fixture.js'; 
-  const fixtures = glob.sync(`bundle-size/${fixtureFilesGlob}`, {
+  const fixtures = glob.sync(`bundle-size/${fixturesGlob}`, {
     absolute: true,
     cwd: process.cwd(),
   });
 
   if (!fixtures.length && fixturesGlob) {    
-    console.log(`${pc.red('[e]')} No matching fixtures found for globbing pattern '${fixturesGlob}'`);
+    console.error(`${pc.red('[e]')} No matching fixtures found for globbing pattern '${fixturesGlob}'`);
     process.exit(1);
   }
 
@@ -112,6 +111,11 @@ const api: CommandModule<Record<string, unknown>, MeasureOptions> = {
         'Relative path to the package root where the artifact files will be stored (monosize.json & bundler output). If specified, "--report-files-glob" in "monosize collect-reports" & "monosize upload-reports" should be set accordingly.',
       default: 'dist/bundle-size',
     },
+    fixtures: {
+      type: 'string',
+      description: 'Filename glob pattern to target whatever fixture files you want to measure.',
+      default: '*.fixture.js'
+    }
   },
 };
 

packages/monosize/src/commands/measure.test.mts

@@ -22,6 +22,12 @@ vitest.mock('../utils/readConfig', () => ({
   }),
 }));
 
+vitest.mock('picocolors', () => ({
+  default: ({
+    red: () => ''
+  })
+}));
+
 async function setup(fixtures: { [key: string]: string }) {
   const packageDir = tmp.dirSync({ unsafeCleanup: true });
 
@@ -39,7 +45,7 @@ async function setup(fixtures: { [key: string]: string }) {
     packageDir: packageDir.name,
   };
 }
-const getMockedFixtures = (fixtureNames: string[]) => (
+const getMockedFixtures = (...fixtureNames: string[]) => (
   fixtureNames.reduce((acc, item) => ({
     ...acc,
     [`${item}.fixture.js`]: `
@@ -49,18 +55,18 @@ const getMockedFixtures = (fixtureNames: string[]) => (
   }), {})
 );
 
-function noop(num?: number) {
-  /* does nothing */
-}
+// eslint-disable-next-line @typescript-eslint/no-empty-function
+const noop = () => {};
 
 describe('measure', () => {
   beforeEach(() => {
     vitest.clearAllMocks();
   });
 
   it('builds fixtures and created a report', async () => {
-    const { packageDir } = await setup(getMockedFixtures(['foo', 'bar']));
-    const options: MeasureOptions = { quiet: true, debug: false, 'artifacts-location': 'output' };
+    const { packageDir } = await setup(getMockedFixtures('foo', 'bar'));
+    const options: MeasureOptions = { quiet: true, debug: false, 'artifacts-location': 'output', fixtures: '*.fixture.js' };
+
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     await api.handler(options as any);
 
@@ -95,7 +101,7 @@ describe('measure', () => {
   }); 
 
   it('builds single targeted fixture when full filename passed', async () => {
-    const { packageDir } = await setup(getMockedFixtures(['foo', 'bar', 'baz']));
+    const { packageDir } = await setup(getMockedFixtures('foo', 'bar', 'baz'));
     const options: MeasureOptions = { quiet: true, debug: false, 'artifacts-location': 'output', fixtures: 'foo.fixture.js' };
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     await api.handler(options as any);
@@ -110,7 +116,7 @@ describe('measure', () => {
   });
 
   it('builds only targeted fixtures with pattern passed', async () => {
-    const { packageDir } = await setup(getMockedFixtures(['foo', 'bar', 'baz']));
+    const { packageDir } = await setup(getMockedFixtures('foo', 'bar', 'baz'));
     const options: MeasureOptions = { quiet: true, debug: false, 'artifacts-location': 'output', fixtures: 'ba*' };
 
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -128,7 +134,7 @@ describe('measure', () => {
   });
 
   it('returns exit code of 1 and displays message when fixtures argument fails to match any fixture filename', async () => {
-    const log = vitest.spyOn(console, 'log').mockImplementation(noop);    
+    const errorLog = vitest.spyOn(console, 'error').mockImplementation(noop);    
     const mockExit = vitest.spyOn(process, 'exit').mockImplementation(noop as any);
 
     await setup({});
@@ -137,7 +143,7 @@ describe('measure', () => {
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     await api.handler(options as any);
 
-    expect(log.mock.calls[0][0]).toMatch(/No matching fixtures found for globbing pattern/);
+    expect(errorLog.mock.calls[0][0]).toMatchInlineSnapshot(`" No matching fixtures found for globbing pattern 'invalid-filename.js'"`);
     expect(mockExit).toHaveBeenCalledWith(1);
   });