Skip to content

Commit aef0d6d

Browse files
committed
fix: address PR review comments for test reliability
- Wait for process termination after kill(9) to prevent orphaned processes - Use fs.mkdtemp() instead of Date.now() for unique temp directories - Clean up temp directories after tests complete - Add proper error cleanup for temp directories
1 parent c0dba6c commit aef0d6d

File tree

1 file changed

+11
-3
lines changed

1 file changed

+11
-3
lines changed

scripts/bin-test/test.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -183,12 +183,16 @@ async function runHttpDiscovery(modulePath: string): Promise<DiscoveryResult> {
183183
return { success: false, error: body };
184184
}
185185
} finally {
186-
proc.kill(9);
186+
if (proc.pid) {
187+
proc.kill(9);
188+
await new Promise<void>((resolve) => proc.on("exit", resolve));
189+
}
187190
}
188191
}
189192

190193
async function runFileDiscovery(modulePath: string): Promise<DiscoveryResult> {
191-
const outputPath = path.join(os.tmpdir(), `firebase-functions-test-${Date.now()}.json`);
194+
const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "firebase-functions-test-"));
195+
const outputPath = path.join(tempDir, "manifest.json");
192196

193197
return new Promise((resolve, reject) => {
194198
const proc = subprocess.spawn("npx", ["firebase-functions"], {
@@ -218,7 +222,7 @@ async function runFileDiscovery(modulePath: string): Promise<DiscoveryResult> {
218222
try {
219223
const manifestJson = await fs.readFile(outputPath, "utf8");
220224
const manifest = JSON.parse(manifestJson) as Record<string, unknown>;
221-
await fs.unlink(outputPath).catch(() => {
225+
await fs.rm(tempDir, { recursive: true }).catch(() => {
222226
// Ignore errors
223227
});
224228
resolve({ success: true, manifest });
@@ -234,6 +238,10 @@ async function runFileDiscovery(modulePath: string): Promise<DiscoveryResult> {
234238

235239
proc.on("error", (err) => {
236240
clearTimeout(timeoutId);
241+
// Clean up temp directory on error
242+
fs.rm(tempDir, { recursive: true }).catch(() => {
243+
// Ignore errors
244+
});
237245
reject(err);
238246
});
239247
});

0 commit comments

Comments
 (0)