chore: clean up core config tests, remove obsolete logic (#435)

- fixed unit tests for `configMiddleware` to check only the immediate
underlying logic
- `readCodePushUpConfig` function and related tests were removed as they
are no longer used (replaced by`readRcByPath`)
- polished comments

Author: Tlacenka

…tions-and-middleware.integration.test.ts …entation/core-config.integration.test.tspackages/cli/src/lib/implementation/core-config.options-and-middleware.integration.test.ts renamed to packages/cli/src/lib/implementation/core-config.integration.test.ts packages/cli/src/lib/implementation/core-config.options-and-middleware.integration.test.ts renamed to packages/cli/src/lib/implementation/core-config.integration.test.ts

@@ -1,58 +1,27 @@
 import { describe, expect, vi } from 'vitest';
-import { ConfigPathError } from '@code-pushup/core';
-import { CoreConfig } from '@code-pushup/models';
-import { CORE_CONFIG_MOCK } from '@code-pushup/testing-utils';
+import { autoloadRc, readRcByPath } from '@code-pushup/core';
 import { coreConfigMiddleware } from './core-config.middleware';
 
 vi.mock('@code-pushup/core', async () => {
+  const { CORE_CONFIG_MOCK }: typeof import('@code-pushup/testing-utils') =
+    await vi.importActual('@code-pushup/testing-utils');
   const core: object = await vi.importActual('@code-pushup/core');
   return {
     ...core,
-    readRcByPath: vi.fn().mockImplementation((filepath: string): CoreConfig => {
-      const extension = filepath.split('.');
-      if (filepath.includes('throw-error')) {
-        throw new ConfigPathError(filepath);
-      }
-      return {
-        ...CORE_CONFIG_MOCK,
-        upload: {
-          ...CORE_CONFIG_MOCK.upload,
-          project: `cli-${extension}`,
-        },
-      };
-    }),
-    autoloadRc: vi.fn().mockImplementation(
-      (): CoreConfig => ({
-        ...CORE_CONFIG_MOCK,
-        upload: {
-          ...CORE_CONFIG_MOCK.upload,
-          project: `cli-autoload`,
-        },
-      }),
-    ),
+    readRcByPath: vi.fn().mockResolvedValue(CORE_CONFIG_MOCK),
+    autoloadRc: vi.fn().mockResolvedValue(CORE_CONFIG_MOCK),
   };
 });
 
 describe('coreConfigMiddleware', () => {
-  it('should load code-pushup.config.(ts|mjs|js) by default', async () => {
-    const config = await coreConfigMiddleware({});
-    expect(config?.upload?.project).toBe('cli-autoload');
+  it('should attempt to load code-pushup.config.(ts|mjs|js) by default', async () => {
+    await coreConfigMiddleware({});
+    expect(autoloadRc).toHaveBeenCalled();
   });
 
-  it.each(['ts', 'mjs', 'js'])(
-    'should load a valid .%s config',
-    async extension => {
-      const config = await coreConfigMiddleware({
-        config: `code-pushup.config.${extension}`,
-      });
-      expect(config.config).toContain(`code-pushup.config.${extension}`);
-      expect(config.upload?.project).toContain(extension);
-    },
-  );
-
-  it('should throw with invalid config path', async () => {
-    await expect(
-      coreConfigMiddleware({ config: 'throw-error' }),
-    ).rejects.toThrow(/Provided path .* is not valid./);
+  it('should directly attempt to load passed config', async () => {
+    await coreConfigMiddleware({ config: 'cli/custom-config.mjs' });
+    expect(autoloadRc).not.toHaveBeenCalled();
+    expect(readRcByPath).toHaveBeenCalledWith('cli/custom-config.mjs');
   });
 });

packages/cli/src/lib/implementation/core-config.middleware.unit.test.ts

@@ -1,23 +1,22 @@
-export { readCodePushupConfig } from './lib/implementation/read-code-pushup-config';
 export {
-  persistReport,
-  PersistError,
-  PersistDirError,
-} from './lib/implementation/persist';
+  CollectAndPersistReportsOptions,
+  collectAndPersistReports,
+} from './lib/collect-and-persist';
+export { CollectOptions, collect } from './lib/implementation/collect';
 export {
+  PluginOutputMissingAuditError,
   executePlugin,
   executePlugins,
-  PluginOutputMissingAuditError,
 } from './lib/implementation/execute-plugin';
-export { collect, CollectOptions } from './lib/implementation/collect';
-export { upload, UploadOptions } from './lib/upload';
-export { GlobalOptions } from './lib/types';
 export {
-  collectAndPersistReports,
-  CollectAndPersistReportsOptions,
-} from './lib/collect-and-persist';
+  PersistDirError,
+  PersistError,
+  persistReport,
+} from './lib/implementation/persist';
 export {
+  ConfigPathError,
   autoloadRc,
   readRcByPath,
-  ConfigPathError,
 } from './lib/implementation/read-rc-file';
+export { GlobalOptions } from './lib/types';
+export { UploadOptions, upload } from './lib/upload';

packages/core/src/index.ts

@@ -4,7 +4,7 @@ import { CONFIG_FILE_NAME, CoreConfig } from '@code-pushup/models';
 import { MEMFS_VOLUME } from '@code-pushup/testing-utils';
 import { autoloadRc } from './read-rc-file';
 
-// Mock bundleRequire inside importEsmModule used for fetching config
+// mock bundleRequire inside importEsmModule used for fetching config
 vi.mock('bundle-require', async () => {
   const { CORE_CONFIG_MOCK }: Record<string, CoreConfig> =
     await vi.importActual('@code-pushup/testing-utils');
@@ -21,8 +21,7 @@ vi.mock('bundle-require', async () => {
               ...CORE_CONFIG_MOCK,
               upload: {
                 ...CORE_CONFIG_MOCK?.upload,
-                // this value is mocked from the above mock setup. It is always the extension of the loaded file
-                project: extension,
+                project: extension, // returns loaded file extension to check format precedence
               },
             },
           },
@@ -31,11 +30,11 @@ vi.mock('bundle-require', async () => {
   };
 });
 
+// Note: memfs files are only listed to satisfy a system check, value is used from bundle-require mock
 describe('autoloadRc', () => {
   it('prioritise a .ts configuration file', async () => {
     vol.fromJSON(
       {
-        // this is just here to satisfy the file system check. the file ise served over a mock in bundleRequire
         [`${CONFIG_FILE_NAME}.js`]: '',
         [`${CONFIG_FILE_NAME}.mjs`]: '',
         [`${CONFIG_FILE_NAME}.ts`]: '',
@@ -45,17 +44,14 @@ describe('autoloadRc', () => {
 
     await expect(autoloadRc()).resolves.toEqual(
       expect.objectContaining({
-        upload: expect.objectContaining({
-          project: 'ts',
-        }),
+        upload: expect.objectContaining({ project: 'ts' }),
       }),
     );
   });
 
   it('should prioritise .mjs configuration file over .js', async () => {
     vol.fromJSON(
       {
-        // this is just here to satisfy the file system check. the file ise served over a mock in bundleRequire
         [`${CONFIG_FILE_NAME}.js`]: '',
         [`${CONFIG_FILE_NAME}.mjs`]: '',
       },
@@ -64,27 +60,17 @@ describe('autoloadRc', () => {
 
     await expect(autoloadRc()).resolves.toEqual(
       expect.objectContaining({
-        upload: expect.objectContaining({
-          project: 'mjs',
-        }),
+        upload: expect.objectContaining({ project: 'mjs' }),
       }),
     );
   });
 
   it('should load a .js configuration file if no other valid extension exists', async () => {
-    vol.fromJSON(
-      {
-        // this is just here to satisfy the file system check. the file ise served over a mock in bundleRequire
-        [`${CONFIG_FILE_NAME}.js`]: '',
-      },
-      MEMFS_VOLUME,
-    );
+    vol.fromJSON({ [`${CONFIG_FILE_NAME}.js`]: '' }, MEMFS_VOLUME);
 
     await expect(autoloadRc()).resolves.toEqual(
       expect.objectContaining({
-        upload: expect.objectContaining({
-          project: 'js',
-        }),
+        upload: expect.objectContaining({ project: 'js' }),
       }),
     );
   });