tests: Added test to keep canonical root at the correct location#1144
tests: Added test to keep canonical root at the correct location#1144unbelievableflavour wants to merge 1 commit intoutkarshdalal:masterfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a Robolectric unit test verifying WineUtils.createDosdevicesSymlinks(context, container) creates expected DOS device symlinks and that the Z: symlink uses the canonicalized ImageFs.rootDir; uses mocked ImageFs, stubbed FileUtils.symlink, and a temporary .wine/dosdevices layout. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/test/java/com/winlator/core/WineUtilsTest.kt (1)
50-51: Optional: Thedrivesmock may be redundant.Since
drivesIterator()returns an empty list, thedrivesproperty mock at line 50 is likely never accessed by the production code. Consider removing it for clarity, or keep it if it documents the container's expected state.♻️ Suggested simplification
every { container.rootDir } returns rootDir - every { container.drives } returns "D:/downloads;E:/storage" every { container.drivesIterator() } returns emptyList<Array<String>>()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/test/java/com/winlator/core/WineUtilsTest.kt` around lines 50 - 51, The test currently stubs both container.drives and container.drivesIterator(), but container.drivesIterator() is configured to return an empty list so container.drives is likely never used; remove the redundant every { container.drives } returns "D:/downloads;E:/storage" mock (or alternatively keep it only as documentation) and leave every { container.drivesIterator() } returns emptyList<Array<String>>() as the single behavior to simplify the test and avoid dead mocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/test/java/com/winlator/core/WineUtilsTest.kt`:
- Around line 50-51: The test currently stubs both container.drives and
container.drivesIterator(), but container.drivesIterator() is configured to
return an empty list so container.drives is likely never used; remove the
redundant every { container.drives } returns "D:/downloads;E:/storage" mock (or
alternatively keep it only as documentation) and leave every {
container.drivesIterator() } returns emptyList<Array<String>>() as the single
behavior to simplify the test and avoid dead mocks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3b68485a-7ad0-4c65-90d1-303108a94703
📒 Files selected for processing (1)
app/src/test/java/com/winlator/core/WineUtilsTest.kt
393465e to
fed79f1
Compare
fed79f1 to
b0adee8
Compare
Description
It's very important this drive dir does not shift. I've implemented a fix earlier. And adding a test now to keep it this way.
Summary by cubic
Add a Robolectric test for
WineUtils.createDosdevicesSymlinksto ensure the Z: drive uses the canonicalImageFsroot so the drive dir doesn’t shift. Also verifies the C: drive symlink points to../drive_cunder.wine/dosdevices.Written for commit b0adee8. Summary will update on new commits.
Summary by CodeRabbit