Skip to content

test: enforce full VFBuilders coverage and fix construction regressions - #25

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

VFBuilders had no executable test coverage. Add regression tests for construction state and material accounting, menus/permissions, placement timers, chunk events/displays, configuration, persistence and lifecycle.

Fix the failures exposed by those tests:

  • Fresh installations failed because categories.yml and stations.yml were requested but not bundled. Include empty defaults while preserving existing files.
  • Menu icons without lore threw NullPointerException. Preserve existing lore and allow plain icons.
  • Cancelled block/furniture breaks removed stations and refunded active construction. Keep protected stations intact.
  • Moving to another world during placement threw distance-comparison exceptions. Reject out-of-world placement and skip its particle trail.
  • An older placement timeout could cancel a newer selection. Bind timer callbacks to the original placement identity.

Maven verify now enforces 100% production line, branch and instruction coverage without exclusions; CI uploads reports. Remove only redundant private checks already guaranteed by caller/API contracts. Checksum-pinned ModelEngine/NBTAPI test inputs and a provided CoreProtect API make VehicleFramework boundary mocks load correctly; these are not bundled.

Validation: Java 21 clean offline Maven verify passes 32 tests, zero failures/errors/skips; 904/904 lines, 408/408 branches, 3,829/3,829 instructions. Regression tests were observed failing before fixes. External server/plugin boundaries are mocked; no live deployment performed.

Summary by CodeRabbit

  • Bug Fixes
    • Inventory menus now handle items without existing descriptions without errors.
    • Construction placement checks now account for players changing worlds, and cancelled block-break events no longer trigger station handling.
    • Placement timers no longer affect a newer placement after the player starts another one.
  • Tests
    • Added broad automated checks for station behavior, configuration loading, inventory menus, displays, and plugin lifecycle, with coverage reports available for build runs.

@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: b9b59ab0-8910-4449-be61-b6667955deb6

📥 Commits

Reviewing files that changed from the base of the PR and between 85ac9dd and 48d94cd.

📒 Files selected for processing (22)
  • .github/dependencies.sha256
  • .github/scripts/install-local-dependencies.sh
  • .github/scripts/prepare-release.sh
  • .github/workflows/build.yml
  • README.md
  • pom.xml
  • src/main/java/net/tfminecraft/vfbuilders/VFBuilders.java
  • src/main/java/net/tfminecraft/vfbuilders/display/StationTimerDisplay.java
  • src/main/java/net/tfminecraft/vfbuilders/managers/InventoryManager.java
  • src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java
  • src/main/resources/categories.yml
  • src/main/resources/stations.yml
  • src/test/java/net/tfminecraft/vfbuilders/DatabaseFixture.java
  • src/test/java/net/tfminecraft/vfbuilders/DatabaseTest.java
  • src/test/java/net/tfminecraft/vfbuilders/DisplayTest.java
  • src/test/java/net/tfminecraft/vfbuilders/DomainTest.java
  • src/test/java/net/tfminecraft/vfbuilders/InventoryTest.java
  • src/test/java/net/tfminecraft/vfbuilders/LifecycleTest.java
  • src/test/java/net/tfminecraft/vfbuilders/LoadersTest.java
  • src/test/java/net/tfminecraft/vfbuilders/ManagerTest.java
  • src/test/java/net/tfminecraft/vfbuilders/SchedulerTest.java
  • src/test/java/net/tfminecraft/vfbuilders/StationTest.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 test dependencies, JaCoCo coverage checks, CI coverage-report uploads, and tests across plugin subsystems. It also updates station placement checks, display handling, inventory lore handling, and configuration resources.

Changes

Test coverage and station behavior

Layer / File(s) Summary
Test dependencies and coverage build
.github/dependencies.sha256, .github/scripts/*, .github/workflows/build.yml, pom.xml, README.md
Adds pinned ModelEngine and NBTAPI dependencies, Maven test and coverage configuration, CI coverage-report uploads, and test documentation.
Configuration, persistence, and domain tests
src/main/java/net/tfminecraft/vfbuilders/VFBuilders.java, src/main/resources/categories.yml, src/main/resources/stations.yml, src/test/java/net/tfminecraft/vfbuilders/Database*, src/test/java/net/tfminecraft/vfbuilders/DomainTest.java, src/test/java/net/tfminecraft/vfbuilders/LifecycleTest.java, src/test/java/net/tfminecraft/vfbuilders/LoadersTest.java
Adds YAML mappings and tests for configuration loading, database persistence, domain behavior, and plugin lifecycle. Blueprint loading no longer checks for null entries before calling isFile().
Station placement and lifecycle
src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java, src/test/java/net/tfminecraft/vfbuilders/ManagerTest.java, src/test/java/net/tfminecraft/vfbuilders/SchedulerTest.java, src/test/java/net/tfminecraft/vfbuilders/StationTest.java
Adds cross-world placement checks and captured-placement checks in scheduled tasks. Cancelled break events no longer remove stations. Tests cover placement, station behavior, manager events, and scheduling.
Display and inventory rendering
src/main/java/net/tfminecraft/vfbuilders/display/StationTimerDisplay.java, src/main/java/net/tfminecraft/vfbuilders/managers/InventoryManager.java, src/test/java/net/tfminecraft/vfbuilders/DisplayTest.java, src/test/java/net/tfminecraft/vfbuilders/InventoryTest.java
Removes several null checks in display methods and handles missing item lore when building category and blueprint items. Adds tests for display operations and inventory views.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~30 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 48d94

The change adds tests, a coverage gate, and small station-behavior safeguards. No concrete merge-blocking issue was found, so it is ready to merge after the normal CI run.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 48d94

The inspected changes strengthen placement safeguards and preserve construction vetoes before materials are consumed. A new plugin-directed retry option and incomplete integration coverage warrant caution. Existing event-ordering and station-cleanup limitations remain, but the inspected comparison does not establish that this PR materially worsens them.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected player-controlled path affects the player's placement and materials and the selected station's construction state. Plugin listeners control construction cancellation and optional retry retention. This establishes a local player–station–plugin boundary, not complete exposure coverage for external plugins or the server.

Trust Boundaries and Controls

  • observed — Construction vetoes are checked after synchronous event dispatch and before inputs are consumed. Retention does not bypass the veto: a subsequent click dispatches another event. Replacement placements invalidate older callbacks by object identity, and quit or the placement's own timeout removes pending state.

Resilience and Maintainability Implications

  • inferred — Two ownership and protection limitations predate this PR: default-priority break handlers can mutate station state before a later listener cancels, and station removal does not invalidate placements referencing the removed instance. The new early returns narrow cancelled-break behavior but do not resolve these limitations. No material worsening was established by the inspected comparison.

Hardening Proposals

  • proposed — Validate the new retry contract with actual construction and protection listeners, including later-priority break cancellation and station removal during a retained placement. Consider final-outcome-aware break mutation and dependent-placement invalidation as follow-up hardening of the pre-existing limitations.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.06% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 66 functions across 16 files. (6 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: enforcing complete VFBuilders coverage and fixing construction-related regressions.
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 6.06% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 66 functions across 16 files. (6 skipped: 6 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 coverage trail
With test jars packed inside a pail
Station paths now mind the world
Lore and displays gently unfurled
New reports hop into view!

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

@ryanbarlow97
ryanbarlow97 marked this pull request as ready for review September 29, 2026 21:17
@ryanbarlow97
ryanbarlow97 merged commit a0609bf into main Sep 29, 2026
2 checks passed
@ryanbarlow97
ryanbarlow97 deleted the test/full-coverage branch September 29, 2026 21:48
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