build: drop the redundant Failsafe version pin in token-sheriff-client - #711
Conversation
The pinned <version.maven.failsafe> rested on a false premise. Its comment
claimed:
"token-sheriff-client inherits no maven-failsafe-plugin pluginManagement
from cui-java-parent or token-sheriff-parent"
cui-parent-pom does declare maven-failsafe-plugin in build/pluginManagement as
${maven.failsafe.plugin.version}, outside <profiles>, so it is inherited
unconditionally. That entry is present in 1.5.11, 1.6.0 and 1.6.1 alike, so the
claim was untrue when it was written, not merely stale.
The pin is what made the last parent bump a deadlock. The enforcer's
RequireSameVersions rule compares Surefire against Failsafe, so a locally pinned
Failsafe turns any parent-driven Surefire move into a build break until the pin
is hand-edited to match. That is exactly what happened: #707 (parent 1.6.1,
Surefire 3.6.0) and #703 (Failsafe 3.6.0) each failed alone and only passed when
landed together. Inheriting the version removes that failure mode - the two
plugins now move in lockstep by construction.
API-Sheriff is the counter-example: it declares Surefire/Failsafe in seven
places and pins a version in none of them.
Only the version is removed. The <executions> binding stays - it is what makes
the wired-flow *IT tier run under `mvn verify`, and that is genuinely local.
Verified with `mvnw clean verify -pl token-sheriff-client`: failsafe:3.6.0
resolves from the parent, the 7 wired-flow ITs execute, refresh-path-coverage-check
passes, BUILD SUCCESS.
Co-authored-by: plan-marshall <noreply@cuioss.de>
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Excluded labels (none allowed) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: cuioss/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
There was a problem hiding this comment.
Sorry @cuioss-oliver, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 1 day and 3 hours by commenting @sourcery-ai review. Upgrade to get a review now.
Removes the locally pinned
<version.maven.failsafe>. This is the line that turned the last parent bump into a deadlock.The comment was wrong
The property justified itself with:
cui-parent-pomdoes declare it, inbuild/pluginManagement/plugins/plugin(lines 308–312):<build>spans lines 156–563 and<profiles>starts at 565, so this is outside any profile — inherited unconditionally. And it is present in 1.5.11, 1.6.0 and 1.6.1 alike, so the claim was untrue when written, not merely stale.Why it mattered
The enforcer's
RequireSameVersionsrule compares Surefire against Failsafe. A locally pinned Failsafe turns any parent-driven Surefire move into a build break until the pin is hand-edited to match. That is precisely what happened last cycle:Two PRs that each looked individually broken, neither fixable by rebasing — an interlock manufactured entirely by this pin. With the version inherited, the two plugins move in lockstep by construction and this failure mode cannot recur.
Counter-example
API-Sheriff declares
maven-surefire-plugin/maven-failsafe-pluginin seven places acrosspom.xml,api-sheriff/,integration-tests/andbenchmarks/, and pins a version in none of them. TokenSheriff'stoken-sheriff-clientwas the only pin in either repo.Scope
Only the
<version>is removed. The<executions>binding stays — that is what makes the wired-flow*ITtier run undermvn verify, and it is genuinely local (no Failsafe binding is inherited, only the version). The stale comment is replaced with one recording why the version is deliberately absent.Verification
./mvnw clean verify -pl token-sheriff-client→ BUILD SUCCESS:failsafe:3.6.0:integration-test— version resolves from the parentWired-flow scaffold— 7 ITs execute, so the binding is intactfailsafe:3.6.0:verifyrunsjacoco:check (refresh-path-coverage-check)— All coverage checks have been metRequireSameVersionspasses (Surefire 3.6.0 / Failsafe 3.6.0)Effective-POM check confirms both plugins resolve to
3.6.0with no local property.