From 91b77ead60146155711fa673b1a3068041b8d2ef Mon Sep 17 00:00:00 2001 From: ChristopherPHolder Date: Wed, 6 Mar 2024 18:46:19 +0100 Subject: [PATCH 1/7] fix: remove openReport options getter --- .../src/lib/commands/collect/command-impl.ts | 7 +-- .../commands/collect/options/openReport.ts | 9 +--- .../collect/processes/collect-reports.ts | 11 ++-- .../collect/utils/persist/open-report.ts | 50 ++++++++++--------- 4 files changed, 36 insertions(+), 41 deletions(-) diff --git a/packages/cli/src/lib/commands/collect/command-impl.ts b/packages/cli/src/lib/commands/collect/command-impl.ts index ea3a149e1..ddd032422 100644 --- a/packages/cli/src/lib/commands/collect/command-impl.ts +++ b/packages/cli/src/lib/commands/collect/command-impl.ts @@ -13,8 +13,9 @@ export async function runCollectCommand(argv: CollectOptions): Promise { await run([ collectRcJson, (cfg: RcJson) => - startServerIfNeededAndExecute(() => collectReports(cfg) - .then() - , cfg.collect) + startServerIfNeededAndExecute( + () => collectReports(cfg, argv.openReport), + cfg.collect + ) ])(cfg); } diff --git a/packages/cli/src/lib/commands/collect/options/openReport.ts b/packages/cli/src/lib/commands/collect/options/openReport.ts index 2ee6d6311..5a913df17 100644 --- a/packages/cli/src/lib/commands/collect/options/openReport.ts +++ b/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; -} diff --git a/packages/cli/src/lib/commands/collect/processes/collect-reports.ts b/packages/cli/src/lib/commands/collect/processes/collect-reports.ts index 8d2133656..06da50cf4 100644 --- a/packages/cli/src/lib/commands/collect/processes/collect-reports.ts +++ b/packages/cli/src/lib/commands/collect/processes/collect-reports.ts @@ -1,25 +1,22 @@ -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'; -export async function collectReports(cfg: RcJson): Promise { +export async function collectReports(cfg: RcJson, openReport: boolean): Promise { 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 }) .then((flow) => persistFlow(flow, { ...persist, ...collect })) - .then(openFlowReport) + .then(handleOpenFlowReports(openReport)) .then(_ => cfg); }) )(cfg); diff --git a/packages/cli/src/lib/commands/collect/utils/persist/open-report.ts b/packages/cli/src/lib/commands/collect/utils/persist/open-report.ts index fa3953ac5..55fd2cb02 100644 --- a/packages/cli/src/lib/commands/collect/utils/persist/open-report.ts +++ b/packages/cli/src/lib/commands/collect/utils/persist/open-report.ts @@ -1,33 +1,35 @@ +import * as openReport from 'open'; 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 { logVerbose } from '../../../../core/loggin'; -import * as openFileInBrowser from 'open'; -export async function openFlowReport(fileNames: string[]): Promise { - // 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 { + 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(openReport: boolean): typeof openFlowReports | undefined { + if (dryRun() || !openReport || !interactive()) { + return; + } + return openFlowReports; +} From c38666296f06d48e63b0c141b71e45b94d2f85dd Mon Sep 17 00:00:00 2001 From: ChristopherPHolder Date: Wed, 6 Mar 2024 18:47:13 +0100 Subject: [PATCH 2/7] test: add unit test for open report --- .../utils/persist/open-report.unit.test.ts | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 packages/cli/src/lib/commands/collect/utils/persist/open-report.unit.test.ts diff --git a/packages/cli/src/lib/commands/collect/utils/persist/open-report.unit.test.ts b/packages/cli/src/lib/commands/collect/utils/persist/open-report.unit.test.ts new file mode 100644 index 000000000..2623358da --- /dev/null +++ b/packages/cli/src/lib/commands/collect/utils/persist/open-report.unit.test.ts @@ -0,0 +1,99 @@ +import { handleOpenFlowReports, openFlowReports } from './open-report'; +import { logVerbose } from '../../../../core/loggin'; +import * as openReport from 'open'; + +import * as dryRun from '../../../../commands/collect/options/dryRun'; +import * as interactive from '../../../../global/options/interactive'; + +jest.mock('../../../../commands/collect/options/dryRun'); +jest.mock('../../../../global/options/interactive'); + +describe('handleOpenFlowReport', () => { + beforeEach(() => { + jest.clearAllMocks(); + }) + + it('should return undefined if openReport is false', () => { + const openReportsProcess = handleOpenFlowReports(false); + expect(openReportsProcess).toEqual(undefined); + }); + + it('should return undefined if dryRun is true', () => { + jest.spyOn(dryRun, 'get').mockReturnValue(true); + const openReportsProcess = handleOpenFlowReports(true); + expect(openReportsProcess).toEqual(undefined); + }); + + it('should return undefined if interactive is false', () => { + jest.spyOn(interactive, 'get').mockReturnValue(false); + const openReportsProcess = handleOpenFlowReports(true); + expect(openReportsProcess).toEqual(undefined); + }); + + it('should return the openFlowReport function if openReport, interactive and not dryRun', async () => { + jest.spyOn(interactive, 'get').mockReturnValue(true); + jest.spyOn(dryRun, 'get').mockReturnValue(false); + const openReportsProcess = handleOpenFlowReports(true); + expect(typeof openReportsProcess).toEqual("function"); + }); +}); + +jest.mock('open'); +jest.mock('../../../../core/loggin'); + +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('should not logVerbose if no file name is passed', async () => { + await openFlowReports([]); + expect(logVerbose).not.toHaveBeenCalled(); + }); + + it('should open the report if filenames includes an html report', async () => { + await openFlowReports(['example.html']); + expect(openReport).toHaveBeenCalled(); + }); + + it('should logVerbose if filenames includes an html report', async () => { + await openFlowReports(['example.html']); + expect(logVerbose).toHaveBeenCalledWith(expect.stringContaining('HTML')); + }); + + it('should open the report if filenames includes a json report', async () => { + await openFlowReports(['example.json']); + expect(openReport).toHaveBeenCalled(); + }); + + it('should logVerbose if filenames includes an json report', async () => { + await openFlowReports(['example.json']); + expect(logVerbose).toHaveBeenCalledWith(expect.stringContaining('JSON')); + }); + + it('should open the report if filenames includes a md report', async () => { + await openFlowReports(['example.md']); + expect(openReport).toHaveBeenCalled(); + }); + + it('should logVerbose if filenames includes an md report', async () => { + await openFlowReports(['example.md']); + expect(logVerbose).toHaveBeenCalledWith(expect.stringContaining('Markdown')); + }); + + 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); + }); + + it('should only logVerbose 1 time report if multiple report formats are passed', async () => { + await openFlowReports(['example.html', 'example.json', 'example.md']); + expect(logVerbose).toHaveBeenCalledTimes(1); + }); +}); From 9bc03fe46aa8cc887de4d79cb2aefd880578f3be Mon Sep 17 00:00:00 2001 From: ChristopherPHolder Date: Wed, 6 Mar 2024 19:29:29 +0100 Subject: [PATCH 3/7] fix: fix typing issues --- .../commands/collect/processes/collect-reports.ts | 2 +- .../commands/collect/utils/persist/open-report.ts | 2 +- packages/cli/src/lib/commands/init/utils.ts | 15 +++++++++++++-- packages/cli/src/lib/pre-set.ts | 2 +- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/lib/commands/collect/processes/collect-reports.ts b/packages/cli/src/lib/commands/collect/processes/collect-reports.ts index 06da50cf4..4b7105b92 100644 --- a/packages/cli/src/lib/commands/collect/processes/collect-reports.ts +++ b/packages/cli/src/lib/commands/collect/processes/collect-reports.ts @@ -5,7 +5,7 @@ import { persistFlow } from '../utils/persist/persist-flow'; import { handleOpenFlowReports } from '../utils/persist/open-report'; import { RcJson } from '../../../types'; -export async function collectReports(cfg: RcJson, openReport: boolean): Promise { +export async function collectReports(cfg: RcJson, openReport?: boolean): Promise { const { collect, persist, assert } = cfg; diff --git a/packages/cli/src/lib/commands/collect/utils/persist/open-report.ts b/packages/cli/src/lib/commands/collect/utils/persist/open-report.ts index 55fd2cb02..5d96f774a 100644 --- a/packages/cli/src/lib/commands/collect/utils/persist/open-report.ts +++ b/packages/cli/src/lib/commands/collect/utils/persist/open-report.ts @@ -27,7 +27,7 @@ export async function openFlowReports(fileNames: string[]): Promise { return Promise.resolve(void 0); } -export function handleOpenFlowReports(openReport: boolean): typeof openFlowReports | undefined { +export function handleOpenFlowReports(openReport?: boolean): typeof openFlowReports | undefined { if (dryRun() || !openReport || !interactive()) { return; } diff --git a/packages/cli/src/lib/commands/init/utils.ts b/packages/cli/src/lib/commands/init/utils.ts index c219ec9b6..ac8f10ab0 100644 --- a/packages/cli/src/lib/commands/init/utils.ts +++ b/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[]): ReportFormat[] { + 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); diff --git a/packages/cli/src/lib/pre-set.ts b/packages/cli/src/lib/pre-set.ts index 88c5bb1aa..2be7d1d3b 100644 --- a/packages/cli/src/lib/pre-set.ts +++ b/packages/cli/src/lib/pre-set.ts @@ -1,5 +1,5 @@ import { ArgvPreset } from './types'; -import { detectCliMode } from './global/cli-mode/cli-mode'; +import { detectCliMode } from './global/cli-mode'; import { DEFAULT_FULL_RC_PATH } from './constants'; export const DEFAULT_PRESET: ArgvPreset = { From 5d75b2a0a37632929e37674a1afd421dcc408416 Mon Sep 17 00:00:00 2001 From: ChristopherPHolder Date: Wed, 6 Mar 2024 20:11:09 +0100 Subject: [PATCH 4/7] fix: use command value instead of getters for dryRun and interactive --- .../src/lib/commands/collect/command-impl.ts | 6 +-- .../src/lib/commands/collect/options/index.ts | 2 + .../collect/processes/collect-reports.ts | 5 ++- .../collect/utils/persist/open-report.ts | 7 ++- .../utils/persist/open-report.unit.test.ts | 45 +++++++++++-------- packages/cli/src/lib/global/options/index.ts | 4 +- .../cli/src/lib/global/options/interactive.ts | 12 ++--- 7 files changed, 44 insertions(+), 37 deletions(-) diff --git a/packages/cli/src/lib/commands/collect/command-impl.ts b/packages/cli/src/lib/commands/collect/command-impl.ts index ddd032422..5d16b4240 100644 --- a/packages/cli/src/lib/commands/collect/command-impl.ts +++ b/packages/cli/src/lib/commands/collect/command-impl.ts @@ -5,16 +5,16 @@ 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 { +export async function runCollectCommand(argv: CollectCommandOptions): Promise { const cfg = getCollectCommandOptionsFromArgv(argv); logVerbose('Collect options: ', cfg); await run([ collectRcJson, (cfg: RcJson) => startServerIfNeededAndExecute( - () => collectReports(cfg, argv.openReport), + () => collectReports(cfg, argv), cfg.collect ) ])(cfg); diff --git a/packages/cli/src/lib/commands/collect/options/index.ts b/packages/cli/src/lib/commands/collect/options/index.ts index bdf603d12..d282f0707 100644 --- a/packages/cli/src/lib/commands/collect/options/index.ts +++ b/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; export type CollectOptions = InferredOptionTypes; +export type CollectCommandOptions = CollectOptions & GlobalCliOptions; diff --git a/packages/cli/src/lib/commands/collect/processes/collect-reports.ts b/packages/cli/src/lib/commands/collect/processes/collect-reports.ts index 4b7105b92..6e122ce11 100644 --- a/packages/cli/src/lib/commands/collect/processes/collect-reports.ts +++ b/packages/cli/src/lib/commands/collect/processes/collect-reports.ts @@ -4,8 +4,9 @@ import { collectFlow, loadFlow } from '../utils/user-flow'; import { persistFlow } from '../utils/persist/persist-flow'; import { handleOpenFlowReports } from '../utils/persist/open-report'; import { RcJson } from '../../../types'; +import { CollectCommandOptions } from '../options'; -export async function collectReports(cfg: RcJson, openReport?: boolean): Promise { +export async function collectReports(cfg: RcJson, argv: CollectCommandOptions): Promise { const { collect, persist, assert } = cfg; @@ -16,7 +17,7 @@ export async function collectReports(cfg: RcJson, openReport?: boolean): Promise ...collect, ...persist, ...assert, dryRun: dryRun() }, { ...provider, path }) .then((flow) => persistFlow(flow, { ...persist, ...collect })) - .then(handleOpenFlowReports(openReport)) + .then(handleOpenFlowReports(argv)) .then(_ => cfg); }) )(cfg); diff --git a/packages/cli/src/lib/commands/collect/utils/persist/open-report.ts b/packages/cli/src/lib/commands/collect/utils/persist/open-report.ts index 5d96f774a..a819afc95 100644 --- a/packages/cli/src/lib/commands/collect/utils/persist/open-report.ts +++ b/packages/cli/src/lib/commands/collect/utils/persist/open-report.ts @@ -1,7 +1,6 @@ import * as openReport from 'open'; -import { get as dryRun } from '../../../../commands/collect/options/dryRun'; -import { get as interactive } from '../../../../global/options/interactive'; import { logVerbose } from '../../../../core/loggin'; +import { CollectCommandOptions } from '../../options'; export async function openFlowReports(fileNames: string[]): Promise { const htmlReport = fileNames.find(i => i.includes('.html')); @@ -27,8 +26,8 @@ export async function openFlowReports(fileNames: string[]): Promise { return Promise.resolve(void 0); } -export function handleOpenFlowReports(openReport?: boolean): typeof openFlowReports | undefined { - if (dryRun() || !openReport || !interactive()) { +export function handleOpenFlowReports({ dryRun, openReport, interactive}: CollectCommandOptions) { + if (dryRun || !openReport || !interactive) { return; } return openFlowReports; diff --git a/packages/cli/src/lib/commands/collect/utils/persist/open-report.unit.test.ts b/packages/cli/src/lib/commands/collect/utils/persist/open-report.unit.test.ts index 2623358da..b37df137d 100644 --- a/packages/cli/src/lib/commands/collect/utils/persist/open-report.unit.test.ts +++ b/packages/cli/src/lib/commands/collect/utils/persist/open-report.unit.test.ts @@ -1,41 +1,50 @@ +import * as openReport from 'open'; import { handleOpenFlowReports, openFlowReports } from './open-report'; import { logVerbose } from '../../../../core/loggin'; -import * as openReport from 'open'; - -import * as dryRun from '../../../../commands/collect/options/dryRun'; -import * as interactive from '../../../../global/options/interactive'; - -jest.mock('../../../../commands/collect/options/dryRun'); -jest.mock('../../../../global/options/interactive'); +import { CollectCommandOptions } from '../../options'; 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(typeof openReportsProcess).toEqual("function"); + }); + it('should return undefined if openReport is false', () => { - const openReportsProcess = handleOpenFlowReports(false); + const openReportsProcess = handleOpenFlowReports({ + openReport: false, + interactive: true, + dryRun: false, + } as CollectCommandOptions); expect(openReportsProcess).toEqual(undefined); }); it('should return undefined if dryRun is true', () => { - jest.spyOn(dryRun, 'get').mockReturnValue(true); - const openReportsProcess = handleOpenFlowReports(true); + const openReportsProcess = handleOpenFlowReports({ + openReport: true, + interactive: true, + dryRun: true, + } as CollectCommandOptions); expect(openReportsProcess).toEqual(undefined); }); it('should return undefined if interactive is false', () => { - jest.spyOn(interactive, 'get').mockReturnValue(false); - const openReportsProcess = handleOpenFlowReports(true); + const openReportsProcess = handleOpenFlowReports({ + openReport: true, + interactive: false, + dryRun: false, + } as CollectCommandOptions); expect(openReportsProcess).toEqual(undefined); }); - it('should return the openFlowReport function if openReport, interactive and not dryRun', async () => { - jest.spyOn(interactive, 'get').mockReturnValue(true); - jest.spyOn(dryRun, 'get').mockReturnValue(false); - const openReportsProcess = handleOpenFlowReports(true); - expect(typeof openReportsProcess).toEqual("function"); - }); + }); jest.mock('open'); diff --git a/packages/cli/src/lib/global/options/index.ts b/packages/cli/src/lib/global/options/index.ts index 54ec7540d..9867e9db4 100644 --- a/packages/cli/src/lib/global/options/index.ts +++ b/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; +export type GlobalCliOptions = InferredOptionTypes; export const globalOptions = { getVerbose, diff --git a/packages/cli/src/lib/global/options/interactive.ts b/packages/cli/src/lib/global/options/interactive.ts index bd8694a19..c6d20fd9f 100644 --- a/packages/cli/src/lib/global/options/interactive.ts +++ b/packages/cli/src/lib/global/options/interactive.ts @@ -1,20 +1,14 @@ import { argv, Options } from 'yargs'; import { getEnvPreset } from '../../pre-set'; -function getDefaultByCliMode(): boolean { - return getEnvPreset().interactive as boolean; -} export const interactive = { alias: 'i', type: 'boolean', description: 'When false questions are skipped with the values from the suggestions. This is useful for CI integrations.', - default: getDefaultByCliMode() + default: getEnvPreset().interactive, } satisfies Options; -const defaultValue = interactive.default; - -// We don't rely on yargs option normalization features as this can happen before cli bootstrap export function get(): boolean { - const { interactive, i } = argv as any as {interactive?: boolean, i?: boolean}; - return interactive !== undefined ? Boolean(interactive) : i !== undefined ? Boolean(i) : defaultValue; + const { interactive } = argv as any as { interactive: boolean }; + return interactive; } From dc26a9998f3e23226495f64baa263e378ce9492a Mon Sep 17 00:00:00 2001 From: ChristopherPHolder Date: Wed, 6 Mar 2024 20:25:37 +0100 Subject: [PATCH 5/7] fix: remove dry run option getter --- packages/cli/src/lib/commands/collect/options/dryRun.ts | 4 ---- .../lib/commands/collect/processes/collect-reports.ts | 5 +---- .../lib/commands/collect/utils/user-flow/collect-flow.ts | 9 +++++---- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/lib/commands/collect/options/dryRun.ts b/packages/cli/src/lib/commands/collect/options/dryRun.ts index d5a3b8818..ba74eb85c 100644 --- a/packages/cli/src/lib/commands/collect/options/dryRun.ts +++ b/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; -} diff --git a/packages/cli/src/lib/commands/collect/processes/collect-reports.ts b/packages/cli/src/lib/commands/collect/processes/collect-reports.ts index 6e122ce11..084749ca6 100644 --- a/packages/cli/src/lib/commands/collect/processes/collect-reports.ts +++ b/packages/cli/src/lib/commands/collect/processes/collect-reports.ts @@ -1,5 +1,4 @@ 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 { handleOpenFlowReports } from '../utils/persist/open-report'; @@ -13,9 +12,7 @@ export async function collectReports(cfg: RcJson, argv: CollectCommandOptions): 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(handleOpenFlowReports(argv)) .then(_ => cfg); diff --git a/packages/cli/src/lib/commands/collect/utils/user-flow/collect-flow.ts b/packages/cli/src/lib/commands/collect/utils/user-flow/collect-flow.ts index c0678851b..1b227de14 100644 --- a/packages/cli/src/lib/commands/collect/utils/user-flow/collect-flow.ts +++ b/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 }); From a1ab2dcf75aab13f964d5065f4ae0d28fd399011 Mon Sep 17 00:00:00 2001 From: ChristopherPHolder Date: Mon, 11 Mar 2024 10:15:28 +0100 Subject: [PATCH 6/7] test: fix merge conflict --- package-lock.json | 15 ++++++++------- package.json | 3 ++- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/package-lock.json b/package-lock.json index 3474d3c18..d6eafb38e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,7 +13,7 @@ "@puppeteer/replay": "^1.3.1", "@types/concurrently": "^7.0.0", "@types/puppeteer": "^5.4.7", - "@types/yargs": "^17.0.9", + "@types/yargs": "^17.0.32", "concurrently": "^7.1.0", "dotenv": "^16.4.1", "enquirer": "^2.3.6", @@ -26,6 +26,7 @@ "rxjs": "^7.8.1", "ts-node": "10.9.1", "tslib": "^2.3.1", + "yargs": "^17.7.2", "zod": "^3.22.4" }, "devDependencies": { @@ -6150,9 +6151,9 @@ } }, "node_modules/@types/yargs": { - "version": "17.0.13", - "resolved": "https://registry.npmjs.org/@types/yargs/-/yargs-17.0.13.tgz", - "integrity": "sha512-9sWaruZk2JGxIQU+IhI1fhPYRcQ0UuTNuKuCW9bR5fp7qi2Llf7WDzNa17Cy7TKnh3cdxDOiyTu6gaLS0eDatg==", + "version": "17.0.32", + "resolved": "https://registry.npmjs.org/@types/yargs/-/yargs-17.0.32.tgz", + "integrity": "sha512-xQ67Yc/laOG5uMfX/093MRlGGCIBzZMarVa+gfNKJxWAIgykYpVGkBdbqEzGDDfCrVUj6Hiff4mTZ5BA6TmAog==", "dependencies": { "@types/yargs-parser": "*" } @@ -26276,9 +26277,9 @@ } }, "@types/yargs": { - "version": "17.0.13", - "resolved": "https://registry.npmjs.org/@types/yargs/-/yargs-17.0.13.tgz", - "integrity": "sha512-9sWaruZk2JGxIQU+IhI1fhPYRcQ0UuTNuKuCW9bR5fp7qi2Llf7WDzNa17Cy7TKnh3cdxDOiyTu6gaLS0eDatg==", + "version": "17.0.32", + "resolved": "https://registry.npmjs.org/@types/yargs/-/yargs-17.0.32.tgz", + "integrity": "sha512-xQ67Yc/laOG5uMfX/093MRlGGCIBzZMarVa+gfNKJxWAIgykYpVGkBdbqEzGDDfCrVUj6Hiff4mTZ5BA6TmAog==", "requires": { "@types/yargs-parser": "*" } diff --git a/package.json b/package.json index 08b647cc0..87d9920dc 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,7 @@ "@puppeteer/replay": "^1.3.1", "@types/concurrently": "^7.0.0", "@types/puppeteer": "^5.4.7", - "@types/yargs": "^17.0.9", + "@types/yargs": "^17.0.32", "concurrently": "^7.1.0", "dotenv": "^16.4.1", "enquirer": "^2.3.6", @@ -37,6 +37,7 @@ "rxjs": "^7.8.1", "ts-node": "10.9.1", "tslib": "^2.3.1", + "yargs": "^17.7.2", "zod": "^3.22.4" }, "devDependencies": { From a716aecb9242297753fb9fb51e23d4037324a99b Mon Sep 17 00:00:00 2001 From: ChristopherPHolder Date: Mon, 11 Mar 2024 10:30:58 +0100 Subject: [PATCH 7/7] test: simplify test --- .../utils/persist/open-report.unit.test.ts | 57 +++++-------------- packages/cli/src/lib/commands/init/utils.ts | 2 +- 2 files changed, 14 insertions(+), 45 deletions(-) diff --git a/packages/cli/src/lib/commands/collect/utils/persist/open-report.unit.test.ts b/packages/cli/src/lib/commands/collect/utils/persist/open-report.unit.test.ts index b37df137d..0e43eb5fb 100644 --- a/packages/cli/src/lib/commands/collect/utils/persist/open-report.unit.test.ts +++ b/packages/cli/src/lib/commands/collect/utils/persist/open-report.unit.test.ts @@ -3,7 +3,11 @@ 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(); }) @@ -14,7 +18,7 @@ describe('handleOpenFlowReport', () => { interactive: true, dryRun: false, } as CollectCommandOptions); - expect(typeof openReportsProcess).toEqual("function"); + expect(openReportsProcess).toEqual(expect.any(Function)); }); it('should return undefined if openReport is false', () => { @@ -23,7 +27,7 @@ describe('handleOpenFlowReport', () => { interactive: true, dryRun: false, } as CollectCommandOptions); - expect(openReportsProcess).toEqual(undefined); + expect(openReportsProcess).toBeUndefined(); }); it('should return undefined if dryRun is true', () => { @@ -32,7 +36,7 @@ describe('handleOpenFlowReport', () => { interactive: true, dryRun: true, } as CollectCommandOptions); - expect(openReportsProcess).toEqual(undefined); + expect(openReportsProcess).toBeUndefined(); }); it('should return undefined if interactive is false', () => { @@ -41,15 +45,10 @@ describe('handleOpenFlowReport', () => { interactive: false, dryRun: false, } as CollectCommandOptions); - expect(openReportsProcess).toEqual(undefined); + expect(openReportsProcess).toBeUndefined(); }); - - }); -jest.mock('open'); -jest.mock('../../../../core/loggin'); - describe('openReports', () => { beforeEach(() => { @@ -61,48 +60,18 @@ describe('openReports', () => { expect(openReport).not.toHaveBeenCalled(); }); - it('should not logVerbose if no file name is passed', async () => { - await openFlowReports([]); - expect(logVerbose).not.toHaveBeenCalled(); - }); - - it('should open the report if filenames includes an html report', async () => { - await openFlowReports(['example.html']); - expect(openReport).toHaveBeenCalled(); - }); - - it('should logVerbose if filenames includes an html report', async () => { - await openFlowReports(['example.html']); - expect(logVerbose).toHaveBeenCalledWith(expect.stringContaining('HTML')); - }); - - it('should open the report if filenames includes a json report', async () => { - await openFlowReports(['example.json']); + it.each(['html', 'json', 'md'])('should open the %s report', async (format) => { + await openFlowReports([`example.${format}`]); expect(openReport).toHaveBeenCalled(); }); - it('should logVerbose if filenames includes an json report', async () => { - await openFlowReports(['example.json']); - expect(logVerbose).toHaveBeenCalledWith(expect.stringContaining('JSON')); - }); - - it('should open the report if filenames includes a md report', async () => { - await openFlowReports(['example.md']); - expect(openReport).toHaveBeenCalled(); - }); - - it('should logVerbose if filenames includes an md report', async () => { - await openFlowReports(['example.md']); - expect(logVerbose).toHaveBeenCalledWith(expect.stringContaining('Markdown')); + 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); }); - - it('should only logVerbose 1 time report if multiple report formats are passed', async () => { - await openFlowReports(['example.html', 'example.json', 'example.md']); - expect(logVerbose).toHaveBeenCalledTimes(1); - }); }); diff --git a/packages/cli/src/lib/commands/init/utils.ts b/packages/cli/src/lib/commands/init/utils.ts index ac8f10ab0..7f9111869 100644 --- a/packages/cli/src/lib/commands/init/utils.ts +++ b/packages/cli/src/lib/commands/init/utils.ts @@ -5,7 +5,7 @@ import { REPORT_FORMAT_VALUES } from '../collect/constants'; const isValidFormat = (value: any): value is ReportFormat => REPORT_FORMAT_VALUES.includes(value); -function sanitizedFormats(formats: string[]): ReportFormat[] { +function sanitizedFormats(formats: string[]) { const validatedFormats: ReportFormat[] = formats.filter(isValidFormat); if (validatedFormats.length !== formats.length) { throw new Error(`${formats} contains invalid format options`);