test: add per-platform startup graph regression tests#66
Merged
robinbraemer merged 4 commits intoJul 25, 2026
Conversation
Add plugin startup/smoke tests for every supported Connect platform (Velocity, Spigot, Bungee) plus a core shared-graph test, so a startup/DI-provisioning regression is caught in CI instead of by users. This is the prevention for the class of bug behind BOTH the Velocity 4.0.0 Guice 7 failure (BedrockIdentityKeyProvider unprovisionable) and the Java-26 Libp2pEndpointRuntime constructor arity failure. - core testFixtures StartupGraphProvisioning: pure-JDK reflective @Inject-graph walker + a Guice 7 injectable-constructor rule replica + javax.inject scan, generalizing BedrockVelocityGuice7ProvisioningTest to the whole per-platform graph so new DI classes are covered automatically. - Per-platform tests provision each platform's real Guice module graph (Velocity boots the real ProxyCommonModule + VelocityPlatformModule child injector; Spigot/Bungee wire Server/ProxyCommonModule with platform SDK stubbed, documented) and assert Guice-7 provisionability. The Velocity test fails on the pre-fix javax annotations and passes on the fix. - Java-26 reflective-arity class stays guarded by the existing signature tests (Libp2pEndpointRuntimeInitTest / Libp2pRuntimeBoundaryTest). - CI: pullrequest.yml runs ./gradlew build on a JDK 17/21 matrix; artifacts from 17. Rationale documented (Gradle 8.5 caps the runnable JDK).
…d platform bindings
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
1 participant
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.
Intent
Add durable CI tests that boot/start the Connect plugin on EVERY supported platform (Velocity, Spigot/Bukkit/Paper, BungeeCord — enumerated from the repo's platform modules; there is no Fabric module so it is intentionally not included) and assert the plugin's DI graph provisions and initializes cleanly, so a startup/DI regression is caught in CI instead of by users. This is the captain-directed prevention for the class of bug that produced BOTH the Velocity 4.0.0 Guice-7 DI failure (BedrockIdentityKeyProvider/BedrockAdmissionCoordinator unprovisionable because they were annotated with javax.inject, which Velocity 4's Guice 7 ignores) AND the Java-26 Libp2pEndpointRuntime reflective-constructor arity failure.
Design decisions/tradeoffs made: (1) A full real boot of ConnectPlatform is deliberately NOT driven because ConnectPlatform.init() loads config via EndpointNameGenerator, which makes a network call — not hermetic. Instead each test provisions the real Guice module graph up to the plugin's key singletons. (2) This repo builds/tests against Guice 6, which accepts BOTH javax.inject and com.google.inject, so a real Guice-6 provision cannot reproduce the Velocity-4 failure; the shared testFixtures helper (core/src/testFixtures StartupGraphProvisioning) instead replays Guice 7's exact injectable-constructor discovery rule + a javax.inject scan across the reflectively-walked @Inject graph — this is what fails pre-fix and passes post-fix, and it generalizes the existing BedrockVelocityGuice7ProvisioningTest from a hand-listed set to the whole per-platform graph. (3) Spigot/Bungee plugin entrypoints extend Bukkit JavaPlugin / Bungee Plugin and cannot be instantiated off-server, so their real-provision half wires Server/ProxyCommonModule with platform-supplied bindings stubbed (documented limitation) — still provisioning the full Connect object graph a DI regression would break. Velocity's entrypoint takes an Injector so its child injector is booted for real. (4) I verified the required property by reverse-applying the #64 fix (restoring javax) and confirming the Velocity + core tests fail naming BedrockIdentityKeyProvider, then restoring the fix so they pass. (5) The Java-26 reflective-arity regression class stays guarded by the existing signature-level tests (Libp2pEndpointRuntimeInitTest/Libp2pRuntimeBoundaryTest), which are JDK-independent and referenced in the new tests' docs. (6) CI: pullrequest.yml now runs ./gradlew build on a JDK 17/21 matrix (artifacts only from 17); it stops at 21 because Gradle 8.5 cannot run on JDK 26, with rationale documented. New tests run on every PR via the existing build task.
Also added test infrastructure to the bungee module (it previously had no tests) and wired core java-test-fixtures consumed by the platform modules. AGENTS.md/CLAUDE.md updated with a concise pointer to the guard.
What Changed
@Injectgraph forjavax.injectregressions.Risk Assessment
✅ Low: The change is bounded to CI and test infrastructure and covers the previously identified startup graph gaps without altering production behavior.
Testing
No baseline test command was provided. On JDK 21 with Gradle 8.5, the targeted Velocity, Spigot, Bungee, shared graph, libp2p reflective-boundary, and Bedrock Guice-7 tests executed successfully; reviewer-visible result extracts were saved in the required evidence directory, and the worktree remains clean.
Evidence: Targeted JUnit results
Evidence: Bedrock Guice-7 results
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed (3) ✅
core/src/testFixtures/java/com/minekube/connect/startup/StartupGraphProvisioning.java:156- Field/method-injected roots are excluded from the checked set:WatcherRegister,VelocityListener,BungeeListener, andSpigotDataAddonhave no injectable constructor, so they are only enqueued, not discovered. The later early return for types without an injectable constructor also skips theirjavax.injectmember scan. A Guice-7 regression on one of these active startup members could therefore pass; include member-injected roots and scan their annotations.velocity/src/test/java/com/minekube/connect/VelocityPluginStartupTest.java:127- The intent requires durable tests for the plugin DI graph on every supported platform, but the real provisioning only creates the common module graph and resolves selected keys; it does not install the platform enable modules or follow explicit interface bindings. For example,VelocityPlatformModule'sVelocityCommandUtil/VelocityPlatformUtilsandVelocityListenerModule'sListenerRegisterare not exercised, with analogous omissions in Spigot/Bungee. A startup DI regression in those concrete platform bindings can pass all new tests. Please confirm whether this narrower key-singleton slice is intentional or expand the graph roots/provisioning.🔧 Fix: Expand startup graph coverage for member-injected platform bindings
1 warning still open:
spigot/src/test/java/com/minekube/connect/SpigotPluginStartupTest.java:83-SpigotPlugin.onEnable()installsSpigotAddonModule, whose providers createAddonManagerAddon,DebugAddon, andPacketHandlerAddon;AddonRegisterthen injects members into every addon. The roots include onlySpigotDataAddon, so Guice-7/member-injection regressions in those three real startup classes are still invisible. Add them tospigotGraphRoots()and assert the walk reaches them.🔧 Fix: Cover Spigot addon bindings in startup graph tests
1 warning still open:
core/src/testFixtures/java/com/minekube/connect/startup/StartupGraphProvisioning.java:117- The shared roots claim to cover startup singletons, but omitCommandRegisterandListenerRegister, which are eager singletons installed by every platform's enable path;AddonRegisteris likewise eager inSpigotAddonModule. Because the real tests do not install those enable modules, the reflective walk never checks their@Injectconstructors/methods. A Guice-7/javax.injectregression in registrar initialization can still pass. Add these registrar classes to the shared/platform roots or provision the enable modules with hermetic stubs.🔧 Fix: Cover enable-time registrars in startup graph tests
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
./gradlew --no-daemon --console=plain :core:test --tests com.minekube.connect.startup.PluginGraphStartupTest --tests com.minekube.connect.tunnel.p2p.Libp2pEndpointRuntimeInitTest --tests com.minekube.connect.tunnel.p2p.Libp2pRuntimeBoundaryTest./gradlew --no-daemon --console=plain :velocity:test --tests com.minekube.connect.VelocityPluginStartupTest./gradlew --no-daemon --console=plain :spigot:test --tests com.minekube.connect.SpigotPluginStartupTest./gradlew --no-daemon --console=plain :bungee:test --tests com.minekube.connect.BungeePluginStartupTest./gradlew --no-daemon --console=plain :core:test --tests com.minekube.connect.bedrock.BedrockVelocityGuice7ProvisioningTestJUnit XML inspection confirming selected test methods executed with zero failures/errorsWorking-tree and evidence cleanup verification✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.