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]

Reply via email to