fix: dxwrappers and graphic drivers downloader handling on custom content files#1577
Merged
utkarshdalal merged 1 commit intoJun 12, 2026
Conversation
Contributor
📝 WalkthroughWalkthroughBoth downloader utilities now gracefully handle missing manifest components by returning ChangesDownloader error handling and filename normalization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/test/java/app/gamenative/utils/downloader/DXWrapperDownloaderTest.kt (1)
237-247: 💤 Low valueConsider using
assertNull()for clarity.While
assertEquals(null, result)is functionally correct,assertNull(result)would be more idiomatic and slightly clearer.✨ Proposed refactor
- assertEquals( - "Invalid component should return null", - null, - result - ) + assertNull("Invalid component should return null", result)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/test/java/app/gamenative/utils/downloader/DXWrapperDownloaderTest.kt` around lines 237 - 247, Replace the assertion in testInvalidComponentReturnsNull to use assertNull for clarity: locate the test method testInvalidComponentReturnsNull that calls DXWrapperDownloader.ensureDXWrapperAvailable and change the assertEquals null check to use assertNull(result) (or assertNull("Invalid component should return null", result) if you want to keep the message) so the intent is more idiomatic and readable.app/src/test/java/app/gamenative/utils/downloader/GraphicsDriverDownloaderTest.kt (1)
254-262: 💤 Low valueConsider using
assertNotNullinstead of!!for better test clarity.Line 258 uses the non-null assertion operator
!!on the manifest lookup. If"vortek-2.1"is ever removed from the manifest, this will throw an NPE instead of a clear assertion failure.♻️ Proposed improvement for test robustness
// Load manifest to get component name val manifestJson = context.assets.open(GraphicsDriverDownloader.GRAPHICS_DRIVER_MANIFEST_FILE).bufferedReader().use { it.readText() } val manifest = Json { ignoreUnknownKeys = true } .decodeFromString<GraphicsDriverDownloader.GraphicsDriverManifest>(manifestJson) -val component = manifest.components.find { it.id == componentId }!! +val component = manifest.components.find { it.id == componentId } +assertNotNull("Component $componentId should exist in manifest", component) // Create a mock cached file cacheDir.mkdirs() -val cachedFile = File(cacheDir, component.name) +val cachedFile = File(cacheDir, component!!.name)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/test/java/app/gamenative/utils/downloader/GraphicsDriverDownloaderTest.kt` around lines 254 - 262, The test uses a non-null assertion (!!) when finding the component from the manifest which will throw an NPE instead of a clear test failure; change the lookup to use an assertion like assertNotNull(manifest.components.find { it.id == componentId }) (or assign to a nullable val and call fail with a descriptive message if null) and then use that asserted value for the cached file creation so the test reports a clear assertion failure instead of an NPE; update the code around GraphicsDriverDownloader.GRAPHICS_DRIVER_MANIFEST_FILE / GraphicsDriverDownloader.GraphicsDriverManifest and the componentId lookup accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@app/src/test/java/app/gamenative/utils/downloader/DXWrapperDownloaderTest.kt`:
- Around line 237-247: Replace the assertion in testInvalidComponentReturnsNull
to use assertNull for clarity: locate the test method
testInvalidComponentReturnsNull that calls
DXWrapperDownloader.ensureDXWrapperAvailable and change the assertEquals null
check to use assertNull(result) (or assertNull("Invalid component should return
null", result) if you want to keep the message) so the intent is more idiomatic
and readable.
In
`@app/src/test/java/app/gamenative/utils/downloader/GraphicsDriverDownloaderTest.kt`:
- Around line 254-262: The test uses a non-null assertion (!!) when finding the
component from the manifest which will throw an NPE instead of a clear test
failure; change the lookup to use an assertion like
assertNotNull(manifest.components.find { it.id == componentId }) (or assign to a
nullable val and call fail with a descriptive message if null) and then use that
asserted value for the cached file creation so the test reports a clear
assertion failure instead of an NPE; update the code around
GraphicsDriverDownloader.GRAPHICS_DRIVER_MANIFEST_FILE /
GraphicsDriverDownloader.GraphicsDriverManifest and the componentId lookup
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 62c9e8bd-db08-42d9-912a-f233c1e56129
📒 Files selected for processing (4)
app/src/main/java/app/gamenative/utils/downloader/DXWrapperDownloader.ktapp/src/main/java/app/gamenative/utils/downloader/GraphicsDriverDownloader.ktapp/src/test/java/app/gamenative/utils/downloader/DXWrapperDownloaderTest.ktapp/src/test/java/app/gamenative/utils/downloader/GraphicsDriverDownloaderTest.kt
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
fix: dxwrappers and graphic drivers downloader handling on custom content files
Recording
Type of Change
Checklist
#code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.CONTRIBUTING.md.Summary by cubic
Fixes downloader handling for DX wrappers and graphics drivers when using custom-named content files. Uses manifest-provided file names and returns null for unknown components to avoid crashes and improve cache reuse.
Bug Fixes
DXWrapperDownloader: usecomponent.namefrom the manifest for cache paths and remote fetch (dxwrapper/${component.name}).DXWrapperDownloaderandGraphicsDriverDownloader: return null when a component is missing from the manifest (no exception).Migration
ensureDXWrapperAvailableandensureGraphicsDriverAvailable.Written for commit 5293ced. Summary will update on new commits.
Summary by CodeRabbit