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]