Mock/Stub out filesystem in F# for unit testing

Question

I'm looking to do some basic verification testing on my functions that write to the filesystem.

I took a hint from here on how to mock out the filesystem using an interface, but I'm kinda bummed on how the FileSystemOperations type needs to be passed along through every function. Any ideas how to improve this, or where I went wrong?

I'm rather new to FP, so any feedback on other design issues are welcome as well.

module FileSystem =
 open System.IO
 type IFileSystemOperations = 
 abstract member copy: string -> string -> bool -> unit
 abstract member delete: string -> unit
 abstract member readAllBytes: string -> byte[]
 abstract member createDirectory: string -> DirectoryInfo
 abstract member directoryExists: string -> bool
 type FileSystemOperations () =
 interface IFileSystemOperations with
 member this.copy source destination overwrite =
 File.Copy(source, destination, overwrite)
 member this.delete path =
 File.Delete path
 member this.readAllBytes path =
 File.ReadAllBytes path
 member this.createDirectory path =
 Directory.CreateDirectory path
 member this.directoryExists path =
 let fileInfo = FileInfo path
 fileInfo.Directory.Exists
module FileMover =
 open FileSystem
 let private ensureDirectoryExists (fileSystemsOperations:IFileSystemOperations) destination =
 let directoryExists = fileSystemsOperations.directoryExists destination
 if not (directoryExists) then
 fileSystemsOperations.createDirectory destination |> ignore
 let private compareFiles (fileSystemsOperations:IFileSystemOperations) moveRequest =
 let sourceStream = fileSystemsOperations.readAllBytes moveRequest.Source
 let destinationStream = fileSystemsOperations.readAllBytes moveRequest.Destination
 sourceStream = destinationStream
 let private copyToDestination (fileSystemsOperations:IFileSystemOperations) (moveRequest:MoveRequest) =
 ensureDirectoryExists fileSystemsOperations moveRequest.Destination
 fileSystemsOperations.copy moveRequest.Source moveRequest.Destination true
 let private deleteSource (fileSystemsOperations:IFileSystemOperations) (move:Move) =
 fileSystemsOperations.delete move.Request.Source
 let private moveFile (fileSystemsOperations:IFileSystemOperations) moveRequest =
 copyToDestination fileSystemsOperations moveRequest
 let filesMatch = compareFiles fileSystemsOperations moveRequest
 let move = match filesMatch with
 | true -> {Request = moveRequest; Result = Success}
 | false -> {Request = moveRequest; Result = Failure BytesDidNotMatch}
 deleteSource fileSystemsOperations move
 let move (fileSystemsOperations:IFileSystemOperations) targetPath pictures =
 let moveFile = moveFile fileSystemsOperations
 pictures
 |> getMoveRequests targetPath
 |> Seq.map moveFile

Update (for reference & anyone interested)

Based on the answers below, here is how I currently refactored the code. It's a work in progress, but it's already more easily testable (main goal) and the responsibilities seem to fit better as well (eg: injecting ensureDirectoryExists into copyToDestination instead of into moveFile).

Thanks for your answers!

• Mark Seemann: This article may be superficially relevant: blog.jessitron.com/2015/06/ultratestable-coding-style.html
• Mark Seemann: FWIW, for some time, I haven't been entirely happy with my below answer, so I wrote a series of articles about this problem, culminating in this one: blog.ploeh.dk/2019/09/16/picture-archivist-in-f

Answer 1

Even though the SOLID principles are known to be principles related to Object-Oriented Design, I'd still take a cue from the Dependency Inversion Principle. As APPP states it (ch. 11): "clients [...] own the abstract interfaces".

You can apply this principle in FP as well, in the sense that you can start by defining the overall behaviour of the function you're interested in, and then see what happens.

As an example, I'd approach the ensureDirectoryExists function like this:

let private ensureDirectoryExists destination =
 let directoryExists = dirExists destination
 if not directoryExists then
 createDir destination

Now, the above version doesn't compile, because dirExists and createDir aren't defined. However, that's easy to fix: just promote them to function arguments:

let private ensureDirectoryExists dirExists createDir destination =
 let directoryExists = dirExists destination
 if not directoryExists then
 createDir destination

Viola: now you have a higher-order function; it's type is

('a -> bool) -> ('a -> unit) -> 'a -> unit

No interfaces are required.

This is already intriguing for a couple of reasons:

• It's no longer about files or directories: 'a could be string values containing file names or directory names, but it could also be DirectoryInfo or something not at all related to files.
• Although it looks complex, it only involves two types: 'a and unit. That should make us pay attention as well.
• You can use partial application to apply the ensureDirectoryExists with the two first arguments, and then use this partially applied function to test any number of different directories.

You can still 'implement' dirExists and createDir, and apply them like this:

open System.IO
let directoryExists path =
 let fileInfo = FileInfo path
 fileInfo.Directory.Exists
let ensureDirectoryExists' =
 ensureDirectoryExists directoryExists (Directory.CreateDirectory >> ignore)

Notice that the createDir 'implementation' is so simple that I chose to inline it. I could also have inlined the dirExists 'implementation' with (fun path -> (FileInfo path).Directory.Exists), but I chose to do this as a separate function in order to illustrate the various options available.

The partially applied ensureDirectoryExists' function has this simple function signature: (string -> unit), which is comparable to the OP version of the function.

---

While I think I'll leave the rest of the functions as an exercise, I'd like to return to the thought-provoking signature of the ensureDirectoryExists function:

('a -> bool) -> ('a -> unit) -> 'a -> unit

There's nothing here about files or directories, so a more generic name might be in place. It looks like the Tester-Doer idiom, so we could call it testDo instead.

One of the great benefits of making the interface more generic is that you don't need to involve the file system at all for testing. Instead, you can test this function using integers, strings, or some other 'easy' values. If the logic is unconstrained generic and works for numbers and strings, it'll also work for directories and files.

• koenmetsu: Thanks for your answer, Mark. I like the thinking behind making these more generic, but won't using these separate functions instead of the IFileSystemOperations bloat the public move function with the injection of all five functions in IFileSystemOperations? ie: let move directoryExists createDirectory readBytes delete copy targetPath pictures = //...
• Mark Seemann: As far as I can tell, move depends on one other function: moveFile (although I can't tell where getMoveRequests comes from, so it may depend on two functions...). It's a simple repetition of the exercise done above. Instead of ending with the public function, start by defining the public function and see what it needs in order to work.
• koenmetsu: Are you suggesting to inject the moveFile function into move? In that case, it seems there's not much reason for move to exist anymore. And if not, then move needs to be injected with the functions upon which moveFile depends, and it gets bloated. It's probably my currently limited understanding, but could you clarify this? Thanks a lot.
• Mark Seemann: I'm suggesting injecting some moveFile function into move. Once you've done that, you can look at its inferred type to figure out what you need to do next. Then you can repeat the process in a recursive fashion. It's an outside-in process that I describe (among much else) in my Type-Driven Development course.
• koenmetsu: I've seen that one, so either I'm still missing something, or I need to take this a bit further than I have been. Thanks for the comments!

Answer 2

I think that Mark's answer gives a great alternative perspective here.

That said, I would probably write something that's quite similar to what you did here. There is a couple of small tricks that I would perhaps use to make the code a bit nicer (at least for my own personal idea of "nicer" F# code :-)), so I thought I'd share those.

First of all, I would probably use PascalCase and tuples-syntax for the definition of the interface. The PascalCase is recommended for members and I think it makes sense to keep those aligned with the .NET style. You can also omit member:

type IFileSystemOperations = 
 abstract Copy: string * string * bool -> unit
 abstract Delete: string -> unit
 abstract ReadAllBytes: string -> byte[]
 abstract CreateDirectory: string -> DirectoryInfo
 abstract DirectoryExists: string -> bool

Another thing you could do would be to use object expression rather than a full class. Here I don't really have one preferred option. But I might write:

let FileSystemOperations =
 { new IFileSystemOperations with
 member this.Copy(source, destination, overwrite) =
 File.Copy(source, destination, overwrite)
 (* ... *) }

The next thing that is a bit unfortunate is that you always have to write fileSystemOperations:IFileSystemOperations, which makes all your method headers long.

If the private helpers are not used anywhere else, you could avoid this by turning them into local functions. That said, this might not be all that nice, because it makes the move function very long (just syntactically, not logically):

let move (fileSystemsOperations:IFileSystemOperations) targetPath pictures =
 let ensureDirectoryExists destination =
 let directoryExists = fileSystemsOperations.directoryExists destination
 if not (directoryExists) then
 fileSystemsOperations.createDirectory destination |> ignore
 let compareFiles moveRequest =
 let sourceStream = fileSystemsOperations.readAllBytes moveRequest.Source
 let destinationStream = fileSystemsOperations.readAllBytes moveRequest.Destination
 sourceStream = destinationStream
 let copyToDestination (moveRequest:MoveRequest) =
 ensureDirectoryExists oveRequest.Destination
 fileSystemsOperations.copy moveRequest.Source moveRequest.Destination true
 let deleteSource (move:Move) =
 fileSystemsOperations.delete move.Request.Source
 let moveFile moveRequest =
 copyToDestination moveRequest
 let filesMatch = compareFiles moveRequest
 let move = 
 { Request = moveRequest; 
 Result = if filesMatch then Success else Failure BytesDidNotMatch}
 deleteSource move
 pictures
 |> getMoveRequests targetPath
 |> Seq.map moveFile

I also think there is no point in pattern matching over booleans, because you are effectively just writing simple if with more complex syntax, so I turned your original pattern match into an inline if in moveFiles

(I also changed spacing to 2 spaces in the last snippet, but that's just to fit things on the web page).

EDIT: Using nested local functions would also quite nicely work with Mark's suggestion to pass around functions. This way, you would not have to propagate them through all the other helper functions, so it might even look nicer than my answer.

• koenmetsu: Thanks a lot, Tomas. Your small improvements are very useful, I was actually trying to go for object expressions but clearly dropped the ball there :D Your comment about nested local functions is very interesting, I'm curious to see what I can achieve with the combination!