gnodet-bot commented on code in PR #355:
URL:
https://github.com/apache/maven-clean-plugin/pull/355#discussion_r4120951137
##########
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:
💡 **Same stale comment** — deletion does not run synchronously in fast mode.
```suggestion
// The old thread-name check ('mvn-background-cleaner') was brittle.
Instead,
// verify the fast-delete path was used by checking for the debug message
// emitted by deleteInBackground().
```
##########
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:
💡 **Stale comment from intermediate commits.** This PR no longer makes
deletion synchronous when `failOnError=true` — deletion is always asynchronous
in fast mode. The real reason the thread name check was replaced is that the
old `mvn-background-cleaner` thread name assertion was brittle; the debug
message works regardless of execution mode.
```suggestion
// The old thread-name check ('mvn-background-cleaner') was brittle.
Instead,
// verify the fast-delete path was used by checking for the debug message
// emitted by deleteInBackground().
```
##########
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:
💡 **Same stale comment** — deletion does not run synchronously in fast mode.
```suggestion
// The old thread-name check ('mvn-background-cleaner') was brittle.
Instead,
// verify the fast-delete path was used by checking for the debug message
// emitted by deleteInBackground().
assert log.contains( 'in background' ) : 'expected background cleaner
staging message in log'
```
##########
src/it/read-only-fast/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:
💡 **Same stale comment** — deletion does not run synchronously in fast mode.
```suggestion
// The old thread-name check ('mvn-background-cleaner') was brittle.
Instead,
// verify the fast-delete path was used by checking for the debug message
// emitted by deleteInBackground().
```
--
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]
