[ https://issues.apache.org/jira/browse/MCLEAN-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17896476#comment-17896476 ]
ASF GitHub Bot commented on MCLEAN-124: --------------------------------------- peterdemaeyer commented on code in PR #60: URL: https://github.com/apache/maven-clean-plugin/pull/60#discussion_r1833389997 ########## src/test/java/org/apache/maven/plugins/clean/CleanerTest.java: ########## @@ -0,0 +1,173 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.plugins.clean; + +import java.io.IOException; +import java.nio.file.AccessDeniedException; +import java.nio.file.DirectoryNotEmptyException; +import java.nio.file.FileSystems; +import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.Set; + +import org.apache.maven.plugin.logging.Log; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.ArgumentCaptor; +import org.mockito.InOrder; + +import static java.nio.file.Files.createDirectory; +import static java.nio.file.Files.createFile; +import static java.nio.file.Files.exists; +import static java.nio.file.Files.getPosixFilePermissions; +import static java.nio.file.Files.setPosixFilePermissions; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assumptions.assumeTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class CleanerTest { + + private static final boolean POSIX_COMPLIANT = + FileSystems.getDefault().supportedFileAttributeViews().contains("posix"); + + private final Log log = mock(); + + @Test + void deleteSucceedsDeeply(@TempDir Path tempDir) throws Exception { + assumeTrue(POSIX_COMPLIANT); + final Path basedir = createDirectory(tempDir.resolve("target")).toRealPath(); + final Path file = createFile(basedir.resolve("file")); + final Cleaner cleaner = new Cleaner(null, log, false, null, null); + cleaner.delete(basedir.toFile(), null, false, true, false); + assertFalse(exists(basedir)); + assertFalse(exists(file)); + } + + @Test + void deleteFailsWithoutRetryWhenNoPermission(@TempDir Path tempDir) throws Exception { + assumeTrue(POSIX_COMPLIANT); + when(log.isWarnEnabled()).thenReturn(true); + final Path basedir = createDirectory(tempDir.resolve("target")).toRealPath(); + createFile(basedir.resolve("file")); + final Set<PosixFilePermission> initialPermissions = getPosixFilePermissions(basedir); + final String rwxrwxr_x = PosixFilePermissions.toString(initialPermissions); + // Prevent directory listing, which will result in a DirectoryNotEmptyException. + final String rw_rw_r__ = rwxrwxr_x.replace('x', '-'); + final Set<PosixFilePermission> permissions = PosixFilePermissions.fromString(rw_rw_r__); + setPosixFilePermissions(basedir, permissions); + try { + final Cleaner cleaner = new Cleaner(null, log, false, null, null); + final IOException exception = + assertThrows(IOException.class, () -> cleaner.delete(basedir.toFile(), null, false, true, false)); + verify(log, never()).warn(any(CharSequence.class), any(Throwable.class)); + assertEquals("Failed to delete " + basedir, exception.getMessage()); + final DirectoryNotEmptyException cause = + assertInstanceOf(DirectoryNotEmptyException.class, exception.getCause()); + assertEquals(basedir.toString(), cause.getMessage()); + } finally { + // Allow the tempDir to be cleared by the @TempDir extension. + setPosixFilePermissions(basedir, initialPermissions); + } + } + + @Test + void deleteFailsAfterRetryWhenNoPermission(@TempDir Path tempDir) throws Exception { + assumeTrue(POSIX_COMPLIANT); + final Path basedir = createDirectory(tempDir.resolve("target")).toRealPath(); + createFile(basedir.resolve("file")); + final Set<PosixFilePermission> initialPermissions = getPosixFilePermissions(basedir); + final String rwxrwxr_x = PosixFilePermissions.toString(initialPermissions); + // Prevent directory listing, which will result in a DirectoryNotEmptyException. + final String rw_rw_r__ = rwxrwxr_x.replace('x', '-'); + final Set<PosixFilePermission> permissions = PosixFilePermissions.fromString(rw_rw_r__); + setPosixFilePermissions(basedir, permissions); + try { + final Cleaner cleaner = new Cleaner(null, log, false, null, null); + final IOException exception = + assertThrows(IOException.class, () -> cleaner.delete(basedir.toFile(), null, false, true, true)); + assertEquals("Failed to delete " + basedir, exception.getMessage()); + final DirectoryNotEmptyException cause = + assertInstanceOf(DirectoryNotEmptyException.class, exception.getCause()); + assertEquals(basedir.toString(), cause.getMessage()); + } finally { + // Allow the tempDir to be cleared by the @TempDir extension. + setPosixFilePermissions(basedir, initialPermissions); + } + } + + @Test + void deleteLogsWarningWithoutRetryWhenNoPermission(@TempDir Path tempDir) throws Exception { + assumeTrue(POSIX_COMPLIANT); + when(log.isWarnEnabled()).thenReturn(true); + final Path basedir = createDirectory(tempDir.resolve("target")).toRealPath(); + final Path file = createFile(basedir.resolve("file")); + final Set<PosixFilePermission> initialPermissions = getPosixFilePermissions(basedir); + final String rwxrwxr_x = PosixFilePermissions.toString(initialPermissions); + final String r_xr_xr_x = rwxrwxr_x.replace('w', '-'); + final Set<PosixFilePermission> permissions = PosixFilePermissions.fromString(r_xr_xr_x); + setPosixFilePermissions(basedir, permissions); + try { + final Cleaner cleaner = new Cleaner(null, log, false, null, null); + assertDoesNotThrow(() -> cleaner.delete(basedir.toFile(), null, false, false, false)); + verify(log, times(2)).warn(any(CharSequence.class), any(Throwable.class)); + InOrder inOrder = inOrder(log); + ArgumentCaptor<AccessDeniedException> cause1 = ArgumentCaptor.forClass(AccessDeniedException.class); + inOrder.verify(log).warn(eq("Failed to delete " + file), cause1.capture()); + assertEquals(file.toString(), cause1.getValue().getMessage()); + ArgumentCaptor<DirectoryNotEmptyException> cause2 = + ArgumentCaptor.forClass(DirectoryNotEmptyException.class); + inOrder.verify(log).warn(eq("Failed to delete " + basedir), cause2.capture()); + assertEquals(basedir.toString(), cause2.getValue().getMessage()); + } finally { + setPosixFilePermissions(basedir, initialPermissions); + } + } + + @Test + void deleteDoesNotLogAnythingWhenNoPermissionAndWarnDisabled(@TempDir Path tempDir) throws Exception { + assumeTrue(POSIX_COMPLIANT); + when(log.isWarnEnabled()).thenReturn(false); + final Path basedir = createDirectory(tempDir.resolve("target")).toRealPath(); + createFile(basedir.resolve("file")); + final Set<PosixFilePermission> initialPermissions = getPosixFilePermissions(basedir); + final String rwxrwxr_x = PosixFilePermissions.toString(initialPermissions); + final String r_xr_xr_x = rwxrwxr_x.replace('w', '-'); + final Set<PosixFilePermission> permissions = PosixFilePermissions.fromString(r_xr_xr_x); + setPosixFilePermissions(basedir, permissions); + try { + final Cleaner cleaner = new Cleaner(null, log, false, null, null); + assertDoesNotThrow(() -> cleaner.delete(basedir.toFile(), null, false, false, false)); + verify(log, never()).warn(any(CharSequence.class), any(Throwable.class)); + } finally { + setPosixFilePermissions(basedir, initialPermissions); Review Comment: See my other answer. > Provide more accurate reason when can not delete a resource > ----------------------------------------------------------- > > Key: MCLEAN-124 > URL: https://issues.apache.org/jira/browse/MCLEAN-124 > Project: Maven Clean Plugin > Issue Type: Improvement > Reporter: Slawomir Jaranowski > Priority: Major > > Currently we have only in stack trace something like: > {noformat} > Caused by: java.io.IOException: Failed to delete > /Users/runner/work/maven-surefire/maven-surefire/surefire-its/target > at org.apache.maven.plugins.clean.Cleaner.delete (Cleaner.java:300) > at org.apache.maven.plugins.clean.Cleaner.delete (Cleaner.java:250) > at org.apache.maven.plugins.clean.Cleaner.delete (Cleaner.java:124) > {noformat} > It will be usable if we have reported a reason ... why can not be deleted -- This message was sent by Atlassian Jira (v8.20.10#820010)