Merge pull request finos#1378 from fabiovincenzi/fix/test-shuffle-independence

fix: make tests independent of execution order

Author: jescalada

package.json

@@ -53,6 +53,7 @@
     "build-ui": "vite build",
     "check-types": "tsc",
     "check-types:server": "tsc --project tsconfig.publish.json --noEmit",
+    "test-shuffle": "NODE_ENV=test vitest --run --dir ./test --sequence.shuffle",
     "test": "cross-env NODE_ENV=test vitest --run --dir ./test",
     "test:e2e": "vitest run --config vitest.config.e2e.ts",
     "test:e2e:watch": "vitest --config vitest.config.e2e.ts",

src/proxy/index.ts

@@ -71,18 +71,26 @@ export class Proxy {
   public async start() {
     await this.proxyPreparations();
     this.expressApp = await this.createApp();
-    this.httpServer = http
-      .createServer(getServerOptions() as any, this.expressApp)
-      .listen(proxyHttpPort, () => {
+    await new Promise<void>((resolve, reject) => {
+      const server = http.createServer(getServerOptions() as any, this.expressApp!);
+      server.on('error', reject);
+      server.listen(proxyHttpPort, () => {
         console.log(`HTTP Proxy Listening on ${proxyHttpPort}`);
+        resolve();
       });
+      this.httpServer = server;
+    });
     // Start HTTPS server only if TLS is enabled
     if (getTLSEnabled()) {
-      this.httpsServer = https
-        .createServer(getServerOptions(), this.expressApp)
-        .listen(proxyHttpsPort, () => {
+      await new Promise<void>((resolve, reject) => {
+        const server = https.createServer(getServerOptions(), this.expressApp!);
+        server.on('error', reject);
+        server.listen(proxyHttpsPort, () => {
           console.log(`HTTPS Proxy Listening on ${proxyHttpsPort}`);
+          resolve();
         });
+        this.httpsServer = server;
+      });
     }
   }
 

src/proxy/processors/push-action/clearBareClone.ts

@@ -1,16 +1,12 @@
 import { Action, Step } from '../../actions';
-import fs from 'node:fs';
+import fs from 'node:fs/promises';
 
 const exec = async (req: any, action: Action): Promise<Action> => {
   const step = new Step('clearBareClone');
 
   // Recursively remove the contents of ./.remote and ignore exceptions
-  fs.rm('./.remote', { recursive: true, force: true }, (err) => {
-    if (err) {
-      throw err;
-    }
-    console.log(`.remote is deleted!`);
-  });
+  await fs.rm('./.remote', { recursive: true, force: true });
+  console.log(`.remote is deleted!`);
 
   action.addStep(step);
   return action;

src/service/index.ts

@@ -194,6 +194,7 @@ async function stop(): Promise<void> {
         reject(err);
       } else {
         console.log('Service stopped');
+        _httpServer = null;
         resolve();
       }
     });

src/service/routes/repo.ts

@@ -178,15 +178,15 @@ const repo = (proxy: any) => {
           const repoDetails = await db.createRepo(req.body);
           const proxyURL = getProxyURL(req);
 
-          // return data on the new repoistory (including it's _id and the proxyUrl)
-          res.send({ ...repoDetails, proxyURL, message: 'created' });
-
           // restart the proxy if we're proxying a new domain
           if (newOrigin) {
             console.log('Restarting the proxy to handle an additional host');
             await theProxy.stop();
             await theProxy.start();
           }
+
+          // return data on the new repository (including it's _id and the proxyUrl)
+          res.send({ ...repoDetails, proxyURL, message: 'created' });
         } catch (e: any) {
           console.error('Repository creation failed due to error: ', e.message ? e.message : e);
           console.error(e.stack);

test/1.test.ts

@@ -69,6 +69,8 @@ describe('init', () => {
 
   // Example test: use vi.doMock to override the config module
   it('should return an array of enabled auth methods when overridden', async () => {
+    // Clear module cache so vi.doMock takes effect on the fresh import below
+    vi.resetModules();
     // fs must be mocked BEFORE importing the config module
     // We also mock existsSync to ensure the file "exists"
     vi.doMock('fs', async (importOriginal) => {

test/ConfigLoader.test.ts

@@ -443,6 +443,17 @@ describe('ConfigLoader', () => {
           enabled: true,
         };
 
+        // Clean up cached clone of the fake repo so the test works regardless of order
+        const envPaths = (await import('env-paths')).default;
+        const paths = envPaths('git-proxy', { suffix: '' });
+        const repoDirName = Buffer.from(source.repository)
+          .toString('base64')
+          .replace(/[^a-zA-Z0-9]/g, '_');
+        const repoDir = path.join(paths.cache, 'git-config-cache', repoDirName);
+        if (fs.existsSync(repoDir)) {
+          fs.rmSync(repoDir, { recursive: true });
+        }
+
         await expect(configLoader.loadFromSource(source)).rejects.toThrow(
           /Failed to clone repository/,
         );

test/extractRawBody.test.ts

@@ -1,4 +1,4 @@
-import { describe, it, beforeEach, expect, vi, Mock, afterAll } from 'vitest';
+import { describe, it, beforeEach, expect, vi, Mock } from 'vitest';
 import { PassThrough } from 'stream';
 
 // Tell Vitest to mock dependencies
@@ -21,6 +21,9 @@ describe('extractRawBody middleware', () => {
   let next: Mock;
 
   beforeEach(() => {
+    (rawBody as Mock).mockClear();
+    (chain.executeChain as Mock).mockClear();
+
     req = new PassThrough();
     req.method = 'POST';
     req.url = '/proj/foo.git/git-upload-pack';
@@ -35,11 +38,6 @@ describe('extractRawBody middleware', () => {
     next = vi.fn();
   });
 
-  afterAll(() => {
-    (rawBody as Mock).mockClear();
-    (chain.executeChain as Mock).mockClear();
-  });
-
   it('skips non-pack posts', async () => {
     req.method = 'GET';
     await extractRawBody(req, res, next);

test/preReceive/preReceive.test.ts

@@ -29,8 +29,7 @@ describe.skipIf(process.platform === 'win32')('Pre-Receive Hook Execution', () =
   });
 
   afterEach(() => {
-    vi.resetModules();
-    vi.restoreAllMocks();
+    vi.clearAllMocks();
   });
 
   it('should catch and handle unexpected errors', async () => {

test/processors/getDiff.test.ts

@@ -32,6 +32,16 @@ describe('getDiff', () => {
   });
 
   it('should get diff between commits', async () => {
+    // Ensure we have a known state: write initial content, then modify it
+    await fs.writeFile(path.join(tempDir, 'test.txt'), 'initial content');
+    await git.add('.');
+    // Use --allow-empty or amend to handle case where content is already 'initial content'
+    try {
+      await git.commit('reset to initial');
+    } catch {
+      // no changes to commit - that's fine
+    }
+
     await fs.writeFile(path.join(tempDir, 'test.txt'), 'modified content');
     await git.add('.');
     await git.commit('second commit');
@@ -51,6 +61,15 @@ describe('getDiff', () => {
   });
 
   it('should get diff between commits with no changes', async () => {
+    // Create a known two-commit state: initial -> same content commit
+    await fs.writeFile(path.join(tempDir, 'test-nochange.txt'), 'initial content');
+    await git.add('.');
+    await git.commit('base commit for no-change test');
+
+    await fs.writeFile(path.join(tempDir, 'test-nochange2.txt'), 'more initial content');
+    await git.add('.');
+    await git.commit('second commit for no-change test');
+
     const action = new Action('1234567890', 'push', 'POST', 1234567890, 'test/repo.git');
     action.proxyGitPath = __dirname; // Temp dir parent path
     action.repoName = 'temp-test-repo';