Refactor random directory generation with optional cleanup#112
Refactor random directory generation with optional cleanup#112
Conversation
WalkthroughThe changes enhance the directory creation functions by introducing a Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
|
Ref: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
internal/utils_fs.go(2 hunks)internal/stage1.go(1 hunks)internal/stage10.go(1 hunks)internal/stage11.go(1 hunks)internal/stage12.go(2 hunks)internal/stage_q1.go(1 hunks)internal/stage_q2.go(1 hunks)internal/stage_q3.go(1 hunks)internal/stage_q4.go(1 hunks)internal/stage_q5.go(1 hunks)internal/stage_r1.go(1 hunks)internal/stage_r2.go(1 hunks)internal/stage_r3.go(1 hunks)internal/stage_r4.go(1 hunks)internal/utils_build.go(1 hunks)
🔇 Additional comments (19)
internal/stage12.go (2)
15-15: Function call updated with cleanup functionality.The change from
getRandomDirectorytogetRandomDirectoryWithCleanupaligns with the PR objective to include optional cleanup functionality. This ensures that temporary directories created during tests are properly cleaned up.
25-25: Consistent use of cleanup functionality.Good use of the new function with cleanup capabilities for the second directory creation as well, maintaining consistency throughout the test implementation.
internal/stage_r4.go (1)
32-32: Updated to use directory generation with cleanup.The function call has been appropriately updated to use
getShortRandomDirectoriesWithCleanup, aligning with the PR objective to introduce optional cleanup for randomly generated directories. The error handling remains unchanged and follows the same pattern as the original code.internal/stage_r3.go (1)
32-32: Consistent implementation of cleanup functionality.The change to
getShortRandomDirectoriesWithCleanuphere follows the same pattern as in other files, providing a consistent approach to directory management across test cases. This modification ensures that test directories are cleaned up after use.internal/stage_r1.go (1)
32-32: Directory creation updated with cleanup capability.The function call has been properly updated to use
getShortRandomDirectoriesWithCleanup. This is part of a consistent pattern across the codebase to introduce optional cleanup for test directories, improving resource management in tests.internal/stage_r2.go (1)
32-32: Function name updated to reflect cleanup behavior.The function call has been updated from
getShortRandomDirectoriestogetShortRandomDirectoriesWithCleanup, which better reflects that this function now includes automatic cleanup functionality. This change is consistent with the refactoring approach across the codebase.internal/stage1.go (1)
14-15: Good explanation for disabling cleanup.The comment clearly explains why cleanup is disabled for this specific case (we don't want to clean up the HOME directory). The explicit
falseparameter makes the intention clear in the code.internal/utils_build.go (1)
30-33: Function names updated to reflect cleanup behavior.The function variables have been properly renamed to use the new functions with explicit cleanup behavior. This change is consistent with the refactoring approach in the rest of the codebase.
internal/utils_fs.go (4)
16-19: Good documentation for modified function.The updated documentation clearly explains the purpose of the new
performCleanupparameter and provides useful context about when to use cleanup.
21-29: Improved directory creation logic.The updated logic now properly checks if a directory already exists and retries with a new random path if needed. This prevents potential issues if a directory with the same random name already exists.
31-37: Well-implemented conditional cleanup.The conditional cleanup logic is cleanly implemented, using the
performCleanupparameter to decide whether to register a teardown function.
42-44: Good wrapper function implementation.This wrapper function provides a clean API for the common case where cleanup is desired, without requiring callers to specify the boolean parameter.
internal/stage_q1.go (1)
26-26: Consistent function update for directory cleanupThe function call has been updated to use
getShortRandomDirectoryWithCleanupinstead ofgetShortRandomDirectory, implementing the new cleanup functionality. This change maintains the same function signature and error handling pattern.internal/stage_q3.go (1)
26-26: Consistent function update for directory cleanupThe function call has been updated to use
getShortRandomDirectoryWithCleanupinstead ofgetShortRandomDirectory, consistent with the changes in other files. This ensures temporary directories created during tests are properly cleaned up.internal/stage11.go (1)
23-23: Consistent adoption of cleanup functionalityThe function call has been updated to use
getRandomDirectoryWithCleanupinstead ofgetRandomDirectory. This change is consistent with the refactoring approach across the codebase to add automatic cleanup for test directories.internal/stage10.go (1)
22-22: Consistent function update for directory cleanupThe function call has been updated to use
getRandomDirectoryWithCleanupinstead ofgetRandomDirectory, maintaining consistency with similar changes across the codebase. This ensures test directories are properly cleaned up after tests complete.internal/stage_q5.go (1)
26-26: Good update to use the improved directory function with cleanupThe change from
getShortRandomDirectorytogetShortRandomDirectoryWithCleanupaligns with the PR objective of refactoring random directory generation to include cleanup functionality. This helps prevent test artifacts from remaining in the filesystem after tests complete.internal/stage_q2.go (1)
28-28: Consistent implementation of directory cleanup functionalityThe function update to
getShortRandomDirectoryWithCleanupmaintains consistency with the changes across other files and implements the cleanup feature as intended in the PR objectives.internal/stage_q4.go (1)
31-31: Appropriate adoption of cleanup-enabled directory functionThis change correctly implements the new directory function with cleanup capabilities, which will help maintain a cleaner test environment by removing temporary directories when they're no longer needed.
internal/stage1.go
Outdated
|
|
||
| randomDir, err := getRandomDirectory(stageHarness) | ||
| // We don't want to cleanup the HOME directory here | ||
| randomDir, err := getRandomDirectory(stageHarness, false) |
There was a problem hiding this comment.
@ryan-gang why can't this be destroyed after the test?
There was a problem hiding this comment.
There was a case, where I found a gocache in the $HOME directory, which was in use when we tried to force cleanup, that led to a panic.
There was a problem hiding this comment.
Seems like we purged the logs for the panic.
I am reverting this change for now, will keep my eyes peeled for anything similar.
|
Functionally not much changed here, our interfaces are still the same, fixtures also didn't change. Still cleanup is present for all stages. |
2 participants
Improvements to random directory generation now allow for optional cleanup. This enhances flexibility in test setups by enabling or disabling cleanup as needed.
Summary by CodeRabbit
New Features
Refactor