[#11449] Fix Mockito agent: use late binding @{} interpolation and extend to failsafe plugin#12369
[#11449] Fix Mockito agent: use late binding @{} interpolation and extend to failsafe plugin#12369Hiteshsai007 wants to merge 2 commits into
Conversation
… extend to failsafe plugin
The mockito profile activated by dependency:properties used Maven property
interpolation (\) which resolves at POM parse time - before the
dependency:properties goal runs and sets the org.mockito:mockito-core:jar
property. This caused the javaagent path to remain unresolved.
Fix: use Surefire/Failsafe late-binding property interpolation (@{...})
which resolves the property at test execution time, after dependency:properties
has already set it.
Also:
- Restore @{jacocoArgLine} placeholder (was dropped in previous attempt)
- Extend the profile to cover maven-failsafe-plugin as well
Fixes: https://issues.apache.org/jira/browse/MNG-11449
There was a problem hiding this comment.
Pull request overview
Updates the root build’s mockito profile so the Mockito -javaagent path is resolved at test execution time (after dependency:properties has populated org.mockito:mockito-core:jar), and applies the same behavior consistently to both unit tests (Surefire) and integration tests (Failsafe).
Changes:
- Switch Mockito agent path interpolation from Maven parse-time (
${...}) to Surefire/Failsafe late-binding (@{...}). - Restore inclusion of
@{jacocoArgLine}in the profile’sargLine. - Add equivalent
argLineconfiguration formaven-failsafe-pluginunder themockitoprofile.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
gnodet
left a comment
There was a problem hiding this comment.
Review — Fix Mockito agent: use late-binding @{} interpolation and extend to failsafe plugin
What the PR does: This 1-file, 8-line change fixes the mockito Maven profile so the Mockito Java agent path resolves correctly at test execution time rather than at POM parse time. It switches the surefire <argLine> from ${org.mockito:mockito-core:jar} (eager, parse-time interpolation) to @{org.mockito:mockito-core:jar} (Surefire/Failsafe late-binding, resolves after dependency:properties has run). It also restores the @{jacocoArgLine} placeholder that the old mockito-profile argLine was missing, and extends the configuration to maven-failsafe-plugin for integration tests.
Verdict: The change looks correct. The fix aligns with how @{jacocoArgLine} is already used throughout this POM (lines 700, 711, 1220) and properly handles the case where jacocoArgLine is empty (it's defaulted to empty at line 174). The pluginManagement placement is consistent with the existing surefire entry in the profile, and Maven's configuration merge semantics mean only <argLine> is overridden — the <forkNode> and <systemPropertyVariables> from the default pluginManagement are inherited correctly.
Findings
No confirmed or plausible bugs were found in this PR's changes.
Pre-existing note (not introduced by this PR)
pom.xml:1220 — When both the jacoco and mockito profiles are active simultaneously, the jacoco profile's direct <build><plugins> surefire config (<argLine>-Xmx1G @{jacocoArgLine}</argLine>) takes precedence over the mockito profile's <build><pluginManagement> surefire config, causing the -javaagent:@{org.mockito:mockito-core:jar} to be silently dropped for unit tests. This was already the case before this PR (the old ${...} syntax had the same override behavior), so it's not a regression — but it may be worth a follow-up if both profiles are expected to be used together. Failsafe is unaffected since the jacoco profile doesn't override failsafe's argLine.
🤖 Generated with Claude Code
When both jacoco and mockito profiles were active, the jacoco profile's surefire configuration overrode the mockito profile's surefire pluginManagement configuration. This caused the mockito javaagent to be silently dropped for unit tests.
Fix: Refactor the surefire and failsafe argLine to be composed dynamically via properties:
- Define argLine.xmx (default: -Xmx256m) and argLine.mockito (default: empty) in the root POM properties.
- Update default pluginManagement configuration for surefire/failsafe to: \ @{jacocoArgLine} \
- Modify the mockito profile to only set argLine.mockito property to the mockito javaagent.
- Modify the jacoco profile to only override argLine.xmx property to -Xmx1G and remove the direct surefire argLine override.
This ensures both profiles can be active simultaneously without overriding each other's configurations.
|
Thanks for the review! I've updated the PR to resolve the profile collision highlighted in the pre-existing note:
This ensures both profiles can be active at the same time without conflicting or dropping each other's parameters. |
The mockito profile activated by dependency:properties used Maven property interpolation () which resolves at POM parse time - before the dependency:properties goal runs and sets the org.mockito:mockito-core:jar property. This caused the javaagent path to remain unresolved.
Fix: use Surefire/Failsafe late-binding property interpolation (@{...}) which resolves the property at test execution time, after dependency:properties has already set it.
Also:
Fixes: #11449
Following this checklist to help us incorporate your
contribution quickly and easily:
Note that commits might be squashed by a maintainer on merge.
This may not always be possible but is a best-practice.
mvn verifyto make sure basic checks pass.A more thorough check will be performed on your pull request automatically.
If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.