diff --git a/.gitignore b/.gitignore index 73a946634..4284058f4 100644 --- a/.gitignore +++ b/.gitignore @@ -44,7 +44,7 @@ testem.log Thumbs.db # generated Code PushUp reports -**/.code-pushup +/.code-pushup # Nx workspace cache .nx diff --git a/e2e/cli-e2e/mocks/fixtures/existing-reports/.code-pushup/target-report.json b/e2e/cli-e2e/mocks/fixtures/existing-reports/.code-pushup/report-after.json similarity index 100% rename from e2e/cli-e2e/mocks/fixtures/existing-reports/.code-pushup/target-report.json rename to e2e/cli-e2e/mocks/fixtures/existing-reports/.code-pushup/report-after.json diff --git a/e2e/cli-e2e/mocks/fixtures/existing-reports/.code-pushup/target-report.md b/e2e/cli-e2e/mocks/fixtures/existing-reports/.code-pushup/report-after.md similarity index 100% rename from e2e/cli-e2e/mocks/fixtures/existing-reports/.code-pushup/target-report.md rename to e2e/cli-e2e/mocks/fixtures/existing-reports/.code-pushup/report-after.md diff --git a/e2e/cli-e2e/mocks/fixtures/existing-reports/.code-pushup/source-report.json b/e2e/cli-e2e/mocks/fixtures/existing-reports/.code-pushup/report-before.json similarity index 100% rename from e2e/cli-e2e/mocks/fixtures/existing-reports/.code-pushup/source-report.json rename to e2e/cli-e2e/mocks/fixtures/existing-reports/.code-pushup/report-before.json diff --git a/e2e/cli-e2e/mocks/fixtures/existing-reports/.code-pushup/source-report.md b/e2e/cli-e2e/mocks/fixtures/existing-reports/.code-pushup/report-before.md similarity index 100% rename from e2e/cli-e2e/mocks/fixtures/existing-reports/.code-pushup/source-report.md rename to e2e/cli-e2e/mocks/fixtures/existing-reports/.code-pushup/report-before.md diff --git a/e2e/cli-e2e/tests/compare.e2e.test.ts b/e2e/cli-e2e/tests/compare.e2e.test.ts index d7510d7f9..4108d9b9e 100644 --- a/e2e/cli-e2e/tests/compare.e2e.test.ts +++ b/e2e/cli-e2e/tests/compare.e2e.test.ts @@ -39,12 +39,7 @@ describe('CLI compare', () => { it('should compare report.json files and create report-diff.json and report-diff.md', async () => { await executeProcess({ command: 'npx', - args: [ - '@code-pushup/cli', - 'compare', - `--before=${path.join('.code-pushup', 'source-report.json')}`, - `--after=${path.join('.code-pushup', 'target-report.json')}`, - ], + args: ['@code-pushup/cli', 'compare'], cwd: existingDir, }); diff --git a/packages/ci/src/lib/cli/commands/compare.ts b/packages/ci/src/lib/cli/commands/compare.ts index 4321e8e5a..6ce6d0726 100644 --- a/packages/ci/src/lib/cli/commands/compare.ts +++ b/packages/ci/src/lib/cli/commands/compare.ts @@ -2,26 +2,19 @@ import { DEFAULT_PERSIST_FORMAT } from '@code-pushup/models'; import { executeProcess, isVerbose } from '@code-pushup/utils'; import type { CommandContext } from '../context.js'; -type CompareOptions = { - before: string; - after: string; - label?: string; -}; - export async function runCompare( - { before, after, label }: CompareOptions, { bin, config, directory, observer }: CommandContext, + { hasFormats }: { hasFormats: boolean }, ): Promise { await executeProcess({ command: bin, args: [ 'compare', ...(isVerbose() ? ['--verbose'] : []), - `--before=${before}`, - `--after=${after}`, - ...(label ? [`--label=${label}`] : []), ...(config ? [`--config=${config}`] : []), - ...DEFAULT_PERSIST_FORMAT.map(format => `--persist.format=${format}`), + ...(hasFormats + ? [] + : DEFAULT_PERSIST_FORMAT.map(format => `--persist.format=${format}`)), ], cwd: directory, observer, diff --git a/packages/ci/src/lib/cli/persist.ts b/packages/ci/src/lib/cli/persist.ts index 843f13d60..b349f5160 100644 --- a/packages/ci/src/lib/cli/persist.ts +++ b/packages/ci/src/lib/cli/persist.ts @@ -9,7 +9,7 @@ import { persistConfigSchema, uploadConfigSchema, } from '@code-pushup/models'; -import { objectFromEntries } from '@code-pushup/utils'; +import { createReportPath, objectFromEntries } from '@code-pushup/utils'; export type EnhancedPersistConfig = Pick; @@ -27,12 +27,12 @@ export function persistedFilesFromConfig( const dir = path.isAbsolute(outputDir) ? outputDir : path.join(directory, outputDir); - const name = isDiff ? `${filename}-diff` : filename; + const suffix = isDiff ? 'diff' : undefined; return objectFromEntries( DEFAULT_PERSIST_FORMAT.map(format => [ format, - path.join(dir, `${name}.${format}`), + createReportPath({ outputDir: dir, filename, format, suffix }), ]), ); } diff --git a/packages/ci/src/lib/run-monorepo.ts b/packages/ci/src/lib/run-monorepo.ts index 8216f3543..7b6786632 100644 --- a/packages/ci/src/lib/run-monorepo.ts +++ b/packages/ci/src/lib/run-monorepo.ts @@ -11,6 +11,7 @@ import { createCommandContext, persistedFilesFromConfig, runCollect, + runCompare, runMergeDiffs, } from './cli/index.js'; import { commentOnPR } from './comment.js'; @@ -29,16 +30,18 @@ import { import { saveOutputFiles } from './output-files.js'; import { type BaseReportArgs, + type CompareReportsArgs, type ReportData, type RunEnv, checkPrintConfig, - compareReports, configFromPatterns, hasDefaultPersistFormats, loadCachedBaseReport, + prepareReportFilesToCompare, printPersistConfig, runInBaseBranch, runOnProject, + saveDiffFiles, saveReportFiles, } from './run-utils.js'; @@ -226,18 +229,38 @@ async function compareProjectsInBulk( ...args, prevReport: args.prevReport || collectedPrevReports[args.project.name], })) - .filter(hasNoNullableProps); + .filter(hasNoNullableProps) satisfies CompareReportsArgs[]; - const projectComparisons = Object.fromEntries( - await asyncSequential(projectsToCompare, async args => [ - args.project.name, - await compareReports(args), - ]), + const projectComparisons = await compareManyProjects( + projectsToCompare, + runManyCommand, + env, ); return finalizeProjectReports(currProjectReports, projectComparisons); } +async function compareManyProjects( + projectsToCompare: ExcludeNullableProps[], + runManyCommand: RunManyCommand, + env: RunEnv, +): Promise> { + await Promise.all(projectsToCompare.map(prepareReportFilesToCompare)); + + await compareMany(runManyCommand, env, { + hasFormats: allProjectsHaveDefaultPersistFormats(projectsToCompare), + }); + + return Object.fromEntries( + await Promise.all( + projectsToCompare.map(async args => [ + args.project.name, + await saveDiffFiles(args), + ]), + ), + ); +} + function finalizeProjectReports( projectReports: ProjectReport[], projectComparisons?: Record, @@ -352,6 +375,26 @@ async function collectMany( ); } +async function compareMany( + runManyCommand: RunManyCommand, + env: RunEnv, + options: { + hasFormats: boolean; + }, +): Promise { + const { settings } = env; + const { hasFormats } = options; + + const ctx: CommandContext = { + ...createCommandContext(settings, null), + bin: await runManyCommand(), + }; + + await runCompare(ctx, { hasFormats }); + + settings.logger.debug('Compared all project reports'); +} + export function allProjectsHaveDefaultPersistFormats( projects: { config: EnhancedPersistConfig }[], ): boolean { diff --git a/packages/ci/src/lib/run-utils.ts b/packages/ci/src/lib/run-utils.ts index c00c82bdf..74f2dda5b 100644 --- a/packages/ci/src/lib/run-utils.ts +++ b/packages/ci/src/lib/run-utils.ts @@ -1,13 +1,20 @@ /* eslint-disable max-lines */ -import { readFile } from 'node:fs/promises'; +import { readFile, writeFile } from 'node:fs/promises'; +import path from 'node:path'; import type { SimpleGit } from 'simple-git'; import { + DEFAULT_PERSIST_FILENAME, DEFAULT_PERSIST_FORMAT, + DEFAULT_PERSIST_OUTPUT_DIR, type Report, type ReportsDiff, } from '@code-pushup/models'; import { + type Diff, + createReportPath, interpolate, + objectFromEntries, + readJsonFile, removeUndefinedAndEmptyProps, stringifyError, } from '@code-pushup/utils'; @@ -53,6 +60,7 @@ type NormalizedGitRefs = { export type CompareReportsArgs = { project: ProjectConfig | null; env: RunEnv; + ctx: CommandContext; base: GitBranch; currReport: ReportData<'current'>; prevReport: ReportData<'previous'>; @@ -152,38 +160,91 @@ export async function runOnProject( return noDiffOutput; } - const compareArgs = { project, env, base, config, currReport, prevReport }; + const compareArgs = { ...baseArgs, currReport, prevReport }; return compareReports(compareArgs); } export async function compareReports( args: CompareReportsArgs, ): Promise { + const { ctx, env, config } = args; + const { logger } = env.settings; + + await prepareReportFilesToCompare(args); + await runCompare(ctx, { hasFormats: hasDefaultPersistFormats(config) }); + + logger.info('Compared reports and generated diff files'); + + return saveDiffFiles(args); +} + +export async function prepareReportFilesToCompare( + args: CompareReportsArgs, +): Promise> { + const { config, project, env, ctx } = args; + const { + outputDir = DEFAULT_PERSIST_OUTPUT_DIR, + filename = DEFAULT_PERSIST_FILENAME, + } = config.persist ?? {}; + const label = project?.name; + const { logger } = env.settings; + + const originalReports = await Promise.all( + [args.currReport, args.prevReport].map(({ files }) => + readJsonFile(files.json), + ), + ); + const labeledReports = label + ? originalReports.map(report => ({ ...report, label })) + : originalReports; + + const reportPaths = labeledReports.map((report, idx) => { + const key: keyof Diff = idx === 0 ? 'after' : 'before'; + const filePath = createReportPath({ + outputDir: path.resolve(ctx.directory, outputDir), + filename, + format: 'json', + suffix: key, + }); + return { key, report, filePath }; + }); + + await Promise.all( + reportPaths.map(({ filePath, report }) => + writeFile(filePath, JSON.stringify(report, null, 2)), + ), + ); + + logger.debug( + [ + 'Prepared', + project && `"${project.name}" project's`, + 'report files for comparison', + `at ${reportPaths.map(({ filePath }) => filePath).join(' and ')}`, + ] + .filter(Boolean) + .join(' '), + ); + + return objectFromEntries( + reportPaths.map(({ key, filePath }) => [key, filePath]), + ); +} + +export async function saveDiffFiles(args: CompareReportsArgs) { const { project, + ctx, env: { settings }, currReport, prevReport, config, } = args; - const logger = settings.logger; - - const ctx = createCommandContext(settings, project); - await runCompare( - { - before: prevReport.files.json, - after: currReport.files.json, - label: project?.name, - }, - ctx, - ); const diffFiles = persistedFilesFromConfig(config, { directory: ctx.directory, isDiff: true, }); - logger.info('Compared reports and generated diff files'); - logger.debug(`Generated diff files at ${diffFiles.json} and ${diffFiles.md}`); return { name: projectToName(project), diff --git a/packages/ci/src/lib/run.int.test.ts b/packages/ci/src/lib/run.int.test.ts index 67393a206..8dcda5e95 100644 --- a/packages/ci/src/lib/run.int.test.ts +++ b/packages/ci/src/lib/run.int.test.ts @@ -139,11 +139,26 @@ describe('runInCI', () => { await mkdir(outputDir, { recursive: true }); let stdout = ''; + const isBulkCommand = /workspaces|concurrency|parallel/.test(command); + const projectOutputDirs = ['cli', 'core', 'utils'].map(project => + path.join(workDir, `packages/${project}/.code-pushup`), + ); + switch (args![0]) { case 'compare': const diffs = fixturePaths.diffs.project; - await copyFile(diffs.json, path.join(outputDir, 'report-diff.json')); - await copyFile(diffs.md, path.join(outputDir, 'report-diff.md')); + if (isBulkCommand) { + await Promise.all( + projectOutputDirs.map(async dir => { + await mkdir(dir, { recursive: true }); + await copyFile(diffs.json, path.join(dir, 'report-diff.json')); + await copyFile(diffs.md, path.join(dir, 'report-diff.md')); + }), + ); + } else { + await copyFile(diffs.json, path.join(outputDir, 'report-diff.json')); + await copyFile(diffs.md, path.join(outputDir, 'report-diff.md')); + } break; case 'print-config': @@ -186,23 +201,14 @@ describe('runInCI', () => { const kind = (await git.branch()).current === 'main' ? 'before' : 'after'; const reports = fixturePaths.reports[kind]; - if (/workspaces|concurrency|parallel/.test(command)) { - // eslint-disable-next-line functional/no-loop-statements - for (const project of ['cli', 'core', 'utils']) { - const projectOutputDir = path.join( - workDir, - `packages/${project}/.code-pushup`, - ); - await mkdir(projectOutputDir, { recursive: true }); - await copyFile( - reports.json, - path.join(projectOutputDir, 'report.json'), - ); - await copyFile( - reports.json, - path.join(projectOutputDir, 'report.md'), - ); - } + if (isBulkCommand) { + await Promise.all( + projectOutputDirs.map(async dir => { + await mkdir(dir, { recursive: true }); + await copyFile(reports.json, path.join(dir, 'report.json')); + await copyFile(reports.md, path.join(dir, 'report.md')); + }), + ); } else { await copyFile(reports.json, path.join(outputDir, 'report.json')); await copyFile(reports.md, path.join(outputDir, 'report.md')); @@ -386,13 +392,7 @@ describe('runInCI', () => { } satisfies utils.ProcessConfig); expect(utils.executeProcess).toHaveBeenNthCalledWith(5, { command: options.bin, - args: [ - 'compare', - `--before=${path.join(outputDir, '.previous/report.json')}`, - `--after=${path.join(outputDir, '.current/report.json')}`, - '--persist.format=json', - '--persist.format=md', - ], + args: ['compare'], cwd: workDir, observer: expectedObserver, } satisfies utils.ProcessConfig); @@ -458,13 +458,7 @@ describe('runInCI', () => { } satisfies utils.ProcessConfig); expect(utils.executeProcess).toHaveBeenNthCalledWith(3, { command: options.bin, - args: [ - 'compare', - `--before=${path.join(outputDir, '.previous/report.json')}`, - `--after=${path.join(outputDir, '.current/report.json')}`, - '--persist.format=json', - '--persist.format=md', - ], + args: ['compare'], cwd: workDir, observer: expectedObserver, } satisfies utils.ProcessConfig); @@ -531,13 +525,7 @@ describe('runInCI', () => { } satisfies utils.ProcessConfig); expect(utils.executeProcess).toHaveBeenNthCalledWith(3, { command: options.bin, - args: [ - 'compare', - `--before=${path.join(outputDir, '.previous/report.json')}`, - `--after=${path.join(outputDir, '.current/report.json')}`, - '--persist.format=json', - '--persist.format=md', - ], + args: ['compare'], cwd: workDir, observer: expectedObserver, } satisfies utils.ProcessConfig); @@ -944,13 +932,13 @@ describe('runInCI', () => { // 3 print-configs for each project // 1 print-config for uncached project // 1 autorun for uncached projects - // 3 compares for each project + // 1 compare for all projects // 1 merge-diffs for all projects expect( executeProcessSpy.mock.calls.filter(([cfg]) => cfg.command.includes('code-pushup'), ), - ).toHaveLength(10); + ).toHaveLength(8); expect(utils.executeProcess).toHaveBeenCalledWith({ command: run, args: [ @@ -967,15 +955,8 @@ describe('runInCI', () => { observer: expectedObserver, } satisfies utils.ProcessConfig); expect(utils.executeProcess).toHaveBeenCalledWith({ - command: run, - args: [ - 'compare', - expect.stringMatching(/^--before=.*\.previous[/\\]report\.json$/), - expect.stringMatching(/^--after=.*\.current[/\\]report\.json$/), - expect.stringMatching(/^--label=\w+$/), - '--persist.format=json', - '--persist.format=md', - ], + command: runMany, + args: ['compare'], cwd: expect.stringContaining(workDir), observer: expectedObserver, } satisfies utils.ProcessConfig); @@ -1060,13 +1041,13 @@ describe('runInCI', () => { // 1 autorun for all projects // 1 autorun for uncached projects - // 3 compares for each project + // 1 compare for all projects // 1 merge-diffs for all projects expect( executeProcessSpy.mock.calls.filter(([cfg]) => cfg.command.includes('code-pushup'), ), - ).toHaveLength(6); + ).toHaveLength(4); expect(utils.executeProcess).toHaveBeenCalledWith({ command: runMany, args: [], @@ -1074,15 +1055,8 @@ describe('runInCI', () => { observer: expectedObserver, } satisfies utils.ProcessConfig); expect(utils.executeProcess).toHaveBeenCalledWith({ - command: run, - args: [ - 'compare', - expect.stringMatching(/^--before=.*\.previous[/\\]report\.json$/), - expect.stringMatching(/^--after=.*\.current[/\\]report\.json$/), - expect.stringMatching(/^--label=\w+$/), - '--persist.format=json', - '--persist.format=md', - ], + command: runMany, + args: ['compare'], cwd: expect.stringContaining(workDir), observer: expectedObserver, } satisfies utils.ProcessConfig); @@ -1161,7 +1135,7 @@ describe('runInCI', () => { observer: expectedObserver, } satisfies utils.ProcessConfig); expect(utils.executeProcess).toHaveBeenCalledWith({ - command: run, + command: runMany, args: expect.arrayContaining(['compare']), cwd: expect.stringContaining(workDir), observer: expectedObserver, @@ -1441,14 +1415,7 @@ describe('runInCI', () => { } satisfies utils.ProcessConfig); expect(utils.executeProcess).toHaveBeenCalledWith({ command: options.bin, - args: [ - 'compare', - expect.stringMatching(/^--before=.*\.previous[/\\]report\.json$/), - expect.stringMatching(/^--after=.*\.current[/\\]report\.json$/), - expect.stringMatching(/^--label=\w+$/), - '--persist.format=json', - '--persist.format=md', - ], + args: ['compare'], cwd: expect.stringContaining(workDir), observer: expectedObserver, } satisfies utils.ProcessConfig); diff --git a/packages/cli/README.md b/packages/cli/README.md index 38bb77e73..0d09ad1ad 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -279,7 +279,7 @@ Refer to the [Common Command Options](#common-command-options) for the list of a | **`--from`** | `string` | n/a | Hash to start in history | | **`--to`** | `string` | n/a | Hash to end in history | -### `compare` command +#### `compare` command Usage: `code-pushup compare --before SOURCE_PATH --after TARGET_PATH [options]` @@ -289,11 +289,15 @@ Compare 2 reports and produce a report diff file. In addition to the [Common Command Options](#common-command-options), the following options are recognized by the `compare` command: -| Option | Required | Type | Description | -| -------------- | :------: | -------- | ----------------------------------- | -| **`--before`** | yes | `string` | Path to source `report.json`. | -| **`--after`** | yes | `string` | Path to target `report.json`. | -| **`--label`** | no | `string` | Label for diff (e.g. project name). | +| Option | Type | Default | Description | +| -------------- | -------- | -------------------------------------- | ----------------------------------- | +| **`--before`** | `string` | `.code-pushup/report-before.json` [^1] | Path to source `report.json`. | +| **`--after`** | `string` | `.code-pushup/report-after.json` [^1] | Path to target `report.json`. | +| **`--label`** | `string` | n/a [^2] | Label for diff (e.g. project name). | + +[^1]: Uses `persist` config to determine report paths, so default file paths are actually `${persist.outputDir}/${persist.filename}-before.json` and `${persist.outputDir}/${persist.filename}-after.json`. + +[^2]: Uses `label` from input `report.json` files is they both have the same value. #### `print-config` command @@ -309,7 +313,7 @@ In addition to the [Common Command Options](#common-command-options), the follow | -------------- | :------: | -------- | -------------------------------------------------------- | | **`--output`** | no | `string` | Path to output file to print config (default is stdout). | -### `merge-diffs` command +#### `merge-diffs` command Usage: `code-pushup merge-diffs --files PATH_1 PATH_2 ... [options]` diff --git a/packages/cli/src/lib/compare/compare-command.ts b/packages/cli/src/lib/compare/compare-command.ts index c098fb848..3934d06f8 100644 --- a/packages/cli/src/lib/compare/compare-command.ts +++ b/packages/cli/src/lib/compare/compare-command.ts @@ -1,10 +1,9 @@ import { bold, gray } from 'ansis'; import type { CommandModule } from 'yargs'; -import { compareReportFiles } from '@code-pushup/core'; +import { type CompareOptions, compareReportFiles } from '@code-pushup/core'; import type { PersistConfig, UploadConfig } from '@code-pushup/models'; import { ui } from '@code-pushup/utils'; import { CLI_NAME } from '../constants.js'; -import type { CompareOptions } from '../implementation/compare.model.js'; import { yargsCompareOptionsDefinition } from '../implementation/compare.options.js'; export function yargsCompareCommandObject() { @@ -25,10 +24,8 @@ export function yargsCompareCommandObject() { const { before, after, label, persist, upload } = options; const outputPaths = await compareReportFiles( - { before, after }, - persist, - upload, - label, + { persist, upload }, + { before, after, label }, ); ui().logger.info( diff --git a/packages/cli/src/lib/compare/compare-command.unit.test.ts b/packages/cli/src/lib/compare/compare-command.unit.test.ts index 571d5b1e2..b2279fd80 100644 --- a/packages/cli/src/lib/compare/compare-command.unit.test.ts +++ b/packages/cli/src/lib/compare/compare-command.unit.test.ts @@ -36,14 +36,18 @@ describe('compare-command', () => { expect(compareReportFiles).toHaveBeenCalledWith< Parameters >( - { before: 'source-report.json', after: 'target-report.json' }, { - outputDir: DEFAULT_PERSIST_OUTPUT_DIR, - filename: DEFAULT_PERSIST_FILENAME, - format: DEFAULT_PERSIST_FORMAT, + persist: { + outputDir: DEFAULT_PERSIST_OUTPUT_DIR, + filename: DEFAULT_PERSIST_FILENAME, + format: DEFAULT_PERSIST_FORMAT, + }, + upload: expect.any(Object), + }, + { + before: 'source-report.json', + after: 'target-report.json', }, - expect.any(Object), - undefined, ); }); @@ -60,19 +64,18 @@ describe('compare-command', () => { expect(compareReportFiles).toHaveBeenCalledWith< Parameters - >( - { before: 'source-report.json', after: 'target-report.json' }, - expect.any(Object), - expect.any(Object), - 'core', - ); + >(expect.any(Object), { + before: 'source-report.json', + after: 'target-report.json', + label: 'core', + }); }); it('should log output paths to stdout', async () => { - await yargsCli( - ['compare', '--before=source-report.json', '--after=target-report.json'], - { ...DEFAULT_CLI_CONFIGURATION, commands: [yargsCompareCommandObject()] }, - ).parseAsync(); + await yargsCli(['compare'], { + ...DEFAULT_CLI_CONFIGURATION, + commands: [yargsCompareCommandObject()], + }).parseAsync(); expect(ui()).toHaveLogged( 'info', diff --git a/packages/cli/src/lib/implementation/compare.model.ts b/packages/cli/src/lib/implementation/compare.model.ts deleted file mode 100644 index fccf191f3..000000000 --- a/packages/cli/src/lib/implementation/compare.model.ts +++ /dev/null @@ -1,3 +0,0 @@ -import type { Diff } from '@code-pushup/utils'; - -export type CompareOptions = Diff & { label?: string }; diff --git a/packages/cli/src/lib/implementation/compare.options.ts b/packages/cli/src/lib/implementation/compare.options.ts index d7ab52985..b6dd4a5e2 100644 --- a/packages/cli/src/lib/implementation/compare.options.ts +++ b/packages/cli/src/lib/implementation/compare.options.ts @@ -1,5 +1,5 @@ import type { Options } from 'yargs'; -import type { CompareOptions } from './compare.model.js'; +import type { CompareOptions } from '@code-pushup/core'; export function yargsCompareOptionsDefinition(): Record< keyof CompareOptions, @@ -9,12 +9,10 @@ export function yargsCompareOptionsDefinition(): Record< before: { describe: 'Path to source report.json', type: 'string', - demandOption: true, }, after: { describe: 'Path to target report.json', type: 'string', - demandOption: true, }, label: { describe: 'Label for diff (e.g. project name)', diff --git a/packages/cli/src/lib/yargs-cli.int.test.ts b/packages/cli/src/lib/yargs-cli.int.test.ts index 7865108cc..a6ac7bd6b 100644 --- a/packages/cli/src/lib/yargs-cli.int.test.ts +++ b/packages/cli/src/lib/yargs-cli.int.test.ts @@ -160,15 +160,6 @@ describe('yargsCli', () => { expect(parsedArgv.after).toBe('target-report.json'); }); - it('should error if required compare option is missing', () => { - expect(() => - yargsCli([], { - options: { ...options, ...yargsCompareOptionsDefinition() }, - noExitProcess: true, - }).parse(), - ).toThrow('Missing required arguments: before, after'); - }); - it('should parse merge-diffs options', async () => { const parsedArgv = await yargsCli( [ diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 0dc27e77e..25cf1d343 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1,14 +1,23 @@ export { - type CollectAndPersistReportsOptions, collectAndPersistReports, + type CollectAndPersistReportsOptions, } from './lib/collect-and-persist.js'; -export { compareReportFiles, compareReports } from './lib/compare.js'; -export { type CollectOptions, collect } from './lib/implementation/collect.js'; +export { + compareReportFiles, + compareReports, + type CompareOptions, +} from './lib/compare.js'; +export { + history, + type HistoryOnlyOptions, + type HistoryOptions, +} from './lib/history.js'; +export { collect, type CollectOptions } from './lib/implementation/collect.js'; export type { ReportsToCompare } from './lib/implementation/compare-scorables.js'; export { - PluginOutputMissingAuditError, executePlugin, executePlugins, + PluginOutputMissingAuditError, } from './lib/implementation/execute-plugin.js'; export { PersistDirError, @@ -16,15 +25,10 @@ export { persistReport, } from './lib/implementation/persist.js'; export { - history, - type HistoryOptions, - type HistoryOnlyOptions, -} from './lib/history.js'; -export { - ConfigPathError, autoloadRc, + ConfigPathError, readRcByPath, } from './lib/implementation/read-rc-file.js'; -export type { GlobalOptions } from './lib/types.js'; -export { type UploadOptions, upload } from './lib/upload.js'; export { mergeDiffs } from './lib/merge-diffs.js'; +export type { GlobalOptions } from './lib/types.js'; +export { upload, type UploadOptions } from './lib/upload.js'; diff --git a/packages/core/src/lib/compare.ts b/packages/core/src/lib/compare.ts index c56ff63d8..13d09ab3d 100644 --- a/packages/core/src/lib/compare.ts +++ b/packages/core/src/lib/compare.ts @@ -1,6 +1,5 @@ import { writeFile } from 'node:fs/promises'; import { createRequire } from 'node:module'; -import path from 'node:path'; import { type Format, type PersistConfig, @@ -12,6 +11,7 @@ import { import { type Diff, calcDuration, + createReportPath, ensureDirectoryExists, generateMdReportsDiff, readJsonFile, @@ -26,17 +26,27 @@ import { } from './implementation/compare-scorables.js'; import { loadPortalClient } from './load-portal-client.js'; +export type CompareOptions = { + before?: string; + after?: string; + label?: string; +}; + export async function compareReportFiles( - inputPaths: Diff, - persistConfig: Required, - uploadConfig: UploadConfig | undefined, - label?: string, + config: { + persist: Required; + upload?: UploadConfig; + }, + options?: CompareOptions, ): Promise { - const { outputDir, filename, format } = persistConfig; + const { outputDir, filename, format } = config.persist; + + const defaultInputPath = (suffix: keyof Diff) => + createReportPath({ outputDir, filename, format: 'json', suffix }); const [reportBefore, reportAfter] = await Promise.all([ - readJsonFile(inputPaths.before), - readJsonFile(inputPaths.after), + readJsonFile(options?.before ?? defaultInputPath('before')), + readJsonFile(options?.after ?? defaultInputPath('after')), ]); const reports: Diff = { before: reportSchema.parse(reportBefore), @@ -44,22 +54,27 @@ export async function compareReportFiles( }; const diff = compareReports(reports); - if (label) { - // eslint-disable-next-line functional/immutable-data - diff.label = label; - } - if (uploadConfig && diff.commits) { - // eslint-disable-next-line functional/immutable-data - diff.portalUrl = await fetchPortalComparisonLink( - uploadConfig, - diff.commits, - ); - } + + const label = options?.label ?? getLabelFromReports(reports); + const portalUrl = + config.upload && + diff.commits && + (await fetchPortalComparisonLink(config.upload, diff.commits)); + + const diffWithLinks: ReportsDiff = + label || portalUrl + ? { ...diff, ...(label && { label }), ...(portalUrl && { portalUrl }) } + : diff; return Promise.all( format.map(async fmt => { - const outputPath = path.join(outputDir, `${filename}-diff.${fmt}`); - const content = reportsDiffToFileContent(diff, fmt); + const outputPath = createReportPath({ + outputDir, + filename, + format: fmt, + suffix: 'diff', + }); + const content = reportsDiffToFileContent(diffWithLinks, fmt); await ensureDirectoryExists(outputDir); await writeFile(outputPath, content); return outputPath; @@ -146,3 +161,14 @@ async function fetchPortalComparisonLink( throw error; } } + +function getLabelFromReports(reports: Diff): string | undefined { + if ( + reports.before.label && + reports.after.label && + reports.before.label === reports.after.label + ) { + return reports.after.label; + } + return undefined; +} diff --git a/packages/core/src/lib/compare.unit.test.ts b/packages/core/src/lib/compare.unit.test.ts index d1ea58ddd..303aaa7cf 100644 --- a/packages/core/src/lib/compare.unit.test.ts +++ b/packages/core/src/lib/compare.unit.test.ts @@ -1,9 +1,10 @@ import { vol } from 'memfs'; -import { readFile } from 'node:fs/promises'; +import * as fs from 'node:fs/promises'; import path from 'node:path'; import { getPortalComparisonLink } from '@code-pushup/portal-client'; import { type Commit, + type PersistConfig, type Report, reportsDiffSchema, } from '@code-pushup/models'; @@ -24,26 +25,65 @@ describe('compareReportFiles', () => { after: REPORT_MOCK.commit!.hash, }; + const persistConfig: Required = { + outputDir: MEMFS_VOLUME, + filename: 'report', + format: ['json', 'md'], + }; + + beforeAll(() => { + vi.spyOn(fs, 'readFile'); + }); + beforeEach(() => { vol.fromJSON( { - 'source-report.json': JSON.stringify(MINIMAL_REPORT_MOCK), - 'target-report.json': JSON.stringify(REPORT_MOCK), + 'report-before.json': JSON.stringify(MINIMAL_REPORT_MOCK), + 'report-after.json': JSON.stringify(REPORT_MOCK), }, MEMFS_VOLUME, ); }); - it('should create valid report-diff.json from report.json files', async () => { + it('should read report files from default locations', async () => { + await compareReportFiles({ persist: persistConfig }); + + expect(fs.readFile).toHaveBeenCalledWith( + path.join(MEMFS_VOLUME, 'report-before.json'), + ); + expect(fs.readFile).toHaveBeenCalledWith( + path.join(MEMFS_VOLUME, 'report-after.json'), + ); + }); + + it('should read report files from custom locations', async () => { + vol.fromJSON( + { + '.code-pushup/.ci/.prev/report.json': JSON.stringify(REPORT_MOCK), + '.code-pushup/.ci/.curr/report.json': JSON.stringify(REPORT_MOCK), + }, + MEMFS_VOLUME, + ); + await compareReportFiles( + { persist: persistConfig }, { - before: path.join(MEMFS_VOLUME, 'source-report.json'), - after: path.join(MEMFS_VOLUME, 'target-report.json'), + before: path.join(MEMFS_VOLUME, '.code-pushup/.ci/.prev/report.json'), + after: path.join(MEMFS_VOLUME, '.code-pushup/.ci/.curr/report.json'), }, - { outputDir: MEMFS_VOLUME, filename: 'report', format: ['json'] }, - undefined, ); + expect(fs.readFile).toHaveBeenCalledWith( + path.join(MEMFS_VOLUME, '.code-pushup/.ci/.prev/report.json'), + ); + expect(fs.readFile).toHaveBeenCalledWith( + path.join(MEMFS_VOLUME, '.code-pushup/.ci/.curr/report.json'), + ); + }); + + it('should create valid report-diff.json from report.json files', async () => { + await compareReportFiles({ persist: persistConfig }); + const reportsDiffPromise = readJsonFile( path.join(MEMFS_VOLUME, 'report-diff.json'), ); @@ -54,14 +94,12 @@ describe('compareReportFiles', () => { }); it('should create all diff files specified by persist.format', async () => { - await compareReportFiles( - { - before: path.join(MEMFS_VOLUME, 'source-report.json'), - after: path.join(MEMFS_VOLUME, 'target-report.json'), + await compareReportFiles({ + persist: { + ...persistConfig, + format: ['json', 'md'], }, - { outputDir: MEMFS_VOLUME, filename: 'report', format: ['json', 'md'] }, - undefined, - ); + }); await expect( fileExists(path.join(MEMFS_VOLUME, 'report-diff.json')), @@ -72,22 +110,18 @@ describe('compareReportFiles', () => { }); it('should include portal link (fetched using upload config) in Markdown file', async () => { - await compareReportFiles( - { - before: path.join(MEMFS_VOLUME, 'source-report.json'), - after: path.join(MEMFS_VOLUME, 'target-report.json'), - }, - { outputDir: MEMFS_VOLUME, filename: 'report', format: ['json', 'md'] }, - { + await compareReportFiles({ + persist: persistConfig, + upload: { server: 'https://api.code-pushup.dev/graphql', apiKey: 'cp_XXXXX', organization: 'dunder-mifflin', project: 'website', }, - ); + }); await expect( - readFile(path.join(MEMFS_VOLUME, 'report-diff.md'), 'utf8'), + fs.readFile(path.join(MEMFS_VOLUME, 'report-diff.md'), 'utf8'), ).resolves.toContain( `[🕵️ See full comparison in Code PushUp portal 🔍](https://code-pushup.example.com/portal/dunder-mifflin/website/comparison/${commitShas.before}/${commitShas.after})`, ); @@ -107,17 +141,10 @@ describe('compareReportFiles', () => { }); it('should not include portal link in Markdown if upload config is missing', async () => { - await compareReportFiles( - { - before: path.join(MEMFS_VOLUME, 'source-report.json'), - after: path.join(MEMFS_VOLUME, 'target-report.json'), - }, - { outputDir: MEMFS_VOLUME, filename: 'report', format: ['json', 'md'] }, - undefined, - ); + await compareReportFiles({ persist: persistConfig }); await expect( - readFile(path.join(MEMFS_VOLUME, 'report-diff.md'), 'utf8'), + fs.readFile(path.join(MEMFS_VOLUME, 'report-diff.md'), 'utf8'), ).resolves.not.toContain( '[🕵️ See full comparison in Code PushUp portal 🔍]', ); @@ -138,20 +165,22 @@ describe('compareReportFiles', () => { ); await compareReportFiles( { - before: path.join(MEMFS_VOLUME, 'source-report.json'), - after: path.join(MEMFS_VOLUME, 'target-report.json'), + persist: persistConfig, + upload: { + server: 'https://api.code-pushup.dev/graphql', + apiKey: 'cp_XXXXX', + organization: 'dunder-mifflin', + project: 'website', + }, }, - { outputDir: MEMFS_VOLUME, filename: 'report', format: ['json', 'md'] }, { - server: 'https://api.code-pushup.dev/graphql', - apiKey: 'cp_XXXXX', - organization: 'dunder-mifflin', - project: 'website', + before: path.join(MEMFS_VOLUME, 'source-report.json'), + after: path.join(MEMFS_VOLUME, 'target-report.json'), }, ); await expect( - readFile(path.join(MEMFS_VOLUME, 'report-diff.md'), 'utf8'), + fs.readFile(path.join(MEMFS_VOLUME, 'report-diff.md'), 'utf8'), ).resolves.not.toContain( '[🕵️ See full comparison in Code PushUp portal 🔍]', ); @@ -160,19 +189,15 @@ describe('compareReportFiles', () => { }); it('should include portal link in JSON file', async () => { - await compareReportFiles( - { - before: path.join(MEMFS_VOLUME, 'source-report.json'), - after: path.join(MEMFS_VOLUME, 'target-report.json'), - }, - { outputDir: MEMFS_VOLUME, filename: 'report', format: ['json', 'md'] }, - { + await compareReportFiles({ + persist: persistConfig, + upload: { server: 'https://api.code-pushup.dev/graphql', apiKey: 'cp_XXXXX', organization: 'dunder-mifflin', project: 'website', }, - ); + }); await expect( readJsonFile(path.join(MEMFS_VOLUME, 'report-diff.json')), @@ -183,25 +208,90 @@ describe('compareReportFiles', () => { ); }); - it('should include label in JSON file', async () => { + it('should include label option in JSON file', async () => { await compareReportFiles( + { persist: persistConfig }, + { label: 'backoffice' }, + ); + + await expect( + readJsonFile(path.join(MEMFS_VOLUME, 'report-diff.json')), + ).resolves.toEqual( + expect.objectContaining({ + label: 'backoffice', + }), + ); + }); + + it('should include label if same in both report.json files', async () => { + vol.fromJSON( { - before: path.join(MEMFS_VOLUME, 'source-report.json'), - after: path.join(MEMFS_VOLUME, 'target-report.json'), + 'report-before.json': JSON.stringify({ + ...MINIMAL_REPORT_MOCK, + label: 'api', + } satisfies Report), + 'report-after.json': JSON.stringify({ + ...REPORT_MOCK, + label: 'api', + } satisfies Report), }, - { outputDir: MEMFS_VOLUME, filename: 'report', format: ['json', 'md'] }, - undefined, - 'backoffice', + MEMFS_VOLUME, ); + await compareReportFiles({ persist: persistConfig }); + await expect( readJsonFile(path.join(MEMFS_VOLUME, 'report-diff.json')), ).resolves.toEqual( expect.objectContaining({ - label: 'backoffice', + label: 'api', }), ); }); + + it('should not include label if missing in report.json files', async () => { + vol.fromJSON( + { + 'report-before.json': JSON.stringify({ + ...MINIMAL_REPORT_MOCK, + label: undefined, + } satisfies Report), + 'report-after.json': JSON.stringify({ + ...REPORT_MOCK, + label: undefined, + } satisfies Report), + }, + MEMFS_VOLUME, + ); + + await compareReportFiles({ persist: persistConfig }); + + await expect( + readJsonFile(path.join(MEMFS_VOLUME, 'report-diff.json')), + ).resolves.not.toHaveProperty('label'); + }); + + it('should not include label if it differs in report.json files', async () => { + vol.fromJSON( + { + 'report-before.json': JSON.stringify({ + ...MINIMAL_REPORT_MOCK, + label: 'frontend', + } satisfies Report), + 'report-after.json': JSON.stringify({ + ...REPORT_MOCK, + label: 'backend', + } satisfies Report), + }, + MEMFS_VOLUME, + ); + + await compareReportFiles({ persist: persistConfig }); + + await expect( + readJsonFile(path.join(MEMFS_VOLUME, 'report-diff.json')), + ).resolves.not.toHaveProperty('label'); + }); }); describe('compareReports', () => { @@ -243,7 +333,7 @@ describe('compareReports', () => { it('should contain all categories/groups/audits in unchanged arrays', () => { const reportsDiff = compareReports(mockReports); expect(reportsDiff.categories.unchanged).toHaveLength( - mockReport.categories.length, + mockReport.categories!.length, ); expect(reportsDiff.groups.unchanged).toHaveLength( mockReport.plugins.reduce((acc, { groups }) => acc + groups!.length, 0), @@ -273,7 +363,7 @@ describe('compareReports', () => { it('should only have added categories (minimal report has none)', () => { const reportsDiff = compareReports(mockReports); expect(reportsDiff.categories.added).toHaveLength( - REPORT_MOCK.categories.length, + REPORT_MOCK.categories!.length, ); expect(reportsDiff.categories.removed).toHaveLength(0); expect(reportsDiff.categories.changed).toHaveLength(0); diff --git a/packages/core/src/lib/implementation/persist.ts b/packages/core/src/lib/implementation/persist.ts index 0d4c062c2..915af9459 100644 --- a/packages/core/src/lib/implementation/persist.ts +++ b/packages/core/src/lib/implementation/persist.ts @@ -1,9 +1,9 @@ import { mkdir, stat, writeFile } from 'node:fs/promises'; -import path from 'node:path'; -import type { PersistConfig, Report } from '@code-pushup/models'; +import type { Format, PersistConfig, Report } from '@code-pushup/models'; import { type MultipleFileResults, type ScoredReport, + createReportPath, directoryExists, generateMdReport, logMultipleFileResults, @@ -30,20 +30,22 @@ export async function persistReport( const { outputDir, filename, format } = options; // collect physical format outputs - const results = format.map(reportType => { - switch (reportType) { - case 'json': - return { - format: 'json', - content: JSON.stringify(report, null, 2), - }; - case 'md': - return { - format: 'md', - content: generateMdReport(sortedScoredReport, { outputDir }), - }; - } - }); + const results = format.map( + (reportType): { format: Format; content: string } => { + switch (reportType) { + case 'json': + return { + format: 'json', + content: JSON.stringify(report, null, 2), + }; + case 'md': + return { + format: 'md', + content: generateMdReport(sortedScoredReport, { outputDir }), + }; + } + }, + ); if (!(await directoryExists(outputDir))) { try { @@ -58,7 +60,7 @@ export async function persistReport( return Promise.allSettled( results.map(result => persistResult( - path.join(outputDir, `${filename}.${result.format}`), + createReportPath({ outputDir, filename, format: result.format }), result.content, ), ), diff --git a/packages/models/docs/models-reference.md b/packages/models/docs/models-reference.md index 599b04b25..bffd0633f 100644 --- a/packages/models/docs/models-reference.md +++ b/packages/models/docs/models-reference.md @@ -1331,6 +1331,7 @@ _Object containing the following properties:_ | **`plugins`** (\*) | | _Array of at least 1 [PluginReport](#pluginreport) items_ | | `categories` | | _Array of [CategoryConfig](#categoryconfig) items_ | | **`commit`** (\*) | Git commit for which report was collected | [Commit](#commit) (_nullable_) | +| `label` | Label (e.g. project name) | `string` | _(\*) Required._ diff --git a/packages/models/src/lib/report.ts b/packages/models/src/lib/report.ts index 6042d2481..e18945dd9 100644 --- a/packages/models/src/lib/report.ts +++ b/packages/models/src/lib/report.ts @@ -52,6 +52,7 @@ export const reportSchema = packageVersionSchema({ commit: commitSchema .describe('Git commit for which report was collected') .nullable(), + label: z.string().optional().describe('Label (e.g. project name)'), }), ) .check(createCheck(findMissingSlugsInCategoryRefs)) diff --git a/packages/models/src/lib/report.unit.test.ts b/packages/models/src/lib/report.unit.test.ts index e1ab2f916..eff7fbf83 100644 --- a/packages/models/src/lib/report.unit.test.ts +++ b/packages/models/src/lib/report.unit.test.ts @@ -207,6 +207,7 @@ describe('reportSchema', () => { ], packageName: 'cli', version: '1.0.1', + label: 'api', } satisfies Report), ).not.toThrow(); }); diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index e2d46c47c..90d8a58eb 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -27,6 +27,7 @@ export { } from './lib/execute-process.js'; export { crawlFileSystem, + createReportPath, directoryExists, ensureDirectoryExists, fileExists, diff --git a/packages/utils/src/lib/file-system.ts b/packages/utils/src/lib/file-system.ts index 647a4adfe..d97982bfe 100644 --- a/packages/utils/src/lib/file-system.ts +++ b/packages/utils/src/lib/file-system.ts @@ -2,6 +2,7 @@ import { bold, gray } from 'ansis'; import { type Options, bundleRequire } from 'bundle-require'; import { mkdir, readFile, readdir, rm, stat } from 'node:fs/promises'; import path from 'node:path'; +import type { Format, PersistConfig } from '@code-pushup/models'; import { formatBytes } from './formatting.js'; import { logMultipleResults } from './log-results.js'; import { ui } from './logging.js'; @@ -84,6 +85,21 @@ export async function importModule(options: Options): Promise { return mod as T; } +export function createReportPath({ + outputDir, + filename, + format, + suffix, +}: Omit, 'format'> & { + format: Format; + suffix?: string; +}): string { + return path.join( + outputDir, + suffix ? `${filename}-${suffix}.${format}` : `${filename}.${format}`, + ); +} + export function pluginWorkDir(slug: string): string { return path.join('node_modules', '.code-pushup', slug); } diff --git a/packages/utils/src/lib/file-system.unit.test.ts b/packages/utils/src/lib/file-system.unit.test.ts index 5aea3d8b6..b141ee4ff 100644 --- a/packages/utils/src/lib/file-system.unit.test.ts +++ b/packages/utils/src/lib/file-system.unit.test.ts @@ -6,6 +6,7 @@ import { MEMFS_VOLUME } from '@code-pushup/test-utils'; import { type FileResult, crawlFileSystem, + createReportPath, ensureDirectoryExists, filePathToCliArg, findLineNumberInText, @@ -29,6 +30,29 @@ describe('ensureDirectoryExists', () => { }); }); +describe('createReportPath', () => { + it('should create report.json path', () => { + expect( + createReportPath({ + outputDir: '.code-pushup', + filename: 'report', + format: 'json', + }), + ).toMatchPath('.code-pushup/report.json'); + }); + + it('should create report-diff.md path', () => { + expect( + createReportPath({ + outputDir: '.code-pushup', + filename: 'report', + format: 'md', + suffix: 'diff', + }), + ).toMatchPath('.code-pushup/report-diff.md'); + }); +}); + describe('logMultipleFileResults', () => { it('should call logMultipleResults with the correct arguments', () => { const logMultipleResultsSpy = vi.spyOn( diff --git a/packages/utils/vitest.unit.config.ts b/packages/utils/vitest.unit.config.ts index af057c331..64208a43e 100644 --- a/packages/utils/vitest.unit.config.ts +++ b/packages/utils/vitest.unit.config.ts @@ -31,6 +31,7 @@ export default defineConfig({ '../../testing/test-setup/src/lib/reset.mocks.ts', '../../testing/test-setup/src/lib/extend/ui-logger.matcher.ts', '../../testing/test-setup/src/lib/extend/markdown-table.matcher.ts', + '../../testing/test-setup/src/lib/extend/path.matcher.ts', ], }, });