refactor(models): add default value for outputDir (#358)

code-pushup · Dec 18, 2023 · 5871f0e · 5871f0e
1 parent 354b15f
commit 5871f0e
Show file tree

Hide file tree

Showing 36 changed files with 341 additions and 161 deletions.
diff --git a/e2e/cli-e2e/mocks/code-pushup.config.js b/e2e/cli-e2e/mocks/code-pushup.config.js
@@ -1,10 +1,8 @@
 // TODO: import plugins using NPM package names using local registry: https://github.com/flowup/quality-metrics-cli/issues/33
-import { join } from 'path';
 import eslintPlugin from '../../../dist/packages/plugin-eslint';
 import lighthousePlugin from '../../../dist/packages/plugin-lighthouse';
 
 export default {
-  persist: { outputDir: join('tmp', 'js') },
   upload: {
     organization: 'code-pushup',
     project: 'cli-js',
@@ -20,10 +18,10 @@ export default {
           plugin: 'lighthouse',
           type: 'audit',
           slug: 'largest-contentful-paint',
-          weight: 1
-        }
-      ]
-    }
+          weight: 1,
+        },
+      ],
+    },
   ],
   plugins: [
     await eslintPlugin({ eslintrc: '.eslintrc.json', patterns: '**/*.ts' }),

diff --git a/e2e/cli-e2e/mocks/code-pushup.config.mjs b/e2e/cli-e2e/mocks/code-pushup.config.mjs
@@ -1,10 +1,8 @@
-import { join } from 'path';
 // TODO: import plugins using NPM package names using local registry: https://github.com/flowup/quality-metrics-cli/issues/33
 import eslintPlugin from '../../../dist/packages/plugin-eslint';
 import lighthousePlugin from '../../../dist/packages/plugin-lighthouse';
 
 export default {
-  persist: { outputDir: join('tmp', 'mjs') },
   upload: {
     organization: 'code-pushup',
     project: 'cli-mjs',
@@ -20,10 +18,10 @@ export default {
           plugin: 'lighthouse',
           type: 'audit',
           slug: 'largest-contentful-paint',
-          weight: 1
-        }
-      ]
-    }
+          weight: 1,
+        },
+      ],
+    },
   ],
   plugins: [
     await eslintPlugin({ eslintrc: '.eslintrc.json', patterns: '**/*.ts' }),

diff --git a/e2e/cli-e2e/mocks/code-pushup.config.ts b/e2e/cli-e2e/mocks/code-pushup.config.ts
@@ -1,11 +1,9 @@
 // TODO: import plugins using NPM package names using local registry: https://github.com/flowup/quality-metrics-cli/issues/33
-import { join } from 'path';
 import eslintPlugin from '../../../dist/packages/plugin-eslint';
 import lighthousePlugin from '../../../dist/packages/plugin-lighthouse';
 import { CoreConfig } from '../../../packages/models/src';
 
 export default {
-  persist: { outputDir: join('tmp', 'ts') },
   upload: {
     organization: 'code-pushup',
     project: 'cli-ts',
@@ -21,10 +19,10 @@ export default {
           plugin: 'lighthouse',
           type: 'audit',
           slug: 'largest-contentful-paint',
-          weight: 1
-        }
-      ]
-    }
+          weight: 1,
+        },
+      ],
+    },
   ],
   plugins: [
     await eslintPlugin({ eslintrc: '.eslintrc.json', patterns: '**/*.ts' }),

diff --git a/e2e/cli-e2e/tests/print-config.e2e.test.ts b/e2e/cli-e2e/tests/print-config.e2e.test.ts
@@ -1,4 +1,9 @@
 import { expect } from 'vitest';
+import {
+  PERSIST_FILENAME,
+  PERSIST_FORMAT,
+  PERSIST_OUTPUT_DIR,
+} from '@code-pushup/models';
 import { executeProcess } from '@code-pushup/utils';
 import { configFile, extensions } from '../mocks/utils';
 
@@ -13,9 +18,6 @@ describe('print-config', () => {
           '--verbose',
           '--no-progress',
           `--config=${configFile(ext)}`,
-          '--persist.outputDir=my-dir',
-          '--persist.format=md',
-          '--persist.filename=my-report',
         ],
       });
 
@@ -24,17 +26,18 @@ describe('print-config', () => {
       expect(JSON.parse(stdout)).toEqual(
         expect.objectContaining({
           config: expect.stringContaining(`code-pushup.config.${ext}`),
+          // filled by command options
+          persist: {
+            outputDir: PERSIST_OUTPUT_DIR,
+            filename: PERSIST_FILENAME,
+            format: PERSIST_FORMAT,
+          },
           upload: {
             organization: 'code-pushup',
             project: `cli-${ext}`,
             apiKey: 'e2e-api-key',
             server: 'https://e2e.com/api',
           },
-          persist: {
-            outputDir: 'my-dir',
-            format: ['md'],
-            filename: 'my-report',
-          },
           plugins: expect.arrayContaining([
             expect.objectContaining({ slug: 'eslint', title: 'ESLint' }),
             expect.objectContaining({
@@ -50,4 +53,32 @@ describe('print-config', () => {
     },
     120000,
   );
+
+  it('should load .ts config file and overwrite it with CLI arguments', async ext => {
+    const { code, stderr, stdout } = await executeProcess({
+      command: 'code-pushup',
+      args: [
+        'print-config',
+        '--verbose',
+        '--no-progress',
+        `--config=${configFile('ts')}`,
+        '--persist.outputDir=my-dir',
+        '--persist.format=md',
+        '--persist.filename=my-report',
+      ],
+    });
+
+    expect(code).toBe(0);
+    expect(stderr).toBe('');
+    expect(JSON.parse(stdout)).toEqual(
+      expect.objectContaining({
+        config: expect.stringContaining(`code-pushup.config.ts`),
+        persist: {
+          outputDir: 'my-dir',
+          format: ['md'],
+          filename: 'my-report',
+        },
+      }),
+    );
+  }, 120000);
 });
diff --git a/examples/plugins/code-pushup.config.ts b/examples/plugins/code-pushup.config.ts
@@ -17,12 +17,8 @@ import fileSizePlugin, {
  *
  */
 
-const outputDir = '.code-pushup';
 // eslint-disable-next-line unicorn/no-unreadable-iife
 const config = (() => ({
-  persist: {
-    outputDir,
-  },
   plugins: [
     fileSizePlugin({
       directory: './dist/packages',

diff --git a/examples/plugins/src/file-size/src/file-size.plugin.ts b/examples/plugins/src/file-size/src/file-size.plugin.ts
@@ -137,7 +137,7 @@ export function fileSizeIssues(options: {
     directory,
     pattern,
     fileTransform: async (file: string) => {
-      // get file size of file
+      // get size of file
       // const filePath = join(directory, file);
       const stats = await stat(file);
 

diff --git a/packages/cli/src/lib/collect/collect-command.unit.test.ts b/packages/cli/src/lib/collect/collect-command.unit.test.ts
@@ -2,6 +2,7 @@ import { bundleRequire } from 'bundle-require';
 import { vol } from 'memfs';
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 import { collectAndPersistReports } from '@code-pushup/core';
+import { MEMFS_VOLUME } from '@code-pushup/testing-utils';
 import { DEFAULT_CLI_CONFIGURATION } from '../../../mocks/constants';
 import { yargsCli } from '../yargs-cli';
 import { yargsCollectCommandObject } from './collect-command';
@@ -20,7 +21,31 @@ describe('collect-command', () => {
       {
         'code-pushup.config.ts': '', // only needs to exist for stat inside readCodePushupConfig
       },
-      '/test',
+      MEMFS_VOLUME,
+    );
+  });
+
+  it('should call collect with default parameters', async () => {
+    // It's hard to test the defaults for `config` so we skipped it as there are other integration tests already
+    await yargsCli(['collect', '--config=/test/code-pushup.config.ts'], {
+      ...DEFAULT_CLI_CONFIGURATION,
+      commands: [yargsCollectCommandObject()],
+    }).parseAsync();
+
+    expect(bundleRequire).toHaveBeenCalledWith({
+      format: 'esm',
+      filepath: '/test/code-pushup.config.ts',
+    });
+
+    expect(collectAndPersistReports).toHaveBeenCalledWith(
+      expect.objectContaining({
+        config: '/test/code-pushup.config.ts',
+        persist: expect.objectContaining({
+          filename: 'report',
+          outputDir: '.code-pushup',
+          format: ['json'],
+        }),
+      }),
     );
   });
 

diff --git a/packages/cli/src/lib/implementation/config-middleware.ts b/packages/cli/src/lib/implementation/config-middleware.ts
@@ -1,5 +1,10 @@
 import { readCodePushupConfig } from '@code-pushup/core';
-import { CoreConfig } from '@code-pushup/models';
+import {
+  CoreConfig,
+  PERSIST_FILENAME,
+  PERSIST_FORMAT,
+  PERSIST_OUTPUT_DIR,
+} from '@code-pushup/models';
 import { GeneralCliOptions, OnlyPluginsOptions } from './model';
 import {
   filterCategoryByOnlyPluginsOption,
@@ -27,9 +32,22 @@ export async function configMiddleware<
         ...importedRc.upload,
         ...cliOptions.upload,
       },
+      // we can't use a async rc file as yargs does not support it. see: https://github.com/yargs/yargs/issues/2234
+      // therefore this can't live in option defaults as the order would be `config`->`provided options`->default
+      // so we have to manually implement the order
       persist: {
-        ...importedRc.persist,
-        ...cliOptions.persist,
+        outputDir:
+          cliOptions?.persist?.outputDir ||
+          importedRc?.persist?.outputDir ||
+          PERSIST_OUTPUT_DIR,
+        filename:
+          cliOptions?.persist?.filename ||
+          importedRc?.persist?.filename ||
+          PERSIST_FILENAME,
+        format:
+          cliOptions?.persist?.format ||
+          importedRc?.persist?.format ||
+          PERSIST_FORMAT,
       },
       plugins: filterPluginsByOnlyPluginsOption(importedRc.plugins, cliOptions),
       categories: filterCategoryByOnlyPluginsOption(

diff --git a/packages/cli/src/lib/implementation/core-config-options.integration.test.ts b/packages/cli/src/lib/implementation/core-config-options.integration.test.ts
@@ -0,0 +1,20 @@
+import { describe, expect } from 'vitest';
+import { CoreConfig } from '@code-pushup/models';
+import { objectToCliArgs } from '@code-pushup/utils';
+import { yargsCli } from '../yargs-cli';
+import { yargsCoreConfigOptionsDefinition } from './core-config-options';
+
+describe('configOptions', () => {
+  function cli(args: Record<string, unknown>): Promise<CoreConfig> {
+    return yargsCli(objectToCliArgs(args), {
+      options: yargsCoreConfigOptionsDefinition(),
+    }).parseAsync() as unknown as Promise<CoreConfig>;
+  }
+
+  it('should fill defaults', async () => {
+    const config = await cli({});
+    expect(config).toBeDefined();
+    // only _ and $0
+    expect(Object.keys(config)).toHaveLength(2);
+  });
+});
diff --git a/packages/cli/src/lib/implementation/core-config-options.ts b/packages/cli/src/lib/implementation/core-config-options.ts
@@ -1,8 +1,26 @@
 import { Options } from 'yargs';
-import { CoreConfigCliOptions } from './model';
+import {
+  CoreConfigCliOptions,
+  PersistConfigCliOptions,
+  UploadConfigCliOptions,
+} from './model';
 
-type ArgNames = keyof CoreConfigCliOptions;
-export function yargsCoreConfigOptionsDefinition(): Record<ArgNames, Options> {
+export function yargsCoreConfigOptionsDefinition(): Record<
+  keyof CoreConfigCliOptions,
+  Options
+> {
+  return {
+    // persist
+    ...yargsPersistConfigOptionsDefinition(),
+    // upload
+    ...yargsUploadConfigOptionsDefinition(),
+  };
+}
+
+export function yargsPersistConfigOptionsDefinition(): Record<
+  keyof PersistConfigCliOptions,
+  Options
+> {
   return {
     // persist
     'persist.outputDir': {
@@ -17,6 +35,14 @@ export function yargsCoreConfigOptionsDefinition(): Record<ArgNames, Options> {
       describe: 'Format of the report output. e.g. `md`, `json`',
       type: 'array',
     },
+  };
+}
+
+export function yargsUploadConfigOptionsDefinition(): Record<
+  keyof UploadConfigCliOptions,
+  Options
+> {
+  return {
     // upload
     'upload.organization': {
       describe: 'Organization slug from portal',

diff --git a/packages/cli/src/lib/implementation/global-options.integration.test.ts b/packages/cli/src/lib/implementation/global-options.integration.test.ts
@@ -0,0 +1,20 @@
+import { describe, expect } from 'vitest';
+import { objectToCliArgs } from '@code-pushup/utils';
+import { yargsCli } from '../yargs-cli';
+import { yargsGlobalOptionsDefinition } from './global-options';
+import { GeneralCliOptions } from './model';
+
+describe('globalOptions', () => {
+  function cli(args: Record<string, unknown>): Promise<GeneralCliOptions> {
+    return yargsCli(objectToCliArgs(args), {
+      options: yargsGlobalOptionsDefinition(),
+    }).parseAsync() as unknown as Promise<GeneralCliOptions>;
+  }
+
+  it('should fill defaults', async () => {
+    const config = await cli({});
+    expect(config.verbose).toBe(false);
+    expect(config.progress).toBe(true);
+    expect(config.config).toBe('code-pushup.config.js');
+  });
+});
diff --git a/packages/cli/src/lib/print-config/print-config-command.unit.test.ts b/packages/cli/src/lib/print-config/print-config-command.unit.test.ts
@@ -1,16 +1,20 @@
 import { vol } from 'memfs';
 import { beforeEach, describe, expect } from 'vitest';
+import { MEMFS_VOLUME } from '@code-pushup/testing-utils';
 import { DEFAULT_CLI_CONFIGURATION } from '../../../mocks/constants';
 import { yargsCli } from '../yargs-cli';
 import { yargsConfigCommandObject } from './print-config-command';
 
 describe('print-config-command', () => {
   beforeEach(() => {
     vol.fromJSON(
+      // the real value comes from vitest mocks configured in vitest.config.ts
+
       {
-        'code-pushup.config.ts': '', // only needs to exist for stat inside readCodePushupConfig
+        // only needs to exist for stat inside readCodePushupConfig
+        'code-pushup.config.ts': '',
       },
-      '/test',
+      MEMFS_VOLUME,
     );
   });
 

diff --git a/packages/core/src/lib/collect-and-persist.ts b/packages/core/src/lib/collect-and-persist.ts
@@ -2,6 +2,7 @@ import { CoreConfig, pluginReportSchema } from '@code-pushup/models';
 import { verboseUtils } from '@code-pushup/utils';
 import { collect } from './implementation/collect';
 import { logPersistedResults, persistReport } from './implementation/persist';
+import { normalizePersistConfig } from './normalize';
 import { GlobalOptions } from './types';
 
 export type CollectAndPersistReportsOptions = Pick<
@@ -14,9 +15,11 @@ export async function collectAndPersistReports(
   options: CollectAndPersistReportsOptions,
 ): Promise<void> {
   const { exec } = verboseUtils(options.verbose);
+
   const report = await collect(options);
 
-  const persistResults = await persistReport(report, options);
+  const persist = normalizePersistConfig(options?.persist);
+  const persistResults = await persistReport(report, persist);
   exec(() => logPersistedResults(persistResults));
 
   // validate report and throw if invalid