[
https://issues.apache.org/jira/browse/HADOOP-19948?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ferenc Erdelyi updated HADOOP-19948:
------------------------------------
Description:
JaCoCo coverage (introduced in HADOOP-15190) injects its agent by having the
shared Surefire/Failsafe configuration in {{hadoop-project/pom.xml}} end its
{{<argLine>}} with the late-bound {{@{argLine}}} token, which
{{jacoco:prepare-agent}} substitutes with the {{-javaagent}} option at test
time.
The problem is that a module-level {{<argLine>}} fully replaces the inherited
one. Any module that overrides {{<argLine>}} (there are ~21 today) must
remember to re-append {{@{argLine}}}; if it doesn't, the JaCoCo agent is never
attached to that module's forked test JVMs and the module contributes empty
coverage.
HADOOP-15190 added a build-time guard ({{check-coverage-modules.sh}}) that now
detects this — it fails the build when a test-bearing module overrides
{{<argLine>}} without {{@{argLine}}}. That closes the "silent" aspect, but it
is a policing net: every module author must still remember the token, and the
guard exists only to catch them when they forget.
Proposal: remove the footgun entirely so the guard has nothing to police. Stop
overriding {{<argLine>}} in individual modules; define the shared
Surefire/Failsafe {{<argLine>}} once (in {{hadoop-project/pom.xml}}) so it
always contains {{@{argLine}}}, and expose a property (e.g.
{{surefire.module.argLine}}, empty by default) for the extra per-module JVM
arguments:
{code:xml}
<!-- shared, in hadoop-project/pom.xml -->
<argLine>@{argLine} ${maven-surefire-plugin.argLine}
${surefire.module.argLine}</argLine>
{code}
Modules that today do {{<argLine>${maven-surefire-plugin.argLine} @{argLine}
-Xmx2048m</argLine>}} would instead set only:
{code:xml}
<surefire.module.argLine>-Xmx2048m</surefire.module.argLine>
{code}
With this, {{@{argLine}}} is guaranteed present for every module and cannot be
dropped.
Scope:
- Refactor the ~21 modules that currently override Surefire/Failsafe
{{<argLine>}} to set the new property instead.
- Update the shared configuration accordingly.
- Once no module overrides {{<argLine>}}, the guard's
{{<argLine>}}/{{@{argLine}}} check (from HADOOP-15190) becomes vestigial and
can be simplified or removed.
Acceptance criteria:
- No module declares its own {{<argLine>}} for Surefire/Failsafe.
- A full coverage build ({{mvn verify -Djacoco.skip=false ...}}) instruments
and reports coverage for every test-bearing module.
Relates to: HADOOP-15190 (added the {{@{argLine}}} mechanism and the detection
guard this issue makes unnecessary).
was:
JaCoCo coverage (introduced in HADOOP-15190) injects its agent by having the
shared Surefire/Failsafe configuration in {{hadoop-project/pom.xml}} end its
{{<argLine>}} with the late-bound {{@{argLine}}} token, which
{{jacoco:prepare-agent}} substitutes with the {{-javaagent}} option at test
time.
The problem is that a module-level {{<argLine>}} fully replaces the inherited
one. Any module that overrides {{<argLine>}} (there are ~21 today) must
remember to re-append {{@{argLine}}}; if it doesn't, the JaCoCo agent is never
attached to that module's forked test JVMs. Its tests still pass, so nothing
fails the build — the module simply contributes empty coverage to the aggregate
report, silently. This is easy to introduce with any new or updated module and
impossible to notice without inspecting the report.
Proposal: remove the footgun by no longer overriding {{<argLine>}} in
individual modules. Define the shared Surefire/Failsafe {{<argLine>}} once (in
{{hadoop-project/pom.xml}}) so it always contains {{@{argLine}}}, and expose a
property (e.g. {{surefire.module.argLine}}, empty by default) for the extra
per-module JVM arguments:
{code:xml}
<!-- shared, in hadoop-project/pom.xml -->
<argLine>@{argLine} ${maven-surefire-plugin.argLine}
${surefire.module.argLine}</argLine>
{code}
Modules that today do {{<argLine>${maven-surefire-plugin.argLine} @{argLine}
-Xmx2048m</argLine>}} would instead set only:
{code:xml}
<surefire.module.argLine>-Xmx2048m</surefire.module.argLine>
{code}
With this, {{@{argLine}}} is guaranteed present for every module and can never
be dropped, eliminating the entire class of silent coverage gaps.
Scope:
- Refactor the ~21 modules that currently override Surefire/Failsafe
{{<argLine>}} to set the new property instead.
- Update the shared configuration accordingly.
Acceptance criteria:
- No module declares its own {{<argLine>}} for Surefire/Failsafe (can be
enforced by the coverage drift-guard added in HADOOP-15190).
- A full coverage build ({{mvn verify -Djacoco.skip=false ...}}) instruments
and reports coverage for every test-bearing module, with none silently empty
due to a missing agent.
Relates to: HADOOP-15190. A lighter-weight static guard (failing the build when
an {{<argLine>}} override lacks {{@{argLine}}}) can catch the mistake, but this
issue removes the ability to make it at all.
> Prevent modules from silently dropping JaCoCo instrumentation by centralizing
> the Surefire/Failsafe argLine
> -----------------------------------------------------------------------------------------------------------
>
> Key: HADOOP-19948
> URL: https://issues.apache.org/jira/browse/HADOOP-19948
> Project: Hadoop Common
> Issue Type: Improvement
> Components: build
> Affects Versions: 3.6.0
> Reporter: Ferenc Erdelyi
> Assignee: Ferenc Erdelyi
> Priority: Minor
>
> JaCoCo coverage (introduced in HADOOP-15190) injects its agent by having the
> shared Surefire/Failsafe configuration in {{hadoop-project/pom.xml}} end its
> {{<argLine>}} with the late-bound {{@{argLine}}} token, which
> {{jacoco:prepare-agent}} substitutes with the {{-javaagent}} option at test
> time.
> The problem is that a module-level {{<argLine>}} fully replaces the inherited
> one. Any module that overrides {{<argLine>}} (there are ~21 today) must
> remember to re-append {{@{argLine}}}; if it doesn't, the JaCoCo agent is
> never attached to that module's forked test JVMs and the module contributes
> empty coverage.
> HADOOP-15190 added a build-time guard ({{check-coverage-modules.sh}}) that
> now detects this — it fails the build when a test-bearing module overrides
> {{<argLine>}} without {{@{argLine}}}. That closes the "silent" aspect, but it
> is a policing net: every module author must still remember the token, and the
> guard exists only to catch them when they forget.
> Proposal: remove the footgun entirely so the guard has nothing to police.
> Stop overriding {{<argLine>}} in individual modules; define the shared
> Surefire/Failsafe {{<argLine>}} once (in {{hadoop-project/pom.xml}}) so it
> always contains {{@{argLine}}}, and expose a property (e.g.
> {{surefire.module.argLine}}, empty by default) for the extra per-module JVM
> arguments:
> {code:xml}
> <!-- shared, in hadoop-project/pom.xml -->
> <argLine>@{argLine} ${maven-surefire-plugin.argLine}
> ${surefire.module.argLine}</argLine>
> {code}
> Modules that today do {{<argLine>${maven-surefire-plugin.argLine} @{argLine}
> -Xmx2048m</argLine>}} would instead set only:
> {code:xml}
> <surefire.module.argLine>-Xmx2048m</surefire.module.argLine>
> {code}
> With this, {{@{argLine}}} is guaranteed present for every module and cannot
> be dropped.
> Scope:
> - Refactor the ~21 modules that currently override Surefire/Failsafe
> {{<argLine>}} to set the new property instead.
> - Update the shared configuration accordingly.
> - Once no module overrides {{<argLine>}}, the guard's
> {{<argLine>}}/{{@{argLine}}} check (from HADOOP-15190) becomes vestigial and
> can be simplified or removed.
> Acceptance criteria:
> - No module declares its own {{<argLine>}} for Surefire/Failsafe.
> - A full coverage build ({{mvn verify -Djacoco.skip=false ...}}) instruments
> and reports coverage for every test-bearing module.
> Relates to: HADOOP-15190 (added the {{@{argLine}}} mechanism and the
> detection guard this issue makes unnecessary).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
