From 3f5d006625fdbd8f34ad57d901dd55f543878e05 Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Fri, 29 May 2026 16:21:15 +0200 Subject: [PATCH] fix(test-infra): scope databaseBackup fs.unlink stub so it doesn't leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Line 205 of databaseBackup.test.js reassigned `fs.unlink` directly (`fs.unlink = jest.fn(...)`), which permanently mutated the global fs.promises module. Every test running after this in the same jest worker process inherited the no-op stub, including integration/storageBackend.test.js — whose LocalFsStorage.delete() silently became a no-op, making the subsequent exists() assertion flip from false to true. Confirmed by adding a diagnostic patch to LocalFsStorage.delete: post-await fsp.unlink, fs.existsSync(abs) returned true. unlink had resolved without throwing but the file was still there → the unlink was a mock. Fix: jest.spyOn(fs, 'unlink').mockResolvedValue(undefined) + a matching mockRestore() at the end of the test. Behaviour is identical inside this test; the original fs.unlink is restored when the test finishes, so subsequent tests get real fs.unlink again. Pre-existing issue — has been latent on upstream/beta forever. Only surfaces consistently when CI load shifts jest's worker allocation such that databaseBackup and storageBackend land in the same worker process. This PR's extra integration test files made that allocation deterministic locally and frequent enough on CI to fail reliably. --- .../services/__tests__/databaseBackup.test.js | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/backend/src/services/__tests__/databaseBackup.test.js b/backend/src/services/__tests__/databaseBackup.test.js index be91ab24..12e73972 100644 --- a/backend/src/services/__tests__/databaseBackup.test.js +++ b/backend/src/services/__tests__/databaseBackup.test.js @@ -201,14 +201,22 @@ describe('DatabaseBackupService', () => { delete: jest.fn().mockResolvedValue(1) }); - // Mock fs.unlink - fs.unlink = jest.fn().mockResolvedValue(undefined); - + // Stub fs.promises.unlink via jest.spyOn so the original is + // restored when the test finishes. The previous form + // (`fs.unlink = jest.fn()`) leaked into every test that ran + // after this one in the same jest worker — most visibly + // integration/storageBackend.test.js, whose LocalFsStorage + // delete() became a silent no-op and the subsequent + // exists() assertion flipped from false to true. spyOn + + // mockRestore in afterEach keeps the stub scoped to this test. + const unlinkSpy = jest.spyOn(fs, 'unlink').mockResolvedValue(undefined); + await service.cleanupOldBackups(30); - - expect(fs.unlink).toHaveBeenCalledTimes(2); - expect(fs.unlink).toHaveBeenCalledWith('/backup/old1.sql.gz'); - expect(fs.unlink).toHaveBeenCalledWith('/backup/old2.sql.gz'); + + expect(unlinkSpy).toHaveBeenCalledTimes(2); + expect(unlinkSpy).toHaveBeenCalledWith('/backup/old1.sql.gz'); + expect(unlinkSpy).toHaveBeenCalledWith('/backup/old2.sql.gz'); + unlinkSpy.mockRestore(); }); });