Copilot commented on code in PR #2667:
URL: https://github.com/apache/groovy/pull/2667#discussion_r3562835184
##########
src/test/groovy/org/codehaus/groovy/tools/FileSystemCompilerTest.java:
##########
@@ -94,6 +103,55 @@ public void testCommandLine() throws Exception {
FileSystemCompiler.commandLineCompile(new String[]
{"src/test/groovy/groovy/LittleClosureTest.groovy", "-d", dir.getPath()});
}
+ // warnings collected during a SUCCESSFUL compile must reach the
command-line user; the
+ // failure path already surfaces them alongside the errors
+ @Test
+ public void testWarningsAreDisplayedOnSuccessfulCompile() throws Exception
{
+ String output = compileCapturingStderr("WarnDemo",
warningEmittingConfiguration());
+ assertTrue(output.contains("spike warning marker"), "expected the
collected warning on stderr but got: " + output);
+ assertTrue(output.contains("1 warning"), "expected a warning summary
but got: " + output);
+ }
+
+ @Test
+ public void testCleanCompilePrintsNothingToStderr() throws Exception {
+ String output = compileCapturingStderr("CleanDemo", null);
+ assertTrue(output.isEmpty(), "clean compile should stay silent but
got: " + output);
+ }
+
+ private static CompilerConfiguration warningEmittingConfiguration() {
+ CompilerConfiguration configuration = new CompilerConfiguration();
+ configuration.addCompilationCustomizers(new
CompilationCustomizer(CompilePhase.CANONICALIZATION) {
+ @Override
+ public void call(SourceUnit source, GeneratorContext context,
ClassNode classNode) {
+
source.getErrorCollector().addWarning(WarningMessage.LIKELY_ERRORS, "spike
warning marker",
+ Token.newString(classNode.getName(), 1, 1), source);
+ }
+ });
+ return configuration;
+ }
+
+ private static String compileCapturingStderr(String className,
CompilerConfiguration configuration) throws Exception {
+ File dir = new File("build/test-generated-classes/warn");
+ dir.mkdirs();
+ File source = new File(dir, className + ".groovy");
+ Files.write(source.toPath(), ("class " + className + " {
}").getBytes());
+
+ if (configuration == null) {
+ configuration = new CompilerConfiguration();
+ }
+ configuration.setTargetDirectory(dir);
+
+ PrintStream originalErr = System.err;
+ ByteArrayOutputStream captured = new ByteArrayOutputStream();
+ try {
+ System.setErr(new PrintStream(captured, true));
+ FileSystemCompiler.doCompilation(configuration, null, new
String[]{source.getPath()}, false);
+ } finally {
+ System.setErr(originalErr);
+ }
+ return captured.toString();
+ }
Review Comment:
The stderr-capturing helper leaks resources and is platform-encoding
dependent (`getBytes()`/`toString()` use the default charset). It also reuses a
fixed output directory without cleanup, which can cause flaky behavior across
repeated/parallel test runs. Prefer a per-test temp directory under `build/`,
explicit UTF-8, and try-with-resources for the capturing stream.
##########
src/main/java/org/codehaus/groovy/tools/FileSystemCompiler.java:
##########
@@ -298,6 +300,7 @@ public static void doCompilation(CompilerConfiguration
configuration, Compilatio
.setResourceLoader(filename -> null);
}
compiler.compile(filenames);
+ displayWarnings(compiler.unit);
Review Comment:
`doCompilation` is used by non-command-line callers (e.g., the Ant `Groovyc`
task) and printing warnings directly to `System.err` changes that API’s
behavior by bypassing the caller’s logging/diagnostic routing. This can lead to
unexpected console output for Ant builds even when they handle errors via
`ErrorReporter`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
