Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support disabling cleanup for @TempDir #2159

Closed
solter opened this issue Jan 20, 2020 · 15 comments · Fixed by #2729
Closed

Support disabling cleanup for @TempDir #2159

solter opened this issue Jan 20, 2020 · 15 comments · Fixed by #2729

Comments

@solter
Copy link

solter commented Jan 20, 2020

When initially developing tests against legacy code that utilizes the filesystem extensively, I often find myself creating using @TempDir, which is fantastic. But oftentimes when debugging and/or initially writing the test I need to capture the outputs to get a "golden file" that I can regression test against on future runs. As such, currently I just use a non-TempDir folder to capture them, then need to update my test using TempDir later.

It would be super helpful if instead the TempDir annotation provided an option so that I could disable cleanup for a couple of runs then re-enable cleanup later in the development lifecycle. I'm envisioning something like the following

@TempDir(cleanup=false)
private Path outputFolder;
@juliette-derancourt
Copy link
Member

Tentatively slated for 5.7 M1 solely for the purpose of team discussion

@leonard84
Copy link
Collaborator

I think a system property might be useful as well.

@marcphilipp
Copy link
Member

marcphilipp commented Feb 13, 2020

Team decision: Let's add a "strategy-like" config parameter (similar to display name generator or method orderer) to configure in which cases temp dirs should be cleaned up with initial implementations of "always", "on_success", and "never". This needs to be spec'ed out a bit more before work can actually be started.

@marcphilipp
Copy link
Member

In addition, we could add a new class-level @TempDirCleanupStrategy or @TempDirCleanupMode annotation to configure it per class.

@stale
Copy link

stale bot commented May 29, 2021

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. Thank you for your contribution.

@stale stale bot added the status: stale label May 29, 2021
@marcphilipp marcphilipp removed this from the 5.8 Backlog milestone Jun 19, 2021
@ghost ghost mentioned this issue Sep 26, 2021
7 tasks
@marcphilipp
Copy link
Member

@hglasgow Thanks for the PR! I think we should first discuss the new APIs here to avoid back and forth on the PR.

I think we should do the following:

  • Introduce a new enum called CleanupMode in the org.junit.jupiter.api.io package with ALWAYS, ON_SUCCESS, and NEVER constants.
  • Add a cleanup attribute to the @TempDir annotation with ALWAYS as its default.
  • Add a junit.jupiter.tempdir.cleanup.mode.default configuration parameter

I'm not convinced we need a @TempDirCleanupStrategy or @TempDirCleanupMode annotation that I proposed above.

@ghost
Copy link

ghost commented Nov 7, 2021

That sounds good. Then you could have a tag like @tempdir(cleanup = CleanupMode.ON_SUCCESS). That way you can specify the cleanup mode per directory.

@marcphilipp
Copy link
Member

Yep, I think that would be the most discoverable API. Do you have time change your PR accordingly?

@ghost
Copy link

ghost commented Nov 7, 2021

Yep, I'll work on it this week.

@delanym
Copy link

delanym commented Jun 10, 2022

Do I need to modify all my @tempdir annotations or is there a global property I can set via surefire-plugin to set the default cleanup mode?

@sbrannen
Copy link
Member

@delanym, you can set the junit.jupiter.tempdir.cleanup.mode.default property to configure a global default mode.

The Maven Surefire documentation has examples for setting such a JUnit Platform configuration parameter

@delanym
Copy link

delanym commented Jun 13, 2022

@sbrannen if a tempdir is created and left on disk, it would be nice to have its path in the build log.

I set my test to fail and added surefire config

+            <properties>
+              <configurationParameters>junit.jupiter.tempdir.cleanup.mode.default=NEVER</configurationParameters>
+            </properties>

The directories remain after the tests. But if I set it to ON_SUCCESS or on_success the directory is always removed.

@sbrannen
Copy link
Member

I set my test to fail and added surefire config

+            <properties>
+              <configurationParameters>junit.jupiter.tempdir.cleanup.mode.default=NEVER</configurationParameters>
+            </properties>

The directories remain after the tests. But if I set it to ON_SUCCESS or on_success the directory is always removed.

That sounds like the expected behavior (for successful tests).

Or are you stating something else?

@sbrannen if a tempdir is created and left on disk, it would be nice to have its path in the build log.

That should already be the case:

if (cleanupMode == NEVER
|| (cleanupMode == ON_SUCCESS && executionContext.getExecutionException().isPresent())) {
logger.info(() -> "Skipping cleanup of temp dir " + dir + " due to cleanup mode configuration.");
return;
}

Have you set the appropriate log category to INFO?

@delanym
Copy link

delanym commented Jun 14, 2022

@sbrannen I configure junit tempdir default as on_success, set a test to fail, and the result is that no junit directories ( e.g. junit10225410630232112052) remain in my temp folder after the tests.
When I then change junit tempdir default to never, 4 directories remain after the tests (for the 4 tests).
When I change to always, no directories remain.
Conclusion: on_success is not working.
I tried setting all the tests to fail. It makes no difference.

This is my surefire 3.0.0-M7 config

            <properties>
              <configurationParameters>junit.jupiter.tempdir.cleanup.mode.default=on_success
                org.apache.logging.log4j.simplelog.StatusLogger.level=info</configurationParameters>
            </properties>

Whether its always/never/on_success, I don't see any mention of the output directory in Maven's log, or in the <project.build.directory>/surefire-reports/* files.

@sbrannen
Copy link
Member

Thanks for the feedback, @delanym.

Conclusion: on_success is not working. I tried setting all the tests to fail. It makes no difference.

Since this issue is closed, please open a new issue to report that bug.

            <properties>
              <configurationParameters>junit.jupiter.tempdir.cleanup.mode.default=on_success
                org.apache.logging.log4j.simplelog.StatusLogger.level=info</configurationParameters>
            </properties>

Whether its always/never/on_success, I don't see any mention of the output directory in Maven's log, or in the <project.build.directory>/surefire-reports/* files.

With regard to the logging, you'll need to find out how to set the org.junit.jupiter.engine.extension (or org.junit.jupiter.engine.extension.TempDirectory.CloseablePath) category to INFO. Please refer to the documentation for your logging framework for details.

runningcode pushed a commit to runningcode/junit5 that referenced this issue Feb 15, 2023
This PR adds a cleanup mode (`ALWAYS` (default), `ON_SUCCESS`, or
`NEVER`) parameter to `@TempDir` that allows to configured when the
temp directory should be deleted. The default can be configured by
setting a configuration parameter.

Resolves junit-team#2159.

Co-authored-by: Marc Philipp <mail@marcphilipp.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants