Create new temporary directory/resource extension

[nipafx]

After Marc Philipp implemented the @TempDir extension in Pioneer, it was later also added to Jupiter. It didn't (and as of September 2020 doesn't) support custom file systems, but other than that is more feature-rich than Pioneer's variant. This creates an unpleasant situation where there are two extensions that solve the same problem with the same approach, but slightly different feature sets (and an identical name to boot).

That said, there are a lot of feature requests for Jupiter's variant (⚠️ some of these are already implemented! ⚠️):

• Multiple declarations of @TempDir in same parameter list or class declaration should fail
• Support @TempDir on constructor parameters
• Introduce cleanup mode in @TempDir
• Multiple @TempDirs should result in multiple directories
• Provide support for setting a custom parent directory for @TempDir
• Support disabling cleanup for @TempDir
• Support custom file system for @TempDir
• Make @TempDir restore mint permissions before closing context to avoid AccessDeniedException

After evaluating the situation in #277, we decided to remove the extension before the 1.0 release with the goal to look for a better solution to the problem in the future (i.e. this issue). So the task is to come up with a temporary directory (or, more generally temporary resource) extension that covers more than what Jupiter does, possibly with an approach that makes different trade-offs (so users of Jupiter and Pioneer have more choices to solve their specific problem).

One interesting proof of concept to look at is @sormuras': 1, 2

[nipafx]

For starters, we're just looking for a design proposal or proof of concept that shows the outline of the extension and how it could provide the features requested above.

Please don't write all the code for all the features, yet! We definitely want to give feedback early, i.e. before descending into the seven circles of filesystem hell. 😈

[jbduncan]

I just wanted to say that this issue is of interest to me, and that I've just signed up for hack.commit.push so that I can dedicate some time to work on it.

I'll see if I can find the time beforehand to submit a proposal, but worst case scenario I'll draft it up on the day.

Is this the right place for me to discuss proposals, or would Discord or some place else be better? :)

[Michael1993]

Feel free to discuss it here, on Discord, on Twitter, via smoke signals, over telegram or carrier pigeon (or any other way). We'd love to hear from you!

Personally, I'd use Discord, for quick/casual feedback and the GitHub issue for a more formal proposal (because the latter is a bit more permanent) but any way you want is fine.

[nipafx]

but any way you want is fine.

I want to amend this: In the end, a summary of the conversation and the proposal needs to land here for future reference. So while preliminary conversations are a better fit for Discord et al, showing the actual proposal here would be easiest.

[keturn]

I had a want for this the other day: junit-team/junit5#1786 : Support @tempdir tags in extensions

[jbduncan]

Just wanted to follow up and say that I'm still interested in this, I've just been feeling tired from work lately, so I've dedicated some time in my calendar to revisit this a week before hack.commit.push to get an initial proposal out there.

[jbduncan]

When I do get around to this and submit my proposal, I should ensure that my eventual implementation satisfies the following JUnit 5 bug report too: junit-team/junit5#2609.

@nipafx Would you mind amending the OP to link to this issue as well? :)

[nipafx]

@jbduncan Done.

[jbduncan]

I now have an initial proposal for what an API for solving this overall issue could look like. (See below.)

Many thanks to @sormuras for the inspiration with his proof-of-concept!

(I've not yet taken all the issues linked in the OP into account, so any feedback on anything that I've missed would be most appreciated!

I'm also open to feedback on anything that's unclear or too complicated.)

issue-348-proposal.md

[jbduncan]

My most up-to-date work can be found at https://github.com/jbduncan/junit-pioneer/tree/issue/348-temp-dir-extension.

[sormuras]

For the record: using my own dog-food "internally" here https://github.com/sormuras/bach/tree/main/test.base/test/java/test/base/resource

[jbduncan]

...and I've just seen I forgot to explain something called @Singleton in my proposal, so I'll see if I can submit an amendment later this week.

[jbduncan]

Here's the new version. issue-348-proposal.md

Or if you prefer, here's the associated Git diff and the above new version as a branch on Git.

[jbduncan]

I've decided to not include the before-mentioned @Singleton annotation, because I'm not sure if I'll have time to implement it on top of the rest of the proposal.

[jbduncan]

In response to feedback during hack.commit.push, here's my latest proposal!

Git diff: jbduncan@22ed80c
Entire Git branch: main...jbduncan:issue/348-temp-dir-extension

[nipafx]

I'm just gonna publish your proposal here. Verbatim in the next message.

[nipafx]

Extension ResourceManager looks for fields and parameters annotated with @New @Share and @Shared.

@New is a field/parameter-level annotation that accepts a Class that implements ResourceSupplier<T>. For example, @New(TemporaryDirectory.class)

@Share is a class-level annotation that accepts a Class that implements ResourceSupplier<T>, as well as a String
name. For example, @Share(name = "aClassWideTempDirectory", value = TemporaryDirectory.class)

@Shared is a field/paramter-level annotation that accepts a String value. For example, @Shared("aClassWideTempDirectory")

When a parameter in a test method is annotated with @New, it checks if the given class implements ResourceSupplier and has a no-args constructor. If so, it creates a new instance of the ResourceSupplier implementation and asks it for a T with ResourceSupplier::get. If @New is used on a test method parameter, the associated ResourceSupplier (and thus the contained T) only lasts as long as the test does and is "torn down" straight afterwards. If @New is used on a test class field, the associated ResourceSupplier is instantiated anew before and "torn down" immediately after each and every test method in the test class.

When a test class is annotated with @Share, it checks if the given class implements ResourceSupplier and has a no-args constructor. If so, the ResourceSupplier instance will be instantiated and saved until all the test methods in that class have been run, at which point it will be "torn down". The ResourceSupplier (and thus the contained T) is saved with a key, where this key comes from @Share's String name field. This allows the T to be reused across tests in the test class.

When a field or test method parameter is annotated with @Shared, it finds a @Share annotation on the same test class whose String name matches the @Shared's String value. If such a @Share can be found, then the field or parameter annotated with @Shared is populated with the T from the ResourceSupplier from the @Share.

When a @New or @Share is "torn down", it calls the ResourceSupplier implementation's close method on the field/parameter's instance of T.

For example:

@ExtendWith(ResourceManager.class)
class FooTests {

  // Before each test method, this annotation creates a new
  // instance of `Path` that points to a new subdirectory of
  // the machine's temporary directory.
  //
  // After each test method, it is torn down, ready to be recreated
  // for the next test method.
  @New(TemporaryDirectory.class)
  Path firstDirectory;
  
  @Test
  void testFoo1(
      // Creates a `Path` pointing to another
      // temporary directory.
      //
      // It is created before this test method starts,
      // and is closed as soon as this method is finished.
      @New(TemporaryDirectory.class) Path secondDirectory,

      // @Dir is a shortcut for @New(TempDirectory.class).
      @Dir Path thirdDirectory,

      // Thus fourthDirectory is different to thirdDirectory.
      @Dir Path fourthDirectory, 
      
      // Creates a new resource for the duration of this
      // test method, which is provided by a new, user-defined
      // `InMemoryDirectory` resource supplier.
      // (See InMemoryDirectory class below.)
      @New(InMemoryDirectory.class) Path inMemoryDirectory) {
    // ...
  }
}

@ExtendWith(ResourceManager.class)
@Share(name = "aClassWideTempDirectory", value = TemporaryDirectory.class)
class BarTests {
  @Test
  void testBar(
      // The key "aClassWideTempDirectory" below references
      // the @Share annotation above.
      // Thus the temporary directory here...
      @Shared("aClassWideTempDirectory")
      Path aClassWideTempDirectory) {
    Files.writeString(aClassWideTempDirectory.resolve("bar.txt"), "bar1");
  }
  
  @Test
  void testBar2(
      // ...is the same temporary directory as here!
      @Shared("aClassWideTempDirectory")
      Path aClassWideTempDirectory) {
    Files.writeString(aClassWideTempDirectory.resolve("bar.txt"), "bar2");
  }
}

// At the end, "aClassWideTempDirectory" will have a
// "bar.txt" file with lines "bar1" and "bar2".

// This ResourceSupplier comes out of the box with this extension.
public final class TemporaryDirectory implements ResourceSupplier<Path> {
  
  private final Path path;
  
  public TemporaryDirectory() {
    // creates a new subdirectory on the machine-wide
    // temporary directory
    this.path = ...
  }
  
  @Override
  public Path get() {
    // returns the subdirectory above
    return path;
  }
  
  @Override
  public void close() {
    // deletes the subdirectory
    deleteRecursively(path);
  }
  
  private static void deleteRecursively(Path path) {
    // ...
  }
}

import com.google.common.jimfs.Configuration;
import com.google.common.jimfs.Jimfs;

// An example of a user-defined resource supplier that uses Google's
// jimfs in-memory filesystem. Used in FooTests above.
public final class InMemoryDirectory implements ResourceSupplier<Path> {
  
  // An example of a "resource" that must eventually be closed
  // to prevent resource starvation.
  private final FileSystem jimFs;
  
  public InMemoryDirectory() {
    this.jimFs = JimFs.newFileSystem(Configuration.unix());
  }
  
  @Override
  public Path get() {
    return jim.getPath("/");
  }
  
  @Override
  public void close() {
    try {
      jim.close();
    } catch (Exception e) {
      throw new RuntimeException("Cannot close in-memory filesystem", e);
    }
  }
  
}

import mockwebserver3.MockWebServer;

// We can even create a resource supplier that holds an OkHttp mock web server!
public final class WebServer implements ResourceSupplier<Path> {
    
  // Another example of a resource that eventually needs to be closed.
  private final MockWebServer mockWebServer;
  
  public WebServer() {
    this.mockWebServer = new MockWebServer();
    this.mockWebServer.start();
  }
  
  @Override
  public MockWebServer get() {
    return mockWebServer;
  }
  
  @Override
  public void close() {
    mockWebServer.close();
  }
  
}

// Example test
@ExtendWith(ResourceManager.class)
class BazTests {
  @Test
  void testBaz(
      @New(WebServer.class) MockWebServer mockWebServer) {
    // given
    mockWebServer.setDispatcher(
        new Dispatcher() {
          @Override
          public MockResponse dispatch(RecordedRequest request) throws InterruptedException {
            return new MockResponse().setResponseCode(404);
          }
        });
    
    // when
    var client = new MyOwnHttpClient(mockWebServer.url("/"));
    MyOwnHttpClientResponse response = client.get();
    
    // then
    assertTrue(response.is404());
  }
}

// The basic building block of this extension.
public interface ResourceSupplier<T> {

  /**
   * Returns an instance of <code>T</code> pointing to a resource managed by
   * <code>this</code> instance of <code>ResourceSupplier</code>.
   * 
   * @return an instance of <code>T</code>.
   */
  T get();

  /**
   * Closes the underlying resource associated with the <code>T</code>
   * returned by {@link #get()}.
   */
  void close();
}

[nipafx]

I really like this proposal. I do recommend a different approach to closing resources, though:

interface Resource<T> extends Supplier<T>, AutoCloseable {

	@Override
	default void close() throws Exception {
		// no op by default
	}

}

interface ResourceFactory<T> extends AutoCloseable {

	Resource<T> create() throws Exception;

	@Override
	default void close() throws Exception {
		// no op by default
	}

}

Then, in the ParameterProvider:

@Override
public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext) throws ParameterResolutionException {
	// we get this from, e.g. `@Named(PathSupplier.class)`
	Class<ResourceFactory<?>> supplierType = null;
	ResourceFactory<?> factory = supplierType.getConstructor().newInstance();
	extensionContext.getStore(null).put("the factory", factory);

	Resource<?> resource = factory.create();

	// say that resource needs cleanup
	// 	~> generally, we can't assume the resource itself is `AutoCloseable`
	//  ~> adding `p` to the store will not lead to cleanup
	extensionContext.getStore(null).put("the resource", resource);

	return resource.get();
}

Btw, adding shared resources to the store will make it easy to retrieve them from there.

[nipafx]

Another thought - how can we have arguments for resource creation, e.g.:

@Test
// how do we specify the name of the file?
void testFoo(@New(TemporaryFile.class) Path file) {
	// ...
}

Oh wait, can we just have an attribute Object[] arguments on @New whose values get passed to the ResourceFactory constructor? It looks like that could work, so let's assume it does and go forward with this proposal. 😃 🟢 (<~ green light)

[jbduncan]

For posterity, I discussed @nipafx's proposal above with him over his Twitch stream, and I'm happy to give it a shot!

[jbduncan]

@nipafx I'll look into making ResourceManagerExtension thread-safe next.

However, I've already spent a few months on this PR, and it seems to me that not all of the JUnit 5 @TempDir issues mentioned in the original post are needed before we release this feature.

So after the thread safety, do you have any objections if we then move onto fixing the bug I mentioned above and finally writing the documentation?

[jbduncan]

Great news, I've fixed the illusive bug! So now the only thing I think I have to do is address the thread safety.

[jbduncan]

I've been on @nipafx's Twitch stream tonight, doing some remote pair programming - thank you very much for your help, Nicolai! - but I now have to solve some other seemingly concurrency-related bugs that have cropped up (noted in my uber TODO list further up). I'll report back when I have the time to delve into things further.

[jbduncan]

And now that this issue is finally implemented (after 1.5 years of effort!), let's see if I can get junit-team/junit5#2677 implemented at some point...