Skip to content

test: enforce full GeigerCounters coverage and fix collection regressions - #21

Merged
ryanbarlow97 merged 2 commits into
mainfrom
test/full-coverage
Sep 29, 2026
Merged

ryanbarlow97 merged 2 commits into
mainfrom
test/full-coverage

Conversation

@ryanbarlow97

@ryanbarlow97 ryanbarlow97 commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

GeigerCounters had no executable coverage suite. Regression tests reproduced collection from the wrong world or a stale source, an older asynchronous move overwriting a newer one, lost legacy message customizations, nonfinite move coordinates, and inaccurate wait times after lowering or disabling drop limits. Fix those cases and cover configuration/migration, commands, collection/rewards, rate limits/persistence, spawn filtering, lifecycle, particles and sounds.

Use the equivalent BukkitWorld wrapper directly for WorldGuard world lookup; the pinned older WorldEdit adapter's static enum initialization is incompatible with the current Paper API. Narrow private-guard and weighted-selection cleanups preserve established behavior.

mvn clean verify enforces 100% production line, branch and instruction coverage without exclusions; CI uploads JaCoCo reports. Java 21 clean offline verify passed: 100 tests, zero failures/errors/skips; 1,163/1,163 lines, 489/489 branches, 5,280/5,280 instructions. External server/plugin boundaries are mocked; no deployment was performed.

Collection now consumes exactly one active counter in either hand, preserves the remaining stack, and delivers one dead counter. Rewards and dead-counter inventory leftovers drop at the player instead of being discarded; full and partially full inventories have regression coverage.

Summary by CodeRabbit

  • Bug Fixes
    • Invalid or non-finite coordinates are rejected when moving a source.
    • Source collection checks use the player’s current world and distance, helping prevent collections based on outdated location information.
    • Replacing a Geiger counter now consumes one item from the selected hand; overflow items and rewards are dropped nearby instead of being lost.
    • Drop-limit wait times now account for updated limits and report no wait when drops are available.
    • Legacy messages are placed in the player message section when creating a new message file.
  • Documentation
    • Added instructions for running tests and viewing coverage reports.

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: b084c917-ced1-40b3-8291-1863525f706f

📥 Commits

Reviewing files that changed from the base of the PR and between c203625 and 0daa648.

📒 Files selected for processing (20)
  • .github/workflows/build.yml
  • README.md
  • pom.xml
  • src/main/java/net/tfminecraft/geigercounters/commands/GeigerCommand.java
  • src/main/java/net/tfminecraft/geigercounters/config/ConfigMigrator.java
  • src/main/java/net/tfminecraft/geigercounters/config/GeigerConfiguration.java
  • src/main/java/net/tfminecraft/geigercounters/config/Messages.java
  • src/main/java/net/tfminecraft/geigercounters/handlers/SourceHandler.java
  • src/main/java/net/tfminecraft/geigercounters/hooks/WorldGuardHook.java
  • src/main/java/net/tfminecraft/geigercounters/managers/DropLimitManager.java
  • src/main/java/net/tfminecraft/geigercounters/managers/GeigerManager.java
  • src/test/java/net/tfminecraft/geigercounters/LifecycleUtilitiesTest.java
  • src/test/java/net/tfminecraft/geigercounters/commands/GeigerCommandTest.java
  • src/test/java/net/tfminecraft/geigercounters/config/ConfigMigratorTest.java
  • src/test/java/net/tfminecraft/geigercounters/config/GeigerConfigurationTest.java
  • src/test/java/net/tfminecraft/geigercounters/handlers/GeigerEffectsTest.java
  • src/test/java/net/tfminecraft/geigercounters/handlers/SourceHandlerTest.java
  • src/test/java/net/tfminecraft/geigercounters/managers/DropLimitManagerTest.java
  • src/test/java/net/tfminecraft/geigercounters/managers/GeigerManagerTest.java
  • src/test/java/net/tfminecraft/geigercounters/validators/SpawnLocationFilterTest.java

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The pull request adds automated tests and JaCoCo coverage enforcement. It also changes command coordinate validation, configuration migration, source movement and collection, drop-limit timing, and WorldGuard region lookup.

Changes

Runtime behavior and verification

Layer / File(s) Summary
Test and coverage execution
pom.xml, .github/workflows/build.yml, README.md
Maven configures JUnit, Mockito, MockBukkit, Surefire, and JaCoCo. Verification requires 100% instruction, line, and branch coverage. CI uploads an existing coverage report. The README documents test execution and coverage requirements.
Command input validation
src/main/java/net/tfminecraft/geigercounters/commands/GeigerCommand.java, src/test/java/net/tfminecraft/geigercounters/commands/GeigerCommandTest.java
The move command rejects non-finite coordinates. Tests cover command dispatch, movement, limits, drop lists, and tab completion.
Configuration migration and lifecycle
src/main/java/net/tfminecraft/geigercounters/config/*, src/test/java/net/tfminecraft/geigercounters/config/*, src/test/java/net/tfminecraft/geigercounters/LifecycleUtilitiesTest.java
Message migration handles newly created files separately, and resource loading checks for missing streams before opening them. Tests cover configuration, messages, plugin lifecycle, and utility behavior.
Source movement and collection
src/main/java/net/tfminecraft/geigercounters/handlers/SourceHandler.java, src/main/java/net/tfminecraft/geigercounters/managers/GeigerManager.java, src/test/java/net/tfminecraft/geigercounters/handlers/SourceHandlerTest.java, src/test/java/net/tfminecraft/geigercounters/managers/GeigerManagerTest.java
Moves ignore stale callbacks. Collection checks the player’s current world and distance. Counter replacement consumes one item from a stack, and inventory leftovers are dropped at the player’s location. Player checks retrieve the source for each player and skip other worlds. Tests cover these behaviors.
Drop-limit wait calculation
src/main/java/net/tfminecraft/geigercounters/managers/DropLimitManager.java, src/test/java/net/tfminecraft/geigercounters/managers/DropLimitManagerTest.java
Wait calculations account for disabled limits and histories above the configured limit. Tests cover persisted history, resets, and changed limits.
Detection effects coverage
src/test/java/net/tfminecraft/geigercounters/handlers/GeigerEffectsTest.java
Tests cover particle rendering and sound playback, expiry, suppression, and pitch limits.
Spawn and WorldGuard validation
src/main/java/net/tfminecraft/geigercounters/hooks/WorldGuardHook.java, src/test/java/net/tfminecraft/geigercounters/validators/SpawnLocationFilterTest.java
Region lookup uses BukkitWorld. Tests cover spawn filtering, optional WorldGuard handling, region matching, validator behavior, and reward accessors.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 0daa6

The collection test exercises its intended retry, and a failed source move has a fallback. No identified issue needs to be fixed before merging.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 0daa6

A manual move now hides the current source while a new location loads. If that operation never finishes, collection can remain unavailable to everyone on the server until an administrator intervenes. Normal command access is restricted, and the reviewed changes also strengthen stale-move and collection checks.

Retained concerns

  • Medium · reliability · inferred: A manual move now removes the live source before its asynchronous chunk request completes, without a demonstrated timeout or rollback. If that request never completes, collection remains unavailable and random relocation shares the unresolved move; another explicit move can supersede it.
Security review details

Security Blast Radius

  • inferred — A stranded source affects collection for players using this plugin on the server, rather than another service or data store. The initiating command is normally restricted to operators or holders of geiger.admin.

Trust Boundaries and Controls

  • observed — The registered command supplies a permission boundary, finite-coordinate validation precedes manual placement, and collection revalidates current source position and world. No unprivileged command-to-placement path was established.

Resilience and Maintainability Implications

  • observed — On a completed chunk-load error, the handler logs and attempts surface placement rather than propagating failure. That fallback existed before this PR; it does not address a future that never completes.

Hardening Proposals

  • proposed — Define a bounded terminal state for manual moves, including timeout or cancellation and a deliberate restore-or-retry policy for the previous source.
  • proposed — If rewards are intended to remain exclusive to the collector, specify how inventory overflow preserves that ownership before relying on ordinary world drops.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 156 functions across 17 files. (3 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main changes: adding full test coverage enforcement and fixing collection-related regressions.
Full details: Docstring Coverage

Explanation

Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 156 functions across 17 files. (3 skipped: 3 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

A rabbit checks the source’s new place
And tests each path at a careful pace
When counters stack, one stays behind
And coverage marks each branch it finds
The report hops safely into CI
Then off through clover fields goes I

Comment @coderabbitai help to get the list of available commands.

@ryanbarlow97
ryanbarlow97 marked this pull request as ready for review September 29, 2026 18:57
@ryanbarlow97
ryanbarlow97 merged commit e108289 into main Sep 29, 2026
2 checks passed
@ryanbarlow97
ryanbarlow97 deleted the test/full-coverage branch September 29, 2026 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant