gnodet commented on code in PR #355:
URL: 
https://github.com/apache/maven-clean-plugin/pull/355#discussion_r4120959821


##########
src/test/java/org/apache/maven/plugins/clean/BackgroundCleanerTest.java:
##########
@@ -346,16 +346,15 @@ void scanForLeftoversIsNoOpWhenFastDirAbsent(@TempDir 
Path tempDir) throws IOExc
     }
 
     // -----------------------------------------------------------------------
-    // failOnError — background path cannot structurally fail the build
+    // async path logs warnings instead of throwing
     // -----------------------------------------------------------------------
 
     /**
      * Background deletion failures must be logged as warnings without 
throwing.
      *
-     * <p>{@code failOnError} has no effect when {@code fast=true}: a 
session-end listener cannot
-     * structurally fail the build — Maven catches whatever a listener throws 
and downgrades it to
-     * a warning. This test verifies that errors are reported as warnings and 
that {@code onEvent}
-     * returns normally.</p>
+     * <p>When {@code failOnError=false}, the deletion is offloaded to the 
background executor
+     * thread. This test verifies that errors in that path are reported as 
warnings and that
+     * {@code onEvent} returns normally.</p>

Review Comment:
   Fixed in 835d16c.



##########
src/it/fast-delete-default/verify.groovy:
##########
@@ -24,4 +24,7 @@ if ( new File( basedir, "target" ).exists() )
 }
 
 File buildLog = new File(basedir, 'build.log')
-return buildLog.text.contains('mvn-background-cleaner')
+// With failOnError=true (the default), deletion runs synchronously — the
+// background cleaner thread name won't appear.  Verify that the fast-delete
+// path was used by checking for the staging-directory debug message.

Review Comment:
   Fixed in 835d16c.



##########
src/it/fast-delete/verify.groovy:
##########
@@ -29,4 +29,7 @@ if ( new File( basedir, ".fastdir" ).exists() )
 }
 
 File buildLog = new File(basedir, 'build.log')
-return buildLog.text.contains('mvn-background-cleaner')
+// With failOnError=true (the default), deletion runs synchronously — the
+// background cleaner thread name won't appear.  Verify that the fast-delete
+// path was used by checking for the staging-directory debug message.

Review Comment:
   Fixed in 835d16c.



##########
src/it/fast-delete-multi-module/verify.groovy:
##########
@@ -31,8 +31,11 @@ assert !new File( basedir, 'module-noforce/target' 
).exists() : 'module-noforce/
 // The shared staging directory should be cleaned after the session ends
 assert !new File( basedir, '.fastdir' ).exists() : '.fastdir staging directory 
should have been deleted'
 
-// The background cleaner thread must have been used (proves session-scoped 
sharing)
-assert log.contains( 'mvn-background-cleaner' ) : 'expected background cleaner 
thread name in log'
+// The background cleaner must have been used (proves session-scoped sharing)
+// With failOnError=true (the default), deletion runs synchronously so the
+// background cleaner thread name won't appear.  Check for the 
staging-directory
+// debug message instead.
+assert log.contains( 'in background' ) : 'expected background cleaner staging 
message in log'

Review Comment:
   Fixed in 835d16c.



-- 
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]

Reply via email to