Introduce TempDirDeletionStrategy

[marcphilipp]

Resolves #4567.

---

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

[strangelookingnerd]

Looks great so far! If I understand correctly cleanup is the when and deletionStrategyis the how temporary files are deleted. Using the synergy between these options should allow to handle most if not all cases we discussed in #4567

Overall I'd hope that there would be more implementations of TempDirDeletionStrategy besides Standard shipped with JUnit.
I don't think it is feasible for each consumer to implement their own strategy. Further one could have already do so via TempDir#factory by implementing a TempDirFactory.

Having the option to ignore errors is still relevant for me and other users seem to agree.
Also the option to delete on VM exit - even though JUnit has no control over that - looks interesting enough for a best-effort option. Maybe it makes more sense to have this as a TempDir#cleanup implementation?

I fully understand that JUnit does not want to provide numerous options to not have the burden of maintaining them, still at least a curated selection should be available.

[marcphilipp]

If I understand correctly cleanup is the when and deletionStrategyis the how temporary files are deleted.

I'd say cleanup is if not when (in a temporal sense).

Overall I'd hope that there would be more implementations of TempDirDeletionStrategy besides Standard shipped with JUnit.
I don't think it is feasible for each consumer to implement their own strategy.

I agree but that can be a second step, maybe even in a later release after we've seen a few implementations.

Further one could have already do so via TempDir#factory by implementing a TempDirFactory.

How do you mean that?

Having the option to ignore errors is still relevant for me and other users seem to agree.

The implementation could be as simple as that:

public class ErrorIgnoringTempDirDeletionStrategy implements TempDirDeletionStrategy {

	@Override
	public Map<Path, Exception> delete(Path tempDir, AnnotatedElementContext elementContext,
			ExtensionContext extensionContext) throws IOException {
		Standard.INSTANCE.delete(tempDir,  elementContext, extensionContext);
		return emptyMap();
	}
}

Whether we want to provide this as a built-in implementation (and thus encouraging users to ignore errors), remains to be discussed.

[strangelookingnerd]

Further one could have already do so via TempDir#factory by implementing a TempDirFactory.

How do you mean that?

I was under the impression that one could already implement custom deletion behavior via TempDirFactory#close - on second look I see that this is only possible in a very limited way as #close is called just after the deletion.

Having the option to ignore errors is still relevant for me and other users seem to agree.

The implementation could be as simple as that:

public class ErrorIgnoringTempDirDeletionStrategy implements TempDirDeletionStrategy {

	@Override
	public Map<Path, Exception> delete(Path tempDir, AnnotatedElementContext elementContext,
			ExtensionContext extensionContext) throws IOException {
		Standard.INSTANCE.delete(tempDir,  elementContext, extensionContext);
		return emptyMap();
	}
}

Whether we want to provide this as a built-in implementation (and thus encouraging users to ignore errors), remains to be discussed.

Just want to emphasize that with the default being Standard (throwing exceptions on errors) you would not really encourage users to ignore errors, but rather providing an option to do so by having sensible alternatives. At least from an argumentative standpoint that's a different thing.

[marcphilipp]

Team decision:

• Create proper return type rather than Map<Path, Exception>
• Add TempDirDeletionStrategy.IgnoreFailures that logs all failures but does not throw an exception

[mpkorstanje]

"within the temporary directory" is doing some heavy lifting here. Would it be appropriate to mention symlinks and junctions here?

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: d9f4867
▶️ Tests: 6316 executed
⚪️ Checks: 15/15 completed

---

Learn more about TestLens at testlens.app.

[mpkorstanje]

LGTM. Pushed a few nitpicks. One question, but not too important.