google / jimfs

An in-memory file system for Java 7+
Apache License 2.0
2.42k stars 278 forks source link

Jimfs support for JUnit 5 `TempDirFactory` #258

Open scordio opened 1 year ago

scordio commented 1 year ago

Starting from version 5.10, JUnit 5 offers a TempDirFactory SPI for customizing how temporary directories are created via the @TempDir annotation.

The SPI allows libraries like Jimfs to provide their own implementation, which can be used:

* As a meta-annotation, e.g.:
```java
@Target({ ANNOTATION_TYPE, FIELD, PARAMETER })
@Retention(RUNTIME)
@TempDir(factory = JimfsTempDirFactory.class)
@interface JimfsTempDir {
}

class MyTest {

  @JimfsTempDir
  private Path tempDir;

  ...
} 

You might notice that the JUnit documentation already refers to Jimfs to demonstrate certain use cases.

Would you consider first-party support for such a feature, providing your own factory implementation and maybe even meta-annotation(s)?

Disclaimer: I authored most of the TempDirFactory changes so happy to hear your feedback and report back improvements, if you see any 🙂

cpovirk commented 1 year ago

Interesting, thanks. Cool that jimfs makes the JUnit docs!

We're not JUnit 5 users (yet? unclear :)), so it probably won't be a priority, but that's not to rule it out entirely. (If nothing else, it makes it harder for us to judge design questions like "Should it be able to use a FileSystem from the test?" and "If so, might it be even better as a filesystem-agnostic utility than as one specific to jimfs?") We can see what demand arises, and then... who knows? :)

scordio commented 1 year ago

We're not JUnit 5 users (yet? unclear :)), so it probably won't be a priority, but that's not to rule it out entirely.

I see, at least you're now aware of it 🙂

it makes it harder for us to judge design questions like "Should it be able to use a FileSystem from the test?" and "If so, might it be even better as a filesystem-agnostic utility than as one specific to jimfs?"

Not sure I fully understood what you mean. I personally consider relying on an in-memory file system in tests a common use case and that was one of my motivations for the changes I proposed to the JUnit 5 team.

Just to point to a real-life example of a TempDirFactory usage, your "competitor" memoryfilesystem offers now a JUnit 5 specific module: https://github.com/marschall/memoryfilesystem-junit-provider

cpovirk commented 1 year ago

Sorry. I'm not sure if I understand what I mean, either :) My thinking was roughly that there might be a way to write a single annotation that could tell JUnit to use an arbitrary given FileSystem implementation, perhaps one pulled from a field in the test class. But this all needs more thought. If we do start adopting JUnit 5 at some point, we may take a look.

mikehearn commented 4 months ago

To demonstrate real-life demand I'll AOL this thread #metoo :) It'd be great to have such a provider, it'd make Jimfs even easier to use.

TobseF commented 1 month ago

Thanks for the JUnit5 example (GitHub link) @scordio 👍

I came along with this version, which also adds fields to set the System (Win, Osx, Linux) and the root dir name:

// Kotlin
@Target(AnnotationTarget.FIELD, AnnotationTarget.VALUE_PARAMETER)
@Retention(AnnotationRetention.RUNTIME)
@TempDir(factory = JimfsTempDirFactory::class)
annotation class JimfsTempDir(
    val system: FileSystem = FileSystem.Unix,
    val folder: String = "junit"
) {
    enum class FileSystem { Unix, OsX, Windows }
}

class JimfsTempDirFactory : TempDirFactory {

    private val fileSystemUnix: Lazy<FileSystem> = lazy { Jimfs.newFileSystem(Configuration.unix()) }
    private val fileSystemOsX: Lazy<FileSystem> = lazy { Jimfs.newFileSystem(Configuration.osX()) }
    private val fileSystemWindows: Lazy<FileSystem> = lazy { Jimfs.newFileSystem(Configuration.windows()) }

    override fun createTempDirectory(elementContext: AnnotatedElementContext, extensionContext: ExtensionContext): Path {
        val tempDirAnnotation = elementContext.findAnnotation<JimfsTempDir>(JimfsTempDir::class.java).orElseThrow()
        val fileSystemType = tempDirAnnotation.system
        val folder = tempDirAnnotation.folder
        return Files.createTempDirectory(getFileSystem(fileSystemType).getPath("/"), folder);
    }

    fun getFileSystem(system: JimfsTempDir.FileSystem) = when (system) {
        JimfsTempDir.FileSystem.Unix -> fileSystemUnix
        JimfsTempDir.FileSystem.OsX -> fileSystemOsX
        JimfsTempDir.FileSystem.Windows -> fileSystemWindows
    }.value

    override fun close() {
        listOf(fileSystemUnix, fileSystemOsX, fileSystemWindows)
            .filter { it.isInitialized() }.forEach { it.value.close() }
    }

}

Feedback for the TempDirFactory

The TempDirFactory can be used to inject multiple fields or may be even shared between all tests. This is how I get it from the guaranteed assumptions. Because of the close() function, there is no way to declare the factory stateless. I need a field to reference in the close. Based on the option of the annotation, I need to create another FileSystem.

I see two ways to enhance this.

  1. A way to force the creation of he TempDirFactory for every annotated field) Then only one option is possible and one FileSystem would be enough.
  2. Provide a way to return a Closeable for every createTempDirectory(). Then the function could be stateless and can return a new FileSystem for every call. Then the TempDirFactory could be shared between all tests.

Considerations for jimfs

I'm thinking about publishing this snipped to maven. But such a small dependency is not the best option. I hope the jimfs team will take it into consideration to add an official support package for JUnit.

It's a perfect option for tests: Fast, reliable, isolated - And the best: It can simulate different systems, independent from the environment. So let's do all, to spread it in the test community and make it as accessible as possible.

In my case I'm planning to replace all @TempFile based Unit tests by jimfs. Thanks for maintaining this great lib 👍

scordio commented 1 month ago

As @cpovirk mentioned, first-party support might be provided only once Google moves to JUnit 5.

As I already had a first draft of an extension based on Jimfs, I decided to push it forward. I'll continue polishing it (README and Javadoc are currently missing 🙂 ) and likely release the first version at the beginning of the week.

A simple usage example:

class JimfsTempDirTests {

  @JimfsTempDir(WINDOWS) // parameter is optional, can also be FOR_CURRENT_PLATFORM, OS_X, UNIX
  Path tempDir;

  @Test
  void test() {
      assertThat(tempDir.getFileSystem().provider().getScheme()).isEqualTo("jimfs");
      assertThat(tempDir.getFileSystem().getSeparator()).isEqualTo("\\");
  }
}

In case you'd like a first look, here it is: https://github.com/scordio/jimfs-junit-jupiter

Feedback appreciated!

scordio commented 1 month ago

@TobseF about your feedback:

The TempDirFactory can be used to inject multiple fields or may be even shared between all tests. This is how I get it from the guaranteed assumptions. Because of the close() function, there is no way to declare the factory stateless. I need a field to reference in the close. Based on the option of the annotation, I need to create another FileSystem.

By default, JUnit creates a new temporary directory for each @TempDir declaration. Plus, the user guide describes that:

Factories can be created by implementing TempDirFactory. Implementations must provide a no-args constructor and should not make any assumptions regarding when and how many times they are instantiated, but they can assume that their createTempDirectory(…​) and close() methods will both be called once per instance, in this order, and from the same thread.

With this, there should be no need to create multiple file systems in the same factory instance. Feel free to check how I approached it: https://github.com/scordio/jimfs-junit-jupiter/blob/6511f81cbb48267dd1a4dec3e93c575a280fec43/src/main/java/io/github/scordio/jimfs/junit/jupiter/JimfsTempDirFactory.java#L37-L50

About something else entirely, I noticed in your snippet that you declared a folder attribute for your annotation (which is actually a folder name prefix when you delegate to Files::createTempDirectory). While this is definitely possible, I would like to know if you have a concrete use case in mind for it.

scordio commented 1 month ago

And here is the first release: https://github.com/scordio/jimfs-junit-jupiter/releases/tag/v0.1.0

Any feedback is welcome!