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

Jimfs support for JUnit 5 TempDirFactory #258

Open
scordio opened this issue Jul 24, 2023 · 8 comments
Open

Jimfs support for JUnit 5 TempDirFactory #258

scordio opened this issue Jul 24, 2023 · 8 comments
Labels
P4 type=addition A new feature

Comments

@scordio
Copy link
Contributor

scordio commented Jul 24, 2023

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:

  • On each @TempDir annotation via the factory attribute, e.g.:
class MyTest {

  @TempDir(factory = JimfsTempDirFactory.class)
  private Path tempDir;

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

class MyTest {

  @JimfsTempDir
  private Path tempDir;

  ...
} 
  • Globally, via a configuration parameter, e.g.:
junit.jupiter.tempdir.factory.default=com.google.common.jimfs.junit.jupiter.JimfsTempDirFactory

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
Copy link
Member

cpovirk commented Jul 24, 2023

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? :)

@cpovirk cpovirk added P4 type=addition A new feature labels Jul 24, 2023
@scordio
Copy link
Contributor Author

scordio commented Jul 24, 2023

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
Copy link
Member

cpovirk commented Aug 4, 2023

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
Copy link

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
Copy link

TobseF commented Jul 26, 2024

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
Copy link
Contributor Author

scordio commented Jul 27, 2024

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
Copy link
Contributor Author

scordio commented Jul 27, 2024

@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
Copy link
Contributor Author

scordio commented Jul 30, 2024

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

Any feedback is welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 type=addition A new feature
Projects
None yet
Development

No branches or pull requests

4 participants