What is the worst thing that could happen if not closing Jimfs?

[oliviercailloux]

A common use case for Jimfs is in small junit tests: create a Jimfs file system with a few files, set their content, and call the method to be tested (making it somehow use the Jimfs file system to mock external environment). I wonder if I should insist on closing the Jimfs after use in such a case (assuming the created files are small).

I usually insist on closing resources after use when they declare they are Closeable, out of good habit, because I don’t like @SuppressWarnings (and my IDE is configured to be paranoïd), because it’s a good default rule…

In such a case, however, it seems perhaps over-cautious. I suppose that nothing bad can happen by not closing: I suppose that an in-memory file system does not reserve OS resources, and the only difference closing makes is possibly about (slightly) faster reclaim of the memory used for the files content. In fact, I even suspect that the only reason Jimfs implements Closeable is that the Java API demands it (and requires the behavior of the file system to change after close).

Also, it would make the code much clearer to not have to close Jimfs after use. I can then put the file system creation and initialization in another method and not worry about it (see example below). Otherwise, all the code in each of the tests using a Jimfs must be surrounded by a try-with-resource statement, or some other (even uglier) mechanism must be used. Transforming a simple 5 lines unit testing code into a 8 lines code with a big surrounding try is not a negligible loss of clarity.

Certainly, sticking to the good old default rule of always closing Closeable resources has value, but clear, readable code also has value. So, if the lack of closing never leads to a big problem in the case described (unit test with only a few simple files with low content), in some cases, it might be worth adding a @SuppressWarnings and not worry about it.

To allow to choose a reasonable trade-off in such a case, I would like to know if my assumption (that nothing very bad can happen in the case described) is justified. Please consider documenting to help users make such trade-offs in an informed manner.

Here is an illustrative example of what I have in mind. The first method is called by many unit tests. This approach does not work if closing the file system is important, for some reason I overlooked.

private Path createFakeFile(List<String> contentLines) throws IOException {
	FileSystem jimfs = Jimfs.newFileSystem();
	Path filePath = jimfs.getPath(FILE_NAME);
	Files.write(filePath, contentLines);
	return filePath;
}

Example use (nice: the createFakeFile hides irrelevant plumbing details about setting the fake file and its environment; and lets the reader see the essence of the test; problem: jimfs is never closed):

@Test
public void testEmptyPasswordFileReadAuthentication() throws Exception {
	QueriesHelper qh = QueriesHelper.newInstance();
	qh.apiLoginFile = createFakeFile(ImmutableList.of("a username", ""));

	LoginOpt myAuth = qh.readAuthentication();

	assertEquals("file username", myAuth.getUsername().get());
	assertEquals("", myAuth.getPassword().get());
}

Possible alternative if jimfs must absolutely be closed:

@Test
public void testEmptyPasswordFileReadAuthentication() throws Exception {
	try (FileSystem jimfs = Jimfs.newFileSystem(Configuration.unix())) {
		Path filePath = jimfs.getPath(FILE_NAME);
		Files.write(filePath, ImmutableList.of("a username", ""));

		QueriesHelper qh = QueriesHelper.newInstance();
		qh.apiLoginFile = filePath;

		final LoginOpt myAuth = qh.readAuthentication();

		assertEquals("file username", myAuth.getUsername().get());
		assertEquals("", myAuth.getPassword().get());
	}
}

Also, using @BeforeTest is not great either in my case, because other tests in the same class do not require jimfs, I’d like to not let the reader think it is used in all tests.

[cgdecker]

Any Jimfs FileSystem that's created is statically cached in the FileSystems class. Closing the FileSystem removes it from the cache. Unless you close it, the FileSystem and any data stored in it will persist for the lifetime of the VM (or at least classloader). This is needed for one thing: to allow access to the FileSystem or a Path from it via URI-based methods like Paths.get(URI).

Ideally we'd have an artifact that provided a JUnit Rule for creating and closing a file system for each test.

Another thing I'd considered (but abandoned, possibly for some good reason I'm not remembering currently; maybe it didn't actually work) is making URI lookup support an optional feature that has to be explicitly enabled when creating a Jimfs FileSystem. That would allow FileSystems created without that support to be garbage collected when no longer referenced.

[yankee42]

Ideally we'd have an artifact that provided a JUnit Rule for creating and closing a file system for each test.

I currently do it like this with TestNG and Kotlin:

class FileSystemTestListener : IInvokedMethodListener {
    override fun beforeInvocation(method: IInvokedMethod, testResult: ITestResult) {
        val instance = testResult.instance
        if ((method.isTestMethod || testResult.method.isBeforeMethodConfiguration) && instance is FileSystemTest) {
            if (instance
                    .runCatching { !fileSystem.isOpen }
                    .recover { if (it is UninitializedPropertyAccessException) true else throw it }
                    .getOrThrow()
            ) {
                val fileSystem =
                    Jimfs.newFileSystem(Configuration.unix().toBuilder().setAttributeViews("posix").build())
                val testDir = fileSystem.getPath("/testDir")
                Files.createDirectory(testDir)
                instance.fileSystem = fileSystem
                instance.testDir = testDir
            }
        }
    }

    override fun afterInvocation(method: IInvokedMethod, testResult: ITestResult) {
        val instance = testResult.instance
        if (method.isTestMethod && instance is FileSystemTest) {
            instance.fileSystem.close()
        }
    }

}

interface FileSystemTest {
    var fileSystem: FileSystem
    var testDir: Path
}

That makes tests easy:

@Listeners(FileSystemTestListener::class)
class MyTest : FileSystemTest {
    override lateinit var fileSystem: FileSystem
    override lateinit var testDir: Path

    @BeforeMethod
    fun beforeMethod() {
        Files.createFile(testDir.resolve("demo"));
    }

    @Test
    fun doTest() {
        assertThat(Files.exists(testDir.resolve("demo")), equalTo(true))
    }
}

Not quite perfect, yet. See also: https://stackoverflow.com/questions/64866149