Move legacy command compatibility tests into an end-to-end build - #16059
Draft
jdaugherty wants to merge 1 commit into
Draft
Move legacy command compatibility tests into an end-to-end build#16059jdaugherty wants to merge 1 commit into
jdaugherty wants to merge 1 commit into
Conversation
jdaugherty
force-pushed
the
test/8.0.x-legacy-command-e2e
branch
3 times, most recently
from
July 29, 2026 03:07
b81851a to
e6b80b1
Compare
Addresses the outstanding review feedback on the Grails 7 command compatibility layer. Build placement. The Grails 7 fixture needs Java 17 - the minimum for a Grails 7 app, so the binary matches what a real Grails 7 plugin is built with - but expressing that as a Gradle toolchain put an unprovisionable JDK requirement into the core build's task graph. Nothing in the repo provisions a 17 (no foojay resolver, no toolchainManagement, no org.gradle.java.installations.*), and because the fixture jar was an implementation dependency, `./gradlew build -PskipTests` reached it - so the build failed on the JDK the project documents in .sdkmanrc and in the reproducible-build container. CI only passed because the runner images happen to ship a 17 that Gradle auto-detects. The three projects now live in a new top-level end-to-end build, so the root build no longer reaches them. The fixture declares its JDK and Gradle version in its own .sdkmanrc, matching what Grails 7 pins (17 and 8.14.5) rather than what this repository builds with, and is excluded from the gradle-bootstrap wrapper propagation for that reason; the end-to-end build itself is added to that propagation and tracks the root. A dedicated workflow provisions both JDKs, reading the versions out of those files. Resolution goes through published artifacts rather than project substitution, which is what makes these tests end-to-end: they consume grails-core the way an application does, through real poms and module metadata. The repository is the same build/local-maven that grails-forge points its generated applications at, populated by publishAllPublicationsToTestCaseMavenRepoRepository in both the root and grails-gradle builds. settings.gradle scopes it with exclusiveContent so a remote snapshot cannot quietly satisfy an org.apache.grails request and leave the suite testing something other than the working tree. That also disposes of the CLI companion problem rather than working around it. grails-core-cli is a secondary capability of :grails-core, not a project, so composite substitution cannot express it and hits a capability self-conflict - but it is a first-class published module whose metadata CliPublishingSupport already rewrites for external consumers, so resolving from the repository gets it for free. Trait-derived command names. Every legacy command in the tree overrode getName()/getDescription(), so the trait's default derivation - what create-command generated on Grails 7, and therefore what most published Grails 7 commands rely on for their registration key - had no coverage. Adds a third precompiled command declaring neither getter. Factory resource failures are no longer silent. loadFactoryDeclarations dropped a malformed resource with no log line at any level, and it backs modern grails-cli.factories discovery, so one bad file lost every one of a plugin's Grails 8 commands and providers. It now warns with the resource URL and cause, keeping the per-resource isolation. skipBootstrap resolution is covered. The lookup moves into a static helper so the adapter-target case - where reading the flag off the adapter instead of its target would let BootStrap run during dbm-update - is guarded by a test. Commands are now deduplicated across the registry and context classloaders the way providers already were. A command declared in a resource visible through both was constructed twice and the second instance discarded, which the surrounding comment says the code exists to avoid; an existing assertion encoded that double construction and now asserts a single one. Legacy commands take part in ordering. Modern commands are sorted before first-wins registration, but legacy ones were registered in factory scan order and the adapter carried none of the target's ordering, so two Grails 7 plugins declaring the same command name resolved by jar order. The adapter now projects the target's Ordered/@order onto itself and the provider sorts before registering, restoring Grails 7 semantics. ThreadDeath handling is removed from the four fatal-error guards. Thread.stop() was removed in JDK 20, so on the 21 baseline the only instances that branch ever saw were the ones the tests constructed, and the type is deprecated for removal. VirtualMachineError handling and the wrapped-cause walk are unchanged, and the specs now prove rethrow across two VirtualMachineError subtypes instead. The duplicated rethrowIfFatal is deliberately left in place: grails-shell-cli cannot see grails-core at compile time, so consolidating would mean new public API on a general-purpose utility for a CLI-internal concern. Test quality. A spec named for a plugin-origin resolution failure described a code path that does not exist and whose failure branch was unreachable; it is renamed to what it verifies, with the dead branch removed. AbstractProfile's unknown-command path, changed by this work but uncovered, gets a spec driving the public Profile.handleCommand. Also collapses two byte-identical instantiate helpers, drops a duplicated fixture version pin in the integration spec, and corrects comments describing the fixture as an included composite build.
jdaugherty
force-pushed
the
test/8.0.x-legacy-command-e2e
branch
from
July 29, 2026 03:15
e6b80b1 to
afa4d80
Compare
🚨 TestLens detected 1 failed test 🚨Here is what you can do:
Test SummaryCI / Build Grails Forge (Java 25, indy=false) > :grails-forge-cli:test [grails-forge]
🏷️ Commit: afa4d80 Test FailuresCreateControllerCommandSpec > test app with controller (:grails-forge-cli:test [grails-forge] in CI / Build Grails Forge (Java 25, indy=false))Muted TestsSelect tests to mute in this pull request:
Reuse successful test results:
Click the checkbox to trigger a rerun:
Learn more about TestLens at testlens.app. |
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.
Addresses the outstanding review feedback on the Grails 7 command compatibility layer.
Build placement. The Grails 7 fixture needs Java 17 - the minimum for a Grails 7 app, so the binary matches what a real Grails 7 plugin is built with - but expressing that as a Gradle toolchain put an unprovisionable JDK requirement into the core build's task graph. Nothing in the repo provisions a 17 (no foojay resolver, no toolchainManagement, no org.gradle.java.installations.*), and because the fixture jar was an implementation dependency,
./gradlew build -PskipTestsreached it - so the build failed on the JDK the project documents in .sdkmanrc and in the reproducible-build container. CI only passed because the runner images happen to ship a 17 that Gradle auto-detects.The three projects now live in a new top-level end-to-end build, so the root build no longer reaches them. Each half declares its JDK in its own .sdkmanrc instead of a toolchain, and a dedicated workflow provisions both, reading the versions out of those files.
Resolution goes through published artifacts rather than project substitution, which is what makes these tests end-to-end: they consume grails-core the way an application does, through real poms and module metadata. The repository is the same build/local-maven that grails-forge points its generated applications at, populated by publishAllPublicationsToTestCaseMavenRepoRepository in both the root and grails-gradle builds. settings.gradle scopes it with exclusiveContent so a remote snapshot cannot quietly satisfy an org.apache.grails request and leave the suite testing something other than the working tree.
That also disposes of the CLI companion problem rather than working around it. grails-core-cli is a secondary capability of :grails-core, not a project, so composite substitution cannot express it and hits a capability self-conflict - but it is a first-class published module whose metadata CliPublishingSupport already rewrites for external consumers, so resolving from the repository gets it for free.
The Grails 7 fixture stays outside all of this as a standalone build under its own JDK, consumed as a prebuilt jar.
Trait-derived command names. Every legacy command in the tree overrode getName()/getDescription(), so the trait's default derivation - what create-command generated on Grails 7, and therefore what most published Grails 7 commands rely on for their registration key - had no coverage. Adds a third precompiled command declaring neither getter.
Factory resource failures are no longer silent. loadFactoryDeclarations dropped a malformed resource with no log line at any level, and it backs modern grails-cli.factories discovery, so one bad file lost every one of a plugin's Grails 8 commands and providers. It now warns with the resource URL and cause, keeping the per-resource isolation.
skipBootstrap resolution is covered. The lookup moves into a static helper so the adapter-target case - where reading the flag off the adapter instead of its target would let BootStrap run during dbm-update - is guarded by a test.
Also drops a duplicated fixture version pin in the integration spec and corrects comments describing the fixture as an included composite build.