[
https://issues.apache.org/jira/browse/GROOVY-6908?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18074639#comment-18074639
]
ASF GitHub Bot commented on GROOVY-6908:
----------------------------------------
Copilot commented on code in PR #2476:
URL: https://github.com/apache/groovy/pull/2476#discussion_r3106490110
##########
subprojects/groovy-ant/src/test/groovy/org/codehaus/groovy/ant/GroovyTest.java:
##########
@@ -179,6 +184,49 @@ public void testFileNameInStackTrace() {
testFileNameInStackTrace("groovyErrorMsg_ExternalFile",
"GroovyTest_errorMessage.groovy");
}
+ public void testInheritAllPassesProjectProperties() {
+ Groovy task = new Groovy();
+ task.setProject(project);
+ project.setProperty("alpha", "1");
+ project.setProperty("beta", "two");
+
+ task.setInheritAll(true);
+ task.passParentAntProperties();
+
+ Map<String, String> passed = collectSysproperties(task);
+ assertEquals("1", passed.get("alpha"));
+ assertEquals("two", passed.get("beta"));
+ }
+
+ public void testInheritAllDoesNotOverrideExplicitSysproperty() {
+ Groovy task = new Groovy();
+ task.setProject(project);
+ project.setProperty("shared", "fromProject");
+
+ Environment.Variable explicit = new Environment.Variable();
+ explicit.setKey("shared");
+ explicit.setValue("fromSysproperty");
+ task.addSysproperty(explicit);
+
+ task.setInheritAll(true);
+ task.passParentAntProperties();
+
+ int sharedCount = 0;
+ for (Environment.Variable v :
task.getSysProperties().getVariablesVector()) {
+ if ("shared".equals(v.getKey())) sharedCount++;
+ }
+ assertEquals(1, sharedCount);
+ assertEquals("fromSysproperty",
collectSysproperties(task).get("shared"));
+ }
Review Comment:
The new tests validate `passParentAntProperties()` directly, but they don’t
exercise the actual forked execution path where this feature is supposed to
work (i.e., that the properties become `-D` system properties in the child
JVM). Consider adding an integration-style test (e.g., an Ant target with
`fork="true" inheritAll="true"` that writes `System.getProperty(...)` values to
an output file which the test can read) to ensure the end-to-end behavior is
covered.
##########
subprojects/groovy-ant/src/main/java/org/codehaus/groovy/ant/Groovy.java:
##########
@@ -697,6 +721,22 @@ private void createClasspathParts() {
}
}
+ // package-private for testing
+ void passParentAntProperties() {
+ Set<String> alreadySet = new HashSet<>();
+ for (Environment.Variable v :
super.getSysProperties().getVariablesVector()) {
+ alreadySet.add(v.getKey());
+ }
+ for (Map.Entry<String, Object> entry :
getProject().getProperties().entrySet()) {
+ Object value = entry.getValue();
+ if (value == null || alreadySet.contains(entry.getKey())) continue;
+ Environment.Variable var = new Environment.Variable();
+ var.setKey(entry.getKey());
+ var.setValue(value.toString());
+ super.addSysproperty(var);
+ }
+ }
Review Comment:
`passParentAntProperties()` is package-private solely to be invoked from
tests. To avoid expanding the production surface area, prefer keeping it
`private` and testing the behavior through the public Ant task API (forked
execution), or otherwise mark the method clearly as test-only (e.g., with a
dedicated annotation/comment policy used elsewhere in the codebase).
> Groovy Ant task should inherit Ant properties when in "forked" mode
> -------------------------------------------------------------------
>
> Key: GROOVY-6908
> URL: https://issues.apache.org/jira/browse/GROOVY-6908
> Project: Groovy
> Issue Type: Improvement
> Components: Ant integration
> Affects Versions: 2.3.3
> Reporter: Kyrill Alyoshin
> Priority: Major
> Labels: contrib
>
> Groovy Ant task (i.e. Groovy class) when run in *forked* mode doesn't inherit
> existing Ant properties from the JVM where it is forked from. This is most
> unfortunate as "forking" is an incredibly useful feature otherwise.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
