gnodet-bot commented on code in PR #355:
URL:
https://github.com/apache/maven-clean-plugin/pull/355#discussion_r4112161957
##########
src/test/java/org/apache/maven/plugins/clean/BackgroundCleanerTest.java:
##########
@@ -345,6 +345,73 @@ void scanForLeftoversIsNoOpWhenFastDirAbsent(@TempDir Path
tempDir) throws IOExc
assertNotNull(bc);
}
+ // -----------------------------------------------------------------------
+ // failOnError=true — synchronous deletion that fails the build
+ // -----------------------------------------------------------------------
+
+ /**
+ * When {@code failOnError=true} and the deletion fails, {@link
BackgroundCleaner#fastDelete}
+ * must throw an {@link IOException} synchronously so that the build can
be failed.
+ * This verifies that {@code failOnError} now works correctly in fast mode
(fixes #352).
+ */
+ @Test
+ @DisabledOnOs(OS.WINDOWS)
+ void failOnErrorThrowsWhenDeletionFails(@TempDir Path tempDir) throws
Exception {
+ Path fastDir = tempDir.resolve(".clean");
+ Path target = createDirectory(tempDir.resolve("target"));
+ Path subDir = createDirectory(target.resolve("subdir"));
+ createFile(subDir.resolve("file.txt"));
+ // Make the directory read-only so deletion of its contents fails
(force=false).
+ Files.setPosixFilePermissions(subDir,
PosixFilePermissions.fromString("r-xr-xr-x"));
+
+ Log log = mock(Log.class);
+ Session session = mockSession(null);
+
+ BackgroundCleaner bc = BackgroundCleaner.getOrCreate(session, log,
fastDir, FastMode.BACKGROUND);
+ try {
+ // failOnError=true must cause fastDelete to throw synchronously.
+ org.junit.jupiter.api.Assertions.assertThrows(
+ IOException.class,
+ () -> bc.fastDelete(target, false, false, true),
+ "fastDelete with failOnError=true must throw when deletion
fails");
+ } finally {
+ makeWritableRecursively(tempDir);
+ }
+ }
+
+ /**
+ * When {@code failOnError=false} and the deletion fails, {@link
BackgroundCleaner#fastDelete}
+ * must return normally (error is logged as a warning at session end, not
propagated).
+ * This is the existing behaviour for the asynchronous path.
+ */
+ @Test
+ @DisabledOnOs(OS.WINDOWS)
+ void failOnErrorFalseDoesNotThrowWhenDeletionFails(@TempDir Path tempDir)
throws Exception {
+ Path fastDir = tempDir.resolve(".clean");
+ Path target = createDirectory(tempDir.resolve("target"));
+ Path subDir = createDirectory(target.resolve("subdir"));
+ createFile(subDir.resolve("file.txt"));
+ Files.setPosixFilePermissions(subDir,
PosixFilePermissions.fromString("r-xr-xr-x"));
+
+ Log log = mock(Log.class);
+ ArgumentCaptor<Listener> captor =
ArgumentCaptor.forClass(Listener.class);
+ Session session = mockSession(captor);
+
+ BackgroundCleaner bc = BackgroundCleaner.getOrCreate(session, log,
fastDir, FastMode.BACKGROUND);
+ try {
+ // failOnError=false: fastDelete must not throw; error is reported
as a warning.
+ bc.fastDelete(target, false, false, false);
+
+ Event event = mock(Event.class);
+ when(event.getType()).thenReturn(EventType.SESSION_ENDED);
+ // Must not throw; error is logged as a warning.
+ captor.getValue().onEvent(event);
+ verify(log, atLeastOnce()).warn(any(CharSequence.class),
any(Throwable.class));
+ } finally {
+ makeWritableRecursively(tempDir);
+ }
+ }
+
// -----------------------------------------------------------------------
// failOnError — background path cannot structurally fail the build
Review Comment:
Stale Javadoc: this section header (and the Javadoc at lines 419-425) still
says _"failOnError has no effect when fast=true"_ and _"a session-end listener
cannot structurally fail the build"_. That was the old behavior; this PR
specifically fixes it. The test now exercises the `failOnError=false` async
path, so the doc should be updated to match (e.g. _"failOnError=false: async
path logs warnings instead of throwing"_).
--
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]