Add unit test for Mandrel.getPkgFromJson#148
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 due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a new JUnit 5 test class Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/test/java/io/foojay/api/distribution/MandrelTest.java (1)
150-153: Consider usingassertNotEqualsfor better failure diagnostics.Using
assertNotEqualsis more idiomatic for JUnit and provides better failure messages by showing the actual value.Proposed fix
- assertTrue(pkg.getArchitecture() != Architecture.NONE, - "Architecture should be resolved for " + pkg.getFilename()); - assertTrue(pkg.getOperatingSystem() != OperatingSystem.NONE, - "OS should be resolved for " + pkg.getFilename()); + assertNotEquals(Architecture.NONE, pkg.getArchitecture(), + "Architecture should be resolved for " + pkg.getFilename()); + assertNotEquals(OperatingSystem.NONE, pkg.getOperatingSystem(), + "OS should be resolved for " + pkg.getFilename());This would require adding:
import static org.junit.jupiter.api.Assertions.assertNotEquals;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/io/foojay/api/distribution/MandrelTest.java` around lines 150 - 153, Replace the two negative assertions in MandrelTest (assertTrue(pkg.getArchitecture() != Architecture.NONE, ...) and assertTrue(pkg.getOperatingSystem() != OperatingSystem.NONE, ...)) with positive JUnit assertions using assertNotEquals to improve failure messages; import static org.junit.jupiter.api.Assertions.assertNotEquals and call assertNotEquals(Architecture.NONE, pkg.getArchitecture(), "Architecture should be resolved for " + pkg.getFilename()) and assertNotEquals(OperatingSystem.NONE, pkg.getOperatingSystem(), "OS should be resolved for " + pkg.getFilename()) to preserve messages while exposing actual values on failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/test/java/io/foojay/api/distribution/MandrelTest.java`:
- Around line 97-98: The test method name getPkgFromJsonParsesLatest10Releases
in MandrelTest is misleading because it asserts 5 releases; rename the method to
getPkgFromJsonParsesLatest5Releases (or alternatively update the test data to
include 10 releases) and update any test references or documentation accordingly
so the method name matches the asserted behavior.
---
Nitpick comments:
In `@src/test/java/io/foojay/api/distribution/MandrelTest.java`:
- Around line 150-153: Replace the two negative assertions in MandrelTest
(assertTrue(pkg.getArchitecture() != Architecture.NONE, ...) and
assertTrue(pkg.getOperatingSystem() != OperatingSystem.NONE, ...)) with positive
JUnit assertions using assertNotEquals to improve failure messages; import
static org.junit.jupiter.api.Assertions.assertNotEquals and call
assertNotEquals(Architecture.NONE, pkg.getArchitecture(), "Architecture should
be resolved for " + pkg.getFilename()) and assertNotEquals(OperatingSystem.NONE,
pkg.getOperatingSystem(), "OS should be resolved for " + pkg.getFilename()) to
preserve messages while exposing actual values on failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c9eafa19-d395-4f0e-a54d-7f059eb05b28
📒 Files selected for processing (1)
src/test/java/io/foojay/api/distribution/MandrelTest.java
| @Test | ||
| public void getPkgFromJsonParsesLatest10Releases() { |
There was a problem hiding this comment.
Misleading test method name: says "10Releases" but tests 5.
The method tests 5 releases (JDK 17, 21, 23, 24, 25), not 10. This mismatch can confuse future maintainers.
Proposed fix
`@Test`
- public void getPkgFromJsonParsesLatest10Releases() {
+ public void getPkgFromJsonParsesReleases() {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Test | |
| public void getPkgFromJsonParsesLatest10Releases() { | |
| `@Test` | |
| public void getPkgFromJsonParsesReleases() { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/test/java/io/foojay/api/distribution/MandrelTest.java` around lines 97 -
98, The test method name getPkgFromJsonParsesLatest10Releases in MandrelTest is
misleading because it asserts 5 releases; rename the method to
getPkgFromJsonParsesLatest5Releases (or alternatively update the test data to
include 10 releases) and update any test references or documentation accordingly
so the method name matches the asserted behavior.
Test parsing of GitHub release JSON using 5 representative Mandrel releases (JDK 17/21/23/24/25). Verifies asset filtering, platform detection, archive types, JDK version extraction, prerelease skipping, and non-matching filename handling. Requires FOOJAY_API_ENVIRONMENT=test to avoid MQTT initialization.
zakkak
force-pushed
the
2026-03-26-mandrel-unittests
branch
from
8bdd409 to
c350f13
Compare
1 participant
Test parsing of GitHub release JSON using 5 representative Mandrel
releases (JDK 17/21/23/24/25). Verifies asset filtering, platform
detection, archive types, JDK version extraction, prerelease skipping,
and non-matching filename handling.
Requires FOOJAY_API_ENVIRONMENT=test to avoid MQTT initialization.
Summary by CodeRabbit