Feature/temp dir cleanup #2729

ghost · 2021-09-26T13:34:11Z

Overview

Addressing issue #2159 .

This PR adds a TempDirStrategy annotation for managing TempDirs. The annotation has a single CleanupMode enum field that can have a value of ALWAYS or NEVER. If the field is NEVER, then a test class's TempDirs are not cleaned up after the test completes. The annotation also works on nested classes, and a config parameter for the default cleanup mode has been added. Unit tests, demo and documentation included.

The issue discussion included the idea of an 'on_success' mode, but I have been unable to work out how to achieve this. Happy to discuss ideas on how this might be done.

I hereby agree to the terms of the JUnit Contributor License Agreement.

Definition of Done

There are no TODOs left in the code
Method preconditions are checked and documented in the method's Javadoc
Coding conventions (e.g. for logging) have been followed
Change is covered by automated tests including corner cases, errors, and exception handling
Public API has Javadoc and @API annotations
Change is documented in the User Guide and Release Notes
All continuous integration builds pass

…f use

…, as this could lead to indeterminate cleanup mode

…it works just fine

codecov · 2021-09-26T14:48:42Z

Codecov Report

Merging #2729 (85493ee) into main (c997035) will increase coverage by 91.07%.
The diff coverage is 90.69%.

@@             Coverage Diff             @@
##             main    #2729       +/-   ##
===========================================
+ Coverage        0   91.07%   +91.07%     
- Complexity      0     4476     +4476     
===========================================
  Files           0      385      +385     
  Lines           0    10846    +10846     
  Branches        0      845      +845     
===========================================
+ Hits            0     9878     +9878     
- Misses          0      744      +744     
- Partials        0      224      +224

Impacted Files	Coverage Δ
.../junit/jupiter/engine/extension/TempDirectory.java	`72.09% <87.09%> (ø)`
...ain/java/org/junit/jupiter/api/io/CleanupMode.java	`100.00% <100.00%> (ø)`
...ter/engine/config/CachingJupiterConfiguration.java	`100.00% <100.00%> (ø)`
...ter/engine/config/DefaultJupiterConfiguration.java	`100.00% <100.00%> (ø)`
...ter/engine/extension/MutableExtensionRegistry.java	`100.00% <100.00%> (ø)`
...r/engine/discovery/DefaultClassOrdererContext.java	`100.00% <0.00%> (ø)`
.../platform/engine/support/descriptor/UriSource.java	`81.81% <0.00%> (ø)`
...r/engine/execution/InvocationInterceptorChain.java	`90.38% <0.00%> (ø)`
...org/junit/platform/commons/util/Preconditions.java	`91.66% <0.00%> (ø)`
...overy/DefensiveAllDefaultPossibilitiesBuilder.java	`100.00% <0.00%> (ø)`
... and 379 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c997035...85493ee. Read the comment docs.

marcphilipp · 2021-10-31T11:47:15Z

@hglasgow Sorry for the wait!

The issue discussion included the idea of an 'on_success' mode, but I have been unable to work out how to achieve this. Happy to discuss ideas on how this might be done.

I think we should be able to pass the ExtensionContext to CloseablePath when it's created and check if ExtensionContext.getExecutionException is empty or contains an instance of TestAbortedException.

…ngineTests.

ghost · 2021-11-19T23:06:34Z

All done, ready for review.

marcphilipp

Thanks! I found one additional minor thing.

marcphilipp · 2021-11-21T15:59:43Z

...piter-engine/src/test/java/org/junit/jupiter/engine/extension/TempDirectoryCleanupTests.java

+		@AfterAll
+		static void afterAll() {
+			if (defaultParameterDir != null && defaultParameterDir.exists()) {
+				defaultParameterDir.delete();


File.delete returns a false if the file wasn't deleted for some reason. Instead, I think we should use Files.deleteIfExists (and Path in the test cases) which only returns false if the file didn't exists and throws an exception if there was a problem deleting it.

Okay, that's done.

Ready for review

...piter-engine/src/test/java/org/junit/jupiter/engine/extension/TempDirectoryCleanupTests.java

documentation/src/docs/asciidoc/release-notes/release-notes-5.9.0-M1.adoc

documentation/src/docs/asciidoc/user-guide/writing-tests.adoc

...piter-engine/src/test/java/org/junit/jupiter/engine/extension/CloseablePathCleanupTests.java

…9.0-M1.adoc Co-authored-by: Marc Philipp <marc@gradle.com>

Co-authored-by: Marc Philipp <marc@gradle.com>

…tension/TempDirectoryCleanupTests.java Co-authored-by: Marc Philipp <marc@gradle.com>

marcphilipp

Thanks! 👍

@SInCE

- Use correct versions in @SInCE tags and @API declarations - Add missing @SInCE tags and @API declarations - Add missing Javadoc - Fix configuration parameter name for default cleanup mode - Clarify meaning of DEFAULT cleanup mode - Improve Javadoc - Polish tests See #2729

See #2729

Prior to this commit, a custom default cleanup mode was ignored when determining the cleanup mode for a parameter annotated with @tempdir. See #2729

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>

@SInCE

- Use correct versions in @SInCE tags and @API declarations - Add missing @SInCE tags and @API declarations - Add missing Javadoc - Fix configuration parameter name for default cleanup mode - Clarify meaning of DEFAULT cleanup mode - Improve Javadoc - Polish tests See junit-team#2729

See junit-team#2729

Prior to this commit, a custom default cleanup mode was ignored when determining the cleanup mode for a parameter annotated with @tempdir. See junit-team#2729

Harry Glasgow added 26 commits September 25, 2021 10:08

Add TempDirCleanupStrategy, and plumb into config, and tests.

71d885e

Fix style problem

b90db93

Fix style problem

dca9fca

Set a default value for the cleanup mode of ALWAYS

900ec13

Rename to TempDirStrategy to provide for more potential flexibility o…

5c6028c

…f use

Spotless fix of * imports

d295303

Spotless fix of indentation

2242f47

Spotless fix of indentation

3019634

Spotless fix of indentation

313fd6b

Spotless fix of indentation

e596f0b

Get Always and Never tests passing

118ffe7

Spotless fix of indentation

ed98bc1

Spotless fix of indentation

476ba11

Move to version 5.9

099aed9

Prevent TempDirStrategy annotations from being added to inner classes…

ede3aee

…, as this could lead to indeterminate cleanup mode

Allow TempDirStrategy annotations from being added to inner classes, …

4934c90

…it works just fine

Remove ON_SUCCESS cleanup mode, and add demo test for documentation

5209310

Add lifecycle tests for temp directory cleanup in different modes

73821e6

Add lifecycle tests for temp directory cleanup of nested tests

bbfdf18

Add documentation for TempDirStrategy

87e2be1

Use static imports of CleanupMode

2c63046

Apply spotless corrections

29e4d0e

Documentation of the cleanup.mode.default configuration parameter

a15cae6

Documentation; configuration parameters link

eafc5c1

Fix @API problem

1fc600c

Fix @API problem

4dcaef5

Harry Glasgow added 2 commits November 7, 2021 20:20

Move to CleanupMode parameter in TempDir.

5b09b66

Typo

90d2a75

Harry Glasgow added 3 commits November 18, 2021 20:24

Rework TempDirectoryParameterResolverTests using AbstractJupiterTestE…

33cd7e6

…ngineTests.

Retrigger build.

56118f0

Retrigger build.

c32ccba

ghost marked this pull request as ready for review November 19, 2021 23:06

marcphilipp requested changes Nov 21, 2021

View reviewed changes

Use nio for safer deletes.

a18d400

marcphilipp requested changes Nov 28, 2021

View reviewed changes

Harry Glasgow and others added 8 commits November 29, 2021 19:09

Update documentation/src/docs/asciidoc/release-notes/release-notes-5.…

05b5f17

…9.0-M1.adoc Co-authored-by: Marc Philipp <marc@gradle.com>

Update documentation/src/docs/asciidoc/user-guide/writing-tests.adoc

1311ce0

Co-authored-by: Marc Philipp <marc@gradle.com>

Update documentation/src/docs/asciidoc/user-guide/writing-tests.adoc

9894363

Co-authored-by: Marc Philipp <marc@gradle.com>

Update junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/ex…

7b6a36d

…tension/TempDirectoryCleanupTests.java Co-authored-by: Marc Philipp <marc@gradle.com>

Simplify parameter directory deletion.

32f9094

Simplify closeable path deletion.

682463e

Clean up the temp directory after use.

55aabfe

Rerun build.

85493ee

marcphilipp approved these changes Nov 29, 2021

View reviewed changes

marcphilipp merged commit 3e06d8f into junit-team:main Nov 29, 2021

marcphilipp linked an issue Nov 29, 2021 that may be closed by this pull request

Support disabling cleanup for @TempDir #2159

Closed

sbrannen added a commit that referenced this pull request Dec 31, 2021

Polish TempDir cleanup support

01353cd

See #2729

sbrannen added a commit that referenced this pull request Dec 31, 2021

Fix bug with @tempdir custom default cleanup mode

498cf7f

Prior to this commit, a custom default cleanup mode was ignored when determining the cleanup mode for a parameter annotated with @tempdir. See #2729

sbrannen added type: enhancement theme: extensions component: Jupiter labels May 15, 2022

sbrannen assigned marcphilipp May 15, 2022

runningcode pushed a commit to runningcode/junit5 that referenced this pull request Feb 15, 2023

Polish TempDir cleanup support

ce51f20

See junit-team#2729

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/temp dir cleanup #2729

Feature/temp dir cleanup #2729

ghost commented Sep 26, 2021 •

edited by marcphilipp

codecov bot commented Sep 26, 2021 •

edited

marcphilipp commented Oct 31, 2021

ghost commented Nov 19, 2021

marcphilipp left a comment

marcphilipp Nov 21, 2021

ghost Nov 22, 2021

ghost Nov 28, 2021

marcphilipp left a comment

Feature/temp dir cleanup #2729

Feature/temp dir cleanup #2729

Conversation

ghost commented Sep 26, 2021 • edited by marcphilipp

Overview

Definition of Done

codecov bot commented Sep 26, 2021 • edited

Codecov Report

marcphilipp commented Oct 31, 2021

ghost commented Nov 19, 2021

marcphilipp left a comment

Choose a reason for hiding this comment

marcphilipp Nov 21, 2021

Choose a reason for hiding this comment

ghost Nov 22, 2021

Choose a reason for hiding this comment

ghost Nov 28, 2021

Choose a reason for hiding this comment

marcphilipp left a comment

Choose a reason for hiding this comment

ghost commented Sep 26, 2021 •

edited by marcphilipp

codecov bot commented Sep 26, 2021 •

edited