[Tests] Add unit tests for CoreCMIHook, Parser, and Sfun branch coverage - #85
Open
DiamondDagger590 wants to merge 1 commit into
Open
[Tests] Add unit tests for CoreCMIHook, Parser, and Sfun branch coverage#85DiamondDagger590 wants to merge 1 commit into
DiamondDagger590 wants to merge 1 commit into
Conversation
Add new test files targeting uncovered branches: - CoreCMIHookTest: covers offline-player and non-Player entity branches - ParserAdditionalBranchTest: covers character filtering, exponential notation edge cases, getTree caching, implicit multiplication, chained operators, and OperatorNode toString bracket logic - SfunAdditionalBranchTest: covers sinh large values, asinh/atanh/tanh negative ranges, cot edge cases, erf negative inputs, gamma boundary cases, logBeta, and factorial boundaries Also adds CMI-API as a testImplementation dependency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hm7psNzaFDzwjGMUpKG1k2
|
Claude finished @DiamondDagger590's task in 1m 33s —— View job Review in progress
|
📝 WalkthroughWalkthroughThe PR adds a test-only CMI-API dependency and new JUnit coverage for CMI hooks, parser branches, expression nodes, and numerical functions. ChangesCoverage test expansion
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (2)
src/test/java/com/diamonddagger590/mccore/external/cmi/CoreCMIHookTest.java-26-41 (1)
26-41: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDeclare the nullability contracts on these helpers.
createHook,createMockCorePlayer, andgetAsBukkitPlayeralways return non-null values. Add@NotNullto their return types.createMockCorePlayeracceptsnullat Line 52. Add@NullabletobukkitPlayer.Proposed fix
+import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + - private CoreCMIHook createHook() { + private `@NotNull` CoreCMIHook createHook() { return new CoreCMIHook(mockPlugin); } - private CorePlayer createMockCorePlayer(Player bukkitPlayer) { + private `@NotNull` CorePlayer createMockCorePlayer(`@Nullable` Player bukkitPlayer) { UUID uuid = UUID.randomUUID(); return new CorePlayer(uuid, mockPlugin) { `@Override` - public Optional<Player> getAsBukkitPlayer() { + public `@NotNull` Optional<Player> getAsBukkitPlayer() {As per coding guidelines, "Use
@NotNullannotation from IntelliJ v12 on all non-null return types and parameters."🤖 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 `@src/test/java/com/diamonddagger590/mccore/external/cmi/CoreCMIHookTest.java` around lines 26 - 41, Add IntelliJ v12 nullability annotations to the CoreCMIHookTest helpers: mark createHook and createMockCorePlayer return types as `@NotNull`, mark the bukkitPlayer parameter of createMockCorePlayer as `@Nullable`, and mark the overridden getAsBukkitPlayer return type as `@NotNull`.Source: Coding guidelines
src/test/java/com/diamonddagger590/mccore/parser/SfunAdditionalBranchTest.java-70-75 (1)
70-75: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse independent numerical expected values.
Line 73 compares the same
Sfun.asinh(-0.5)call with itself. Line 166 only verifies thatSfun.gamma(-1.5)is finite. Both tests pass for incorrect finite results.Assert known values or values calculated independently with
Math.Proposed fix
void asinh_returnsConsistentResult_whenXIsNegativeInSeriesRange() { double x = -0.5; - double result = Sfun.asinh(x); - assertEquals(Sfun.asinh(-0.5), result, DELTA); - assertTrue(Double.isFinite(result)); + double expected = -Math.log(-x + Math.sqrt(x * x + 1.0)); + assertEquals(expected, Sfun.asinh(x), DELTA); } void gamma_returnsCorrectValue_whenXIsNegativeNonIntBetweenNeg2AndNeg1() { - double result = Sfun.gamma(-1.5); - assertTrue(Double.isFinite(result)); + assertEquals(2.3632718012073546, Sfun.gamma(-1.5), RELAXED_DELTA); }Also applies to: 163-167
🤖 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 `@src/test/java/com/diamonddagger590/mccore/parser/SfunAdditionalBranchTest.java` around lines 70 - 75, Update asinh_returnsConsistentResult_whenXIsNegativeInSeriesRange and the gamma test around Sfun.gamma(-1.5) to compare results against independently calculated expected values, using known constants or Java Math-based calculations rather than calling the same Sfun methods or only checking finiteness. Preserve the existing tolerance and input cases.
🤖 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.
Other comments:
In `@src/test/java/com/diamonddagger590/mccore/external/cmi/CoreCMIHookTest.java`:
- Around line 26-41: Add IntelliJ v12 nullability annotations to the
CoreCMIHookTest helpers: mark createHook and createMockCorePlayer return types
as `@NotNull`, mark the bukkitPlayer parameter of createMockCorePlayer as
`@Nullable`, and mark the overridden getAsBukkitPlayer return type as `@NotNull`.
In
`@src/test/java/com/diamonddagger590/mccore/parser/SfunAdditionalBranchTest.java`:
- Around line 70-75: Update
asinh_returnsConsistentResult_whenXIsNegativeInSeriesRange and the gamma test
around Sfun.gamma(-1.5) to compare results against independently calculated
expected values, using known constants or Java Math-based calculations rather
than calling the same Sfun methods or only checking finiteness. Preserve the
existing tolerance and input cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Pro Plus
Run ID: ff98eee3-fd35-4e4d-b8af-67d04089a6a4
📒 Files selected for processing (4)
build.gradle.ktssrc/test/java/com/diamonddagger590/mccore/external/cmi/CoreCMIHookTest.javasrc/test/java/com/diamonddagger590/mccore/parser/ParserAdditionalBranchTest.javasrc/test/java/com/diamonddagger590/mccore/parser/SfunAdditionalBranchTest.java
4 tasks
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
2 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.
Summary
CoreCMIHookTestcovering the offline-player and non-Player entity early-return branches (CMI's JavaPlugin base + Vault transitive dependency prevent mocking the CMI instance itself)ParserAdditionalBranchTestcovering character filtering, exponential notation edge cases, getTree caching, implicit multiplication, chained operators (modulo/division/subtraction), built-in constants (pi/e), and OperatorNode toString bracket logicSfunAdditionalBranchTestcovering sinh large values, asinh/atanh/tanh negative input ranges, cot edge cases, erf negative inputs, gamma boundary cases, logBeta, and factorial boundariestestImplementationdependency inbuild.gradle.ktsCoverage impact
Test plan
Generated by Claude Code
Summary by CodeRabbit