fix: remove open report getter

Author: ChristopherPHolder

packages/cli/src/lib/commands/collect/command-impl.ts

@@ -5,16 +5,17 @@ import { run } from '../../core/processing/behaviors';
 import { collectRcJson } from '../init/processes/collect-rc-json';
 import { startServerIfNeededAndExecute } from './utils/serve-command';
 import { collectReports } from './processes/collect-reports';
-import { CollectOptions } from './options';
+import { CollectCommandOptions } from './options';
 
-export async function runCollectCommand(argv: CollectOptions): Promise<void> {
+export async function runCollectCommand(argv: CollectCommandOptions): Promise<void> {
   const cfg = getCollectCommandOptionsFromArgv(argv);
   logVerbose('Collect options: ', cfg);
   await run([
     collectRcJson,
     (cfg: RcJson) =>
-      startServerIfNeededAndExecute(() => collectReports(cfg)
-          .then()
-        , cfg.collect)
+      startServerIfNeededAndExecute(
+        () => collectReports(cfg, argv),
+        cfg.collect
+      )
   ])(cfg);
 }

packages/cli/src/lib/commands/collect/options/dryRun.ts

@@ -8,7 +8,3 @@ export const dryRun = {
   default: getEnvPreset().dryRun as boolean
 } satisfies Options;
 
-export function get(): boolean {
-  const { dryRun } = argv as any as { dryRun: boolean };
-  return dryRun;
-}

packages/cli/src/lib/commands/collect/options/index.ts

@@ -10,6 +10,7 @@ import { awaitServeStdout } from './awaitServeStdout';
 import { dryRun } from './dryRun';
 import { assertOptions } from '../../assert/options';
 import { config } from './config';
+import { GlobalCliOptions } from '../../../global/options';
 
 export const persistOptions = {
   outPath,
@@ -29,3 +30,4 @@ export const collectOptions = {
   ...assertOptions
 } satisfies Record<string, Options>;
 export type CollectOptions = InferredOptionTypes<typeof collectOptions>;
+export type CollectCommandOptions = CollectOptions & GlobalCliOptions;

packages/cli/src/lib/commands/collect/options/openReport.ts

@@ -1,15 +1,10 @@
-import { argv, Options } from 'yargs';
+import { Options } from 'yargs';
 import { getEnvPreset } from '../../../pre-set';
 
 export const openReport = {
   alias: 'e',
   type: 'boolean',
   description: 'Opens browser automatically after the user-flow is collected. (true by default)',
-  default: getEnvPreset().openReport as boolean,
+  default: getEnvPreset().openReport,
   requiresArg: true
 } satisfies Options;
-
-export function get(): boolean {
-  const { openReport } = argv as any as { openReport: boolean };
-  return openReport;
-}

packages/cli/src/lib/commands/collect/processes/collect-reports.ts

@@ -1,25 +1,20 @@
-import { UserFlowProvider } from '../utils/user-flow/types';
 import { concat } from '../../../core/processing/behaviors';
-import { get as dryRun } from '../../../commands/collect/options/dryRun';
 import { collectFlow, loadFlow } from '../utils/user-flow';
 import { persistFlow } from '../utils/persist/persist-flow';
-import { openFlowReport } from '../utils/persist/open-report';
+import { handleOpenFlowReports } from '../utils/persist/open-report';
 import { RcJson } from '../../../types';
+import { CollectCommandOptions } from '../options';
 
-export async function collectReports(cfg: RcJson): Promise<RcJson> {
+export async function collectReports(cfg: RcJson, argv: CollectCommandOptions): Promise<RcJson> {
 
   const { collect, persist, assert } = cfg;
 
-  let userFlows = [] as ({ exports: UserFlowProvider, path: string })[];
-  // Load and run user-flows in sequential
-  userFlows = loadFlow(collect);
+  const userFlows = loadFlow(collect);
   await concat(userFlows.map(({ exports: provider, path }) =>
     (_: any) => {
-      return collectFlow({
-        ...collect, ...persist, ...assert, dryRun: dryRun()
-      }, { ...provider, path })
+      return collectFlow({ ...collect, ...persist, ...assert }, { ...provider, path }, argv)
         .then((flow) => persistFlow(flow, { ...persist, ...collect }))
-        .then(openFlowReport)
+        .then(handleOpenFlowReports(argv))
         .then(_ => cfg);
     })
   )(cfg);

packages/cli/src/lib/commands/collect/utils/persist/open-report.ts

@@ -1,33 +1,34 @@
-import { get as dryRun } from '../../../../commands/collect/options/dryRun';
-import { get as openReport } from '../../options/openReport';
-import { get as interactive } from '../../../../global/options/interactive';
+import * as openReport from 'open';
 import { logVerbose } from '../../../../core/loggin';
-import * as openFileInBrowser from 'open';
+import { CollectCommandOptions } from '../../options';
 
-export async function openFlowReport(fileNames: string[]): Promise<void> {
-  // open report if requested and not in executed in CI
-  if (!dryRun() && openReport() && interactive()) {
-
-    const htmlReport = fileNames.find(i => i.includes('.html'));
-    if (htmlReport) {
-      logVerbose('open HTML report in browser');
-      await openFileInBrowser(htmlReport, { wait: false });
-      return Promise.resolve(void 0);
-    }
+export async function openFlowReports(fileNames: string[]): Promise<void> {
+  const htmlReport = fileNames.find(i => i.includes('.html'));
+  if (htmlReport) {
+    logVerbose('open HTML report in browser');
+    await openReport(htmlReport, { wait: false });
+    return Promise.resolve(void 0);
+  }
 
-    const mdReport = fileNames.find(i => i.includes('.md'));
-    if (mdReport) {
-      logVerbose('open Markdown report in browser');
-      await openFileInBrowser(mdReport, { wait: false });
-      return Promise.resolve(void 0);
-    }
+  const mdReport = fileNames.find(i => i.includes('.md'));
+  if (mdReport) {
+    logVerbose('open Markdown report in browser');
+    await openReport(mdReport, { wait: false });
+    return Promise.resolve(void 0);
+  }
 
-    const jsonReport = fileNames.find(i => i.includes('.json'));
-    if (jsonReport) {
-      logVerbose('open JSON report in browser');
-      // @TODO if JSON is given open the file in https://googlechrome.github.io/lighthouse/viewer/
-      await openFileInBrowser(jsonReport, { wait: false });
-    }
+  const jsonReport = fileNames.find(i => i.includes('.json'));
+  if (jsonReport) {
+    logVerbose('open JSON report in browser');
+    // @TODO if JSON is given open the file in https://googlechrome.github.io/lighthouse/viewer/
+    await openReport(jsonReport, { wait: false });
   }
   return Promise.resolve(void 0);
 }
+
+export function handleOpenFlowReports({ dryRun, openReport, interactive}: CollectCommandOptions) {
+  if (dryRun || !openReport || !interactive) {
+    return;
+  }
+  return openFlowReports;
+}

packages/cli/src/lib/commands/collect/utils/persist/open-report.unit.test.ts

@@ -0,0 +1,77 @@
+import * as openReport from 'open';
+import { handleOpenFlowReports, openFlowReports } from './open-report';
+import { logVerbose } from '../../../../core/loggin';
+import { CollectCommandOptions } from '../../options';
+
+jest.mock('open');
+jest.mock('../../../../core/loggin');
+
+describe('handleOpenFlowReport', () => {
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  })
+
+  it('should return the openFlowReport function if openReport, interactive and not dryRun', async () => {
+    const openReportsProcess = handleOpenFlowReports({
+      openReport: true,
+      interactive: true,
+      dryRun: false,
+    } as CollectCommandOptions);
+    expect(openReportsProcess).toEqual(expect.any(Function));
+  });
+
+  it('should return undefined if openReport is false', () => {
+    const openReportsProcess = handleOpenFlowReports({
+      openReport: false,
+      interactive: true,
+      dryRun: false,
+    } as CollectCommandOptions);
+    expect(openReportsProcess).toBeUndefined();
+  });
+
+  it('should return undefined if dryRun is true', () => {
+    const openReportsProcess = handleOpenFlowReports({
+      openReport: true,
+      interactive: true,
+      dryRun: true,
+    } as CollectCommandOptions);
+    expect(openReportsProcess).toBeUndefined();
+  });
+
+  it('should return undefined if interactive is false', () => {
+    const openReportsProcess = handleOpenFlowReports({
+      openReport: true,
+      interactive: false,
+      dryRun: false,
+    } as CollectCommandOptions);
+    expect(openReportsProcess).toBeUndefined();
+  });
+});
+
+describe('openReports', () => {
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('should not open the report if no file name is passed', async () => {
+    await openFlowReports([]);
+    expect(openReport).not.toHaveBeenCalled();
+  });
+
+  it.each(['html', 'json', 'md'])('should open the %s report', async (format) => {
+    await openFlowReports([`example.${format}`]);
+    expect(openReport).toHaveBeenCalled();
+  });
+
+  it('should not logVerbose if no file name is passed', async () => {
+    await openFlowReports([]);
+    expect(logVerbose).not.toHaveBeenCalled();
+  });
+
+  it('should only open 1 time report if multiple report formats are passed', async () => {
+    await openFlowReports(['example.html', 'example.json', 'example.md']);
+    expect(openReport).toHaveBeenCalledTimes(1);
+  });
+});

packages/cli/src/lib/commands/collect/utils/user-flow/collect-flow.ts

@@ -5,17 +5,18 @@ import { Browser, LaunchOptions, Page } from 'puppeteer';
 import { normalize } from 'path';
 // @ts-ignore
 import { startFlow, UserFlow } from 'lighthouse/lighthouse-core/fraggle-rock/api';
-import { get as dryRun } from '../../../../commands/collect/options/dryRun';
 import { UserFlowMock } from './user-flow.mock';
-import { detectCliMode } from '../../../../global/cli-mode/cli-mode';
+import { detectCliMode } from '../../../../global/cli-mode';
 import { CollectArgvOptions } from '../../options/types';
 import { getLhConfigFromArgv, mergeLhConfig } from '../config';
 import { PersistArgvOptions } from '../../options/types';
 import { AssertRcOptions } from '../../../assert/options';
+import { CollectCommandOptions } from '../../options';
 
 export async function collectFlow(
   cliOption: CollectArgvOptions & PersistArgvOptions & AssertRcOptions,
-  userFlowProvider: UserFlowProvider & { path: string }
+  userFlowProvider: UserFlowProvider & { path: string },
+  argv: CollectCommandOptions
 ) {
   let {
     path,
@@ -37,7 +38,7 @@ export async function collectFlow(
   logVerbose(`User-flow path: ${normalize(path)}`);
   let start = Date.now();
 
-  const flow: UserFlow = !dryRun() ? await startFlow(page, flowOptions) : new UserFlowMock(page, flowOptions);
+  const flow: UserFlow = !argv.dryRun ? await startFlow(page, flowOptions) : new UserFlowMock(page, flowOptions);
 
   // run custom interactions
   await interactions({ flow, page, browser, collectOptions: cliOption });

packages/cli/src/lib/commands/init/utils.ts

@@ -1,6 +1,17 @@
-import { CollectRcOptions, PersistRcOptions } from '../collect/options/types';
+import { CollectRcOptions, PersistRcOptions, ReportFormat } from '../collect/options/types';
 import { InitOptions } from './options';
 import { AssertRcOptions } from '../assert/options';
+import { REPORT_FORMAT_VALUES } from '../collect/constants';
+
+const isValidFormat = (value: any): value is ReportFormat => REPORT_FORMAT_VALUES.includes(value);
+
+function sanitizedFormats(formats: string[]) {
+  const validatedFormats: ReportFormat[] = formats.filter(isValidFormat);
+  if (validatedFormats.length !== formats.length) {
+    throw new Error(`${formats} contains invalid format options`);
+  }
+  return validatedFormats;
+}
 
 export function getInitCommandOptionsFromArgv(argv: InitOptions) {
   let {
@@ -18,7 +29,7 @@ export function getInitCommandOptionsFromArgv(argv: InitOptions) {
 
   let persist = {} as PersistRcOptions;
   outPath && (persist.outPath = outPath);
-  format && (persist.format = format);
+  format && (persist.format = sanitizedFormats(format));
 
   let assert = {} as AssertRcOptions;
   budgetPath && (assert.budgetPath = budgetPath);

packages/cli/src/lib/global/options/index.ts

@@ -1,13 +1,15 @@
+import { InferredOptionTypes, Options } from 'yargs';
+
 import { verbose, get as getVerbose } from './verbose';
 import { rcPath, get as getRcPath } from '../rc-json/options/rc';
 import { interactive, get as getInteractive } from './interactive';
-import { Options } from 'yargs';
 
 export const GLOBAL_OPTIONS_YARGS_CFG = {
   verbose,
   rcPath,
   interactive
 } satisfies Record<string, Options>;
+export type GlobalCliOptions = InferredOptionTypes<typeof GLOBAL_OPTIONS_YARGS_CFG>;
 
 export const globalOptions = {
   getVerbose,