search

junit-team / junit5

✅ The 5th major version of the programmer-friendly testing framework for Java and the JVM
https://junit.org
Eclipse Public License 2.0
6.44k stars 1.5k forks source link

@TempDir directory cannot be deleted on Windows with Java 11 #2811

Open renatoathaydes opened 2 years ago

renatoathaydes commented 2 years ago

I have a test that has been failing on windows-latest with Java 11 on GitHub Actions CI, but not on Java 17.

This is my test:

    @Test
    void canFetchArtifactToSpecificDir(@TempDir File tempDir) throws Exception {
        var outputDir = new File(tempDir, "outDir");
        final var expectedFileLocation = new File(outputDir, "opentest4j-1.2.0.pom");
        outputDir.deleteOnExit();
        expectedFileLocation.deleteOnExit();

        var result = runWithIntTestRepo("fetch", "org.opentest4j:opentest4j:1.2.0:pom",
                "-d", outputDir.getPath());
        verifySuccessful("jbuild fetch", result);

        assertThat(result.getStdout()).startsWith("JBuild success in ");
        assertThat(expectedFileLocation).isFile();

        try (var stream = new FileInputStream(expectedFileLocation)) {
            var pom = MavenUtils.parsePom(stream);
            assertThat(pom.getArtifact()).isEqualTo(new Artifact("org.opentest4j", "opentest4j", "1.2.0"));
        }
    }

I added those deleteOnExit calls to try to "help" JUnit to get rid of that temp dir, to no avail.

Link to GitHub Actions CI Log Link to JUnit report

The error it shows:

❌ canFetchArtifactToSpecificDir(File)
    java.io.IOException: Failed to delete temp directory C:\Users\RUNNER~1\AppData\Local\Temp\junit13368809472936183425. The following paths could not be deleted (see suppressed exceptions for details): , outDir, outDir\opentest4j-1.2.0.pom

FetchTest > canFetchArtifactToSpecificDir(File) FAILED
    java.io.IOException at TempDirectory.java:262

Notice that this same test passes on Java 11 on MacOS and Linux, and on Java 17 on MacOS, Linux and Windows. It seems to fail very consistently on Java 11/Windows.

Steps to reproduce

It should be very easy to reproduce the problem as long as you can run your tests on the specific Windows and Java versions being used by GitHub Actions CI. The code under test will basically just copy a file from one location to another.

Feel free to fork my project to make it easy to run the tests.

GitHub Environment:

Current runner version: '2.286.0'
Operating System
  Microsoft Windows Server 2019
  10.0.17763
  Datacenter
Virtual Environment
  Environment: windows-2019
  Version: 20220110.1
  Included Software: https://github.com/actions/virtual-environments/blob/win19/20220110.1/images/win/Windows2019-Readme.md
  Image Release: https://github.com/actions/virtual-environments/releases/tag/win19%2F20220110.1

Java configuration:
  Distribution: zulu
  Version: 11.0.13+8
  Path: C:\hostedtoolcache\windows\Java_Zulu_jdk\11.0.13-8\x64

Context

dependencies {
    testImplementation 'org.junit.jupiter:junit-jupiter-api:5.7.0'
    testImplementation 'org.assertj:assertj-core:3.21.0'
    testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.7.0'

    intTestImplementation 'org.junit.jupiter:junit-jupiter-api:5.7.0'
    intTestImplementation 'org.assertj:assertj-core:3.21.0'
    intTestRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.7.0'
}

Link to build file

Gradle downloaded from (see the wrapper properties):

distributionUrl=https\://services.gradle.org/distributions/gradle-7.3.3-bin.zip
sbrannen commented 2 years ago

JUnit Jupiter 5.7.0 is no longer supported, and changes have been made to the @TempDir support since then (for example, #2624).

So, please try out JUnit Jupiter 5.8.2 and report back here if you still encounter issues with @TempDir.

renatoathaydes commented 2 years ago

Sorry, I should've checked my versions (the IDE even warned me about it).

Unfortunately, the error persists:

https://github.com/renatoathaydes/jbuild/runs/4845950867#r2

renatoathaydes commented 2 years ago

That test was failing a lot but not always :( I got rid of this annotation and now the test passes consistently:

https://github.com/renatoathaydes/jbuild/commit/8f18a692c5e64d976c2a4ec882f6128bf8af435e

sormuras commented 2 years ago

I'll try to reproduce it.

kgeilmann commented 2 years ago

This bug hit me yesterday, and I was able to reduce my failing test to a very small example:

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

public class FailedTest {

    @TempDir
    File tmp;

    @Test
    void test() throws IOException {
        final File f = new File(tmp, "input");
        f.mkdir();
        Files.list(f.toPath());
    }
}

Using: JUnit 5.8.2 (but also fails on all versions >= 5.7.1)

Unfortunately the original test only fails on 5.8.x but passes on 5.7.x (I cannot publish that one).

marcphilipp commented 2 years ago

@kgeilmann Files.list() returns a Stream that needs to be closed.

koying commented 2 years ago

I can confirm. Same issue on 5.8.1 but working on 5.7.2

nmck257 commented 2 years ago

EDIT: see https://github.com/junit-team/junit5/issues/2811#issuecomment-1551824624

Adding another diagnostic set, for what I expect is the same root issue:

JUnit version: 5.9.0-RC1 Java version: 11.0.15 OS version: Microsoft Windows [Version 10.0.14393] Stack Trace:

Jul 15, 2022 1:58:43 PM org.junit.jupiter.engine.execution.JupiterEngineExecutionContext close
SEVERE: Caught exception while closing extension context: org.junit.jupiter.engine.descriptor.MethodExtensionContext@3383649e
java.io.IOException: Failed to delete temp directory C:\Users\Me\AppData\Local\Temp\1\junit6470649931803583148. The following paths could not be deleted (see suppressed exceptions for details): , shallowClone, shallowClone\.git, shallowClone\.git\objects, shallowClone\.git\objects\pack
    at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath.createIOExceptionWithAttachedFailures(TempDirectory.java:350)
    at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath.close(TempDirectory.java:251)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.jupiter.engine.execution.ExtensionValuesStore.lambda$closeAllStoredCloseableValues$3(ExtensionValuesStore.java:68)
    at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
    at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
    at java.base/java.util.stream.SortedOps$RefSortingSink.end(SortedOps.java:395)
    at java.base/java.util.stream.Sink$ChainedReference.end(Sink.java:258)
    at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:485)
    at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
    at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
    at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
    at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497)
    at org.junit.jupiter.engine.execution.ExtensionValuesStore.closeAllStoredCloseableValues(ExtensionValuesStore.java:68)
    at org.junit.jupiter.engine.descriptor.AbstractExtensionContext.close(AbstractExtensionContext.java:80)
    at org.junit.jupiter.engine.execution.JupiterEngineExecutionContext.close(JupiterEngineExecutionContext.java:53)
    at org.junit.jupiter.engine.descriptor.JupiterTestDescriptor.cleanUp(JupiterTestDescriptor.java:222)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$cleanUp$1(TestMethodTestDescriptor.java:155)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.cleanUp(TestMethodTestDescriptor.java:155)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.cleanUp(TestMethodTestDescriptor.java:68)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$cleanUp$10(NodeTestTask.java:167)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.cleanUp(NodeTestTask.java:167)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:98)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
    at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
    at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
    at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
    at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
    at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
    at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:147)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:127)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:90)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:55)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:102)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:54)
    at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114)
    at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:86)
    at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:86)
    at org.junit.platform.launcher.core.SessionPerRequestLauncher.execute(SessionPerRequestLauncher.java:53)
    at com.intellij.junit5.JUnit5IdeaTestRunner.startRunnerWithArgs(JUnit5IdeaTestRunner.java:71)
    at com.intellij.rt.junit.IdeaTestRunner$Repeater$1.execute(IdeaTestRunner.java:38)
    at com.intellij.rt.execution.junit.TestsRepeater.repeat(TestsRepeater.java:11)
    at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:35)
    at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:235)
    at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:54)
    Suppressed: java.nio.file.DirectoryNotEmptyException: C:\Users\Me\AppData\Local\Temp\1\junit6470649931803583148
        at java.base/sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:271)
        at java.base/sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:105)
        at java.base/java.nio.file.Files.delete(Files.java:1142)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath$1.deleteAndContinue(TempDirectory.java:293)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath$1.postVisitDirectory(TempDirectory.java:288)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath$1.postVisitDirectory(TempDirectory.java:264)
        at java.base/java.nio.file.Files.walkFileTree(Files.java:2743)
        at java.base/java.nio.file.Files.walkFileTree(Files.java:2797)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath.deleteAllFilesAndDirectories(TempDirectory.java:264)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath.close(TempDirectory.java:249)
        ... 64 more
    Suppressed: java.nio.file.DirectoryNotEmptyException: C:\Users\Me\AppData\Local\Temp\1\junit6470649931803583148\shallowClone
        at java.base/sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:271)
        at java.base/sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:105)
        at java.base/java.nio.file.Files.delete(Files.java:1142)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath$1.deleteAndContinue(TempDirectory.java:293)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath$1.postVisitDirectory(TempDirectory.java:288)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath$1.postVisitDirectory(TempDirectory.java:264)
        at java.base/java.nio.file.Files.walkFileTree(Files.java:2743)
        at java.base/java.nio.file.Files.walkFileTree(Files.java:2797)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath.deleteAllFilesAndDirectories(TempDirectory.java:264)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath.close(TempDirectory.java:249)
        ... 64 more
    Suppressed: java.nio.file.DirectoryNotEmptyException: C:\Users\Me\AppData\Local\Temp\1\junit6470649931803583148\shallowClone\.git
        at java.base/sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:271)
        at java.base/sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:105)
        at java.base/java.nio.file.Files.delete(Files.java:1142)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath$1.deleteAndContinue(TempDirectory.java:293)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath$1.postVisitDirectory(TempDirectory.java:288)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath$1.postVisitDirectory(TempDirectory.java:264)
        at java.base/java.nio.file.Files.walkFileTree(Files.java:2743)
        at java.base/java.nio.file.Files.walkFileTree(Files.java:2797)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath.deleteAllFilesAndDirectories(TempDirectory.java:264)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath.close(TempDirectory.java:249)
        ... 64 more
    Suppressed: java.nio.file.DirectoryNotEmptyException: C:\Users\Me\AppData\Local\Temp\1\junit6470649931803583148\shallowClone\.git\objects
        at java.base/sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:271)
        at java.base/sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:105)
        at java.base/java.nio.file.Files.delete(Files.java:1142)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath$1.deleteAndContinue(TempDirectory.java:293)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath$1.postVisitDirectory(TempDirectory.java:288)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath$1.postVisitDirectory(TempDirectory.java:264)
        at java.base/java.nio.file.Files.walkFileTree(Files.java:2743)
        at java.base/java.nio.file.Files.walkFileTree(Files.java:2797)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath.deleteAllFilesAndDirectories(TempDirectory.java:264)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath.close(TempDirectory.java:249)
        ... 64 more
    Suppressed: java.nio.file.DirectoryNotEmptyException: C:\Users\Me\AppData\Local\Temp\1\junit6470649931803583148\shallowClone\.git\objects\pack
        at java.base/sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:271)
        at java.base/sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:105)
        at java.base/java.nio.file.Files.delete(Files.java:1142)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath$1.deleteAndContinue(TempDirectory.java:293)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath$1.postVisitDirectory(TempDirectory.java:288)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath$1.postVisitDirectory(TempDirectory.java:264)
        at java.base/java.nio.file.Files.walkFileTree(Files.java:2743)
        at java.base/java.nio.file.Files.walkFileTree(Files.java:2797)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath.deleteAllFilesAndDirectories(TempDirectory.java:264)
        at org.junit.jupiter.engine.extension.TempDirectory$CloseablePath.close(TempDirectory.java:249)
        ... 64 more

Test Code: https://github.com/openrewrite/rewrite/blob/ded5f0e65e17bfc338c2e5f498cc6ee6e591605c/rewrite-core/src/test/kotlin/org/openrewrite/marker/GitProvenanceTest.kt#L139-L171

/cc @jkschneider

EDIT: see https://github.com/junit-team/junit5/issues/2811#issuecomment-1551824624

aliiabdii commented 2 years ago

Happened to me as well. JUnit version: 5.8.2 Java Version: 16.0.1 OS: Windows 10

IZUMI-Zu commented 2 years ago

I also met the same problem, is there any progress?

mikethecalamity commented 2 years ago

I'm seeing this in 5.9.1 too. Any updates?

In the meantime I've been doing:

    private static Path tempDir;

    @BeforeAll
    public static void before() throws IOException {
        tempDir = Files.createTempDirectory(null);
    }

    @AfterAll
    public static void after() {
        tempDir.toFile().delete();
    }
marcphilipp commented 1 year ago

@nmck257 The resource returned by Git.open is never closed in your test from https://github.com/junit-team/junit5/issues/2811#issuecomment-1185779074. The latest version on main does close it, though. Is the problem resolved for you now?

nmck257 commented 1 year ago

@nmck257 The resource returned by Git.open is never closed in your test from #2811 (comment). The latest version on main does close it, though. Is the problem resolved for you now?

You are right. It sounds like my previous report was in line with the documented behavior of the feature, given an unclosed resource in the client code before the test method finished.

That said, is there an opportunity to refine the exception thrown in this scenario?

Consider this test case:

@Test
void foo(@TempDir Path tempDir) throws Exception {
    Path filePath = tempDir.resolve("foo.txt");
    Files.writeString(filePath, "foo");
    Files.newInputStream(filePath);
}

It yields:

java.io.IOException: Failed to delete temp directory C:\Users\Me\AppData\Local\Temp\1\junit5699197767000038053. The following paths could not be deleted (see suppressed exceptions for details): [...] Suppressed: java.nio.file.DirectoryNotEmptyException: C:\Users\Me\AppData\Local\Temp\1\junit5699197767000038053

It tells you that the directory is not empty, but does not tell you the logical root cause: that a specific file in that directory could not be deleted during cleanup. That information would likely be more useful for identifying that there is an unclosed resource.

marcphilipp commented 1 year ago

@nmck257 Which version did you run this with? We improved error reporting in #3236 for 5.9.3.

nmck257 commented 1 year ago

@marcphilipp I actually still get this same behavior on 5.9.3.

In the debugger, I can see this line getting called, and trace it all the way down to a native delete method, which completes without exception -- but the file is not deleted at that moment.

https://github.com/junit-team/junit5/blob/main/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/TempDirectory.java#L338

The file is deleted after the process terminates, though. Maybe the windows filesystem is "accepting" the delete command but deferring it until IO handles are released?

Naively, you could add an exists check after the delete invocation in TempDirectory, but this does seem like an odd behavior from Windows which might be worth understanding more precisely before adding conditions.

My environment:

OS Name: Microsoft Windows Server 2016 Standard OS Version: 10.0.14393 N/A Build 14393 java version "17.0.7" 2023-04-18 LTS Java(TM) SE Runtime Environment (build 17.0.7+8-LTS-224) Java HotSpot(TM) 64-Bit Server VM (build 17.0.7+8-LTS-224, mixed mode, sharing)

demery-pivotal commented 1 year ago

The Windows fileapi.h docs say this:

The DeleteFile function marks a file for deletion on close. Therefore, the file deletion does not occur until the last handle to the file is closed.

So if a file is still open during TempDirectory.close(), deleteAndContinue() will succeed during the walk, but the held file will remain in the directory, merely marked for deletion. Then the subsequent delete() of the dir will fail.

So when TempDirectory hits this problem, it's likely that something involved in the test is still holding a handle to some file in the dir.

It would be very useful if TempDirectory would handle this error by finding and reporting the lingering files. That way, users could look for places where they may be holding the file open.

cgicgi commented 7 months ago

Encountered a similar issue. Interestingly, tests were green when setting a break point in the last line of the test (just some random assert). However, Thread.sleep() didn't help - so it was not about timing.

I then tried out to do a System.gc() at the end - et voilá, all tests green.

My fix now is:

    @AfterEach
    @EnabledOnOs(OS.WINDOWS)
    void cleanUp() {
        System.gc();
    }

My naive explanation: Some resource that went already out of scope still has to be finalized, before windows can actually delete the file.

Claudenw commented 6 months ago

I found that @cgicgi's solution worked for me as well. My application is being tested under multiple JVMs under both Windows and Linux; only the Windows executions fail.

System settings for the last run before success with the cgicgi's change are:

Maven dependency management entry

     <dependency>
        <groupId>org.junit</groupId>
        <artifactId>junit-bom</artifactId>
        <version>5.10.2</version>
        <type>pom</type>
        <scope>import</scope>
      </dependency>

Execution environment

Apache Maven 3.9.6 (bc0240f3c744dd6b6ec2920b3cd08dcc295161ae)
Default locale: en_US, platform encoding: Cp1252
OS name: "windows server 2022", version: "10.0", arch: "amd64", family: "windows"

Build failed with

Java version: 1.8.0_412, vendor: Temurin, runtime: C:\hostedtoolcache\windows\Java_Adopt_jdk\8.0.412-8\x64\jre
Java version: 11.0.23, vendor: Eclipse Adoptium, runtime: C:\hostedtoolcache\windows\Java_Adopt_jdk\11.0.23-9\x64
Java version: 12.0.2, vendor: AdoptOpenJDK, runtime: C:\hostedtoolcache\windows\Java_Adopt_jdk\12.0.2-10.1\x64
Java version: 13.0.2, vendor: AdoptOpenJDK, runtime: C:\hostedtoolcache\windows\Java_Adopt_jdk\13.0.2-8.1\x64
Java version: 16.0.2, vendor: Eclipse Foundation, runtime: C:\hostedtoolcache\windows\Java_Adopt_jdk\16.0.2-7\x64
Java version: 21.0.3, vendor: Eclipse Adoptium, runtime: C:\hostedtoolcache\windows\Java_Adopt_jdk\21.0.3-9.0.LTS\x64

Build passed with

Java version: 14.0.2, vendor: AdoptOpenJDK, runtime: C:\hostedtoolcache\windows\Java_Adopt_jdk\14.0.2-12\x64 
Java version: 15.0.2, vendor: AdoptOpenJDK, runtime: C:\hostedtoolcache\windows\Java_Adopt_jdk\15.0.2-7\x64

I think the passing cases were an effect of noisy neighbour conditions where the file closure managed to occur before the clean-up of the @TempDir

marcphilipp commented 6 months ago

Thanks for posting this workaround! I'm not convinced we should call System.gc() during temp dir cleanup on Windows in general, though. It might have surprising side effects for some users. Thoughts?

sbrannen commented 6 months ago

I'm not convinced we should call System.gc() during temp dir cleanup on Windows in general, though. It might have surprising side effects for some users. Thoughts?

Indeed, I'm also not convinced that we should do that by default.

Though, we could consider making it opt-in via an annotation attribute, global configuration parameter, or similar.

cgicgi commented 6 months ago

Thank you for discussing this issue so thoroughly. As it is far from sure that the issue affects every Tempdir Junit test on every Windows platform, I'd vote for a) a friendly message in the exception b) opt-in via annotation as @sbrannen suggested

Claudenw commented 6 months ago

I would also suggest that a ticket be opened with Microsoft (or whomever is providing their JDK) with an easily reproducible example.

On Fri, May 17, 2024 at 8:06 AM cgicgi @.***> wrote:

Thank you for discussing this issue so thoroughly. As it is far from sure that the issue affects every Tempdir Junit test on every Windows platform, I'd vote for a) a friendly message in the exception b) opt-in via annotation as @sbrannen https://github.com/sbrannen suggested

— Reply to this email directly, view it on GitHub https://github.com/junit-team/junit5/issues/2811#issuecomment-2116788752, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASTVHT6JVEL363PZYKJ66LZCWM6XAVCNFSM5MDG4SQ2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMJRGY3TQOBXGUZA . You are receiving this because you commented.Message ID: @.***>

-- LinkedIn: http://www.linkedin.com/in/claudewarren

cgicgi commented 6 months ago

Here is an example that demonstrates the issue and should be reproducible

OS: Windows 11

Java: java version "17.0.1" 2021-10-19 LTS Java(TM) SE Runtime Environment (build 17.0.1+12-LTS-39) Java HotSpot(TM) 64-Bit Server VM (build 17.0.1+12-LTS-39, mixed mode, sharing)

junit 5.10.1

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

import java.io.FileOutputStream;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;

public class JUnitTempdirOnWindowsTest {
    @Test
    void failsOnWindowsWithoutGc(@TempDir Path tempDir) throws IOException {
        new FileOutputStream(Files.createDirectories(tempDir).resolve("foo.txt").toFile());
    }

    @Test
    void succeedsOnWindowsWithGc(@TempDir Path tempDir) throws IOException {
        new FileOutputStream(Files.createDirectories(tempDir).resolve("foo.txt").toFile());
        System.gc();
    }
}
sormuras commented 6 months ago

Yes, it is reproducible. Yes, it is also a programmer's error to not close a resource after using it.

    @Test
    void fixedOnWindowsWithoutGc(@TempDir Path tempDir) throws IOException {
        var file = Files.createDirectories(tempDir).resolve("foo.txt").toFile();
        try (var stream = new FileOutputStream(file)) {
            stream.write("123".getBytes());
        }
    }
cgicgi commented 6 months ago

@sormuras that's exactly the point here: I included some (simplified) production code in the unit test, which normally would be somewhere outside the junit test. The test should reliably identify the programmer's error (in this case: not closing the resource) independant from OS. For the simplified case I presented here, Linux would not detect the issue, while Windows does.

Claudenw commented 6 months ago

I think the issue I had was because the file was originally created in a static block and even though it was closed there was an error. I will see if I can reproduce it in a small test case.

On Tue, May 21, 2024 at 3:29 PM cgicgi @.***> wrote:

@sormuras https://github.com/sormuras that's exactly the point here: I included some (simplified) production code in the unit test, which normally would be somewhere outside the junit test. The test should reliably identify the programmer's error (in this case: not closing the resource) independant from OS. For the simplified case I presented here, Linux would not detect the issue, while Windows does.

— Reply to this email directly, view it on GitHub https://github.com/junit-team/junit5/issues/2811#issuecomment-2122642152, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASTVHT2ID36S2OKADMXGNTZDND3TAVCNFSM5MDG4SQ2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMJSGI3DIMRRGUZA . You are receiving this because you commented.Message ID: @.***>

-- LinkedIn: http://www.linkedin.com/in/claudewarren

sormuras commented 6 months ago

I agree with "not closing the resource" should be "reliably identified as a programmer's error" - but wouldn't expect a (JUnit-based) test case to report that nor should the operating system play a role here. Wouldn't something like javac -Xlint:try do a better job to fail the compilation of the production in the first place?

In other words, this issue

is not about detecting programming errors - but "only" about deleting a JUnit-created temporary directories on Windows.

I think the issue I had was because the file was originally created in a static block and even though it was closed there was an error.

A static block in the test class?

Claudenw commented 6 months ago

Yes, a static method in support of the MethodSource annotation, but looking back at the code now I see that I moved that to a standalone test without the static.

On Tue, May 21, 2024 at 4:02 PM Christian Stein @.***> wrote:

I agree with "not closing the resource" should be "reliably identified as a programmer's error" - but wouldn't expect a (JUnit-based) test case to report that nor should the operating system play a role here. Wouldn't something like javac -Xlint:try do a better job to fail the compilation of the production in the first place?

In other words, this issue

is not about detecting programming errors - but "only" about deleting a JUnit-created temporary directories on Windows.

I think the issue I had was because the file was originally created in a static block and even though it was closed there was an error.

A static block in the test class?

— Reply to this email directly, view it on GitHub https://github.com/junit-team/junit5/issues/2811#issuecomment-2122711934, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASTVHX5TGXHX42RAU4T6XTZDNHV5AVCNFSM5MDG4SQ2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMJSGI3TCMJZGM2A . You are receiving this because you commented.Message ID: @.***>

-- LinkedIn: http://www.linkedin.com/in/claudewarren