@TempDir cleanup does not handle Windows junctions correctly

[quintesse]

Using junit 5.11.4

It's a very specific situation but the cleanup of a directory created with @TempDir will fail on Windows if it contains junctions that point to a directory that no longer exists. The following code reproduces the issue:

@Test
void testTempDirWithJunction(@TempDir Path dir) throws IOException, InterruptedException {
    Path test = dir.resolve("test");
    Files.createDirectory(test);
    Path link = dir.resolve("link");
    // This creates a Windows "junction" which you can't do with Java code
    Runtime.getRuntime().exec(String.format("cmd.exe /c mklink /j %s %s", link.toString(), test.toString())).waitFor();
    // The error might also occur without the source folder being deleted
    // but it depends on the order that the TempDir cleanup does its work,
    // so the following line forces the error to occur always
    Files.delete(test);
}

Why are junctions used instead of plain symbolic (or hard) links? Well on Windows you need special privileges to create links which not everybody has, while junctions are allowed by default. (Btw, junctions can only be used for directories, not files!)

Perhaps the easiest way to solve this would be to add an ignoreCleanupErrors options to @TempDir, which would continue trying to delete files and not throw any errors, because not being able to fully delete a temp folder might not be seen as a test failure in many cases. (But of course junit should also be able to handle this situation properly and delete broken junctions).

[quintesse]

Some extra info:

• Files.isSymbolicLink(link) will return false (junctions are not symlinks)
• LinkOption.NOFOLLOW_LINKS will work for junctions

So you can do the following to detect a broken junction (or any broken link):

• !Files.exists(path) && Files.exists(path, LinkOption.NOFOLLOW_LINKS) will return true for broken links

If you want to test if path points to a link you can also do something like the following:

• path.toRealPath().equals(path.toRealPath(LinkOption.NOFOLLOW_LINKS)) will return false in the case of (non-broken) links

[quintesse]

PS: if we add an extra Files.delete(link); to the test the error goes away. This means that the fileOperation.delete(path) operation must be does something more than simply Files.delete() but I wasn't able to encounter the implementing class so I can't confirm if that's true or if there's something else going on.

[marcphilipp]

This means that the fileOperation.delete(path) operation must be does something more than simply Files.delete() but I wasn't able to encounter the implementing class so I can't confirm if that's true or if there's something else going on.

It's doing exactly that: 🤔

junit5/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/TempDirectory.java

Line 485 in 6430ba4

FileOperations DEFAULT = Files::delete;

[marcphilipp]

I submitted a draft PR (#4301) with your test case to see if the issue can be reproduced on the Windows runner of GitHub Actions because I don't currently have a Windows machine available.

[marcphilipp]

I've added some logging and the problem seems to be that Java fails to create a DirectoryStream for the function and throws a NoSuchFileException instead:

10:17:40.799 [main] TRACE org.junit.jupiter.engine.extension.TempDirectory - preVisitDirectory: C:\Users\RUNNER~1\AppData\Local\Temp\junit-5649267812304671570	
10:17:40.801 [main] TRACE org.junit.jupiter.engine.extension.TempDirectory - visitFileFailed: C:\Users\RUNNER~1\AppData\Local\Temp\junit-5649267812304671570\link	
java.nio.file.NoSuchFileException: C:\Users\RUNNER~1\AppData\Local\Temp\junit-5649267812304671570\link	
	at java.base/sun.nio.fs.WindowsException.translateToIOException(WindowsException.java:85) ~[?:?]	
	at java.base/sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:103) ~[?:?]	
	at java.base/sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:108) ~[?:?]	
	at java.base/sun.nio.fs.WindowsDirectoryStream.<init>(WindowsDirectoryStream.java:86) ~[?:?]	
	at java.base/sun.nio.fs.WindowsFileSystemProvider.newDirectoryStream(WindowsFileSystemProvider.java:541) ~[?:?]	
	at java.base/java.nio.file.Files.newDirectoryStream(Files.java:482) ~[?:?]	
	at java.base/java.nio.file.FileTreeWalker.visit(FileTreeWalker.java:301) ~[?:?]

which is then seen as "nothing to do" and ignored here:

junit5/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/TempDirectory.java

Lines 409 to 418 in f9b1ff6

	@Override
	public FileVisitResult visitFileFailed(Path file, IOException exc) {
	LOGGER.trace(exc, () -> "visitFileFailed: " + file);
	if (exc instanceof NoSuchFileException) {
	return CONTINUE;
	}
	// IOException includes `AccessDeniedException` thrown by non-readable or non-executable flags
	resetPermissionsAndTryToDeleteAgain(file, exc);
	return CONTINUE;
	}

[quintesse]

Awesome!