test: clean up temp files #2199
Conversation
📝 WalkthroughWalkthroughA new mechanism for tracking and cleaning up temporary directories created during test project setup is implemented. The Changes
Estimated code review effort2 (30–90 minutes) 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/testtools/src/schema.ts (1)
33-33: Consider thread safety for concurrent test execution.The global
tempDirsarray could cause issues if tests run concurrently, as multiple test processes might interfere with each other's temporary directory tracking.Consider using a more robust solution like:
-const tempDirs: string[] = []; +const tempDirs = new Set<string>();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/testtools/src/schema.ts(3 hunks)tests/integration/test-setup.ts(2 hunks)tests/regression/test-setup.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
tests/integration/test-setup.ts (1)
packages/testtools/src/schema.ts (1)
cleanUpTemps(429-441)
tests/regression/test-setup.ts (1)
packages/testtools/src/schema.ts (1)
cleanUpTemps(429-441)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
- GitHub Check: OSSAR-Scan
- GitHub Check: build-test (20.x)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: build-test (20.x)
🔇 Additional comments (6)
packages/testtools/src/schema.ts (2)
258-258: Good tracking of temporary directories.The implementation correctly tracks temporary directories when they're created, ensuring they can be cleaned up later.
429-441: Verify error handling strategy for cleanup failures.The cleanup function silently ignores all errors during directory removal. While this prevents test failures from cleanup issues, it might hide legitimate problems like permission errors or corrupted filesystems.
Consider logging errors at debug level instead of completely ignoring them:
try { fs.rmSync(d, { recursive: true, force: true }); - } catch { - // ignore + } catch (error) { + console.debug('Failed to clean up temp dir:', d, error); }This would help with debugging while not failing tests due to cleanup issues.
tests/regression/test-setup.ts (2)
1-1: Correct import of cleanup function.The import statement properly brings in the
cleanUpTempsfunction from the testtools package.
20-22: Proper cleanup integration with Jest lifecycle.The
afterAllhook is correctly placed to ensure temporary directories are cleaned up after all regression tests complete.tests/integration/test-setup.ts (2)
1-1: Consistent import across test suites.The import statement matches the pattern used in regression tests, maintaining consistency.
20-22: Consistent cleanup implementation.The
afterAllhook implementation matches the regression test setup, ensuring uniform cleanup behavior across test suites.
No description provided.