fix(test-infra): scope databaseBackup fs.unlink stub so it doesn't leak

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.
This commit is contained in:
Luca
2026-05-29 16:21:15 +02:00
parent ecb2aeacf9
commit 3f5d006625
@@ -201,14 +201,22 @@ describe('DatabaseBackupService', () => {
delete: jest.fn().mockResolvedValue(1) delete: jest.fn().mockResolvedValue(1)
}); });
// Mock fs.unlink // Stub fs.promises.unlink via jest.spyOn so the original is
fs.unlink = jest.fn().mockResolvedValue(undefined); // 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); await service.cleanupOldBackups(30);
expect(fs.unlink).toHaveBeenCalledTimes(2); expect(unlinkSpy).toHaveBeenCalledTimes(2);
expect(fs.unlink).toHaveBeenCalledWith('/backup/old1.sql.gz'); expect(unlinkSpy).toHaveBeenCalledWith('/backup/old1.sql.gz');
expect(fs.unlink).toHaveBeenCalledWith('/backup/old2.sql.gz'); expect(unlinkSpy).toHaveBeenCalledWith('/backup/old2.sql.gz');
unlinkSpy.mockRestore();
}); });
}); });