Making legacy code unit-testable#
This guide describes how to make legacy code testable and how to test it. The following techniques do not require any API change: they concern testing each single module as a unit without touching modules depending on it. These techniques are applicable to legacy code: you do not need to have an exact knowledge of every implementation detail of a module to test it.
The goals are:
to gain confidence in a module as a developer (or even part of test-driven development)
to document it: tests are examples
to have a safety net as a maintainer (non-regression testing), e.g., when: - adding/changing features - performing various forms of refactoring (changing the API or not)
In the rest of this page, “testable” means “unit-testable”, for conciseness.
Here “legacy” is a loose term, encompassing:
existing code written by other developers, not necessarily containing a right amount of built-in unit tests
code that is only tested by heavy/slow/high level tests, like Tezt - this is often a symptom of code that is hard to unit-test
tightly-coupled code (e.g. module
A
directly depends on functions of modulesB
,C
andD
, thus unit-testingA
requires setting everything up forB
,C
andD
too, which not only makes unit testing harder, but is also complex and brittle)code that calls system functions (e.g.
Unix.chmod
, which requires modifying actual files on the filesystem, also making unit testing harder) or more generally any kind of side-effecting function
Solutions#
Testing a module and all its dependencies#
Sometimes, legacy modules depending on other modules may be “unit-tested” without modifying them at all.
This approach is technically not “unit” testing, as the module/function is not tested in isolation from its dependencies. However as the intention is close to unit testing, we include it here.
Pros:
No change at all in business code (not even an
Internal_for_tests
module)
Cons:
Code not tested in isolation
Tests are harder to write and maintain
Works best when:
Code and its dependencies are pure, or “pure enough” (e.g. code may contain logging and other unharmful/unimportant side effects)
Dependency tree is not too deep (e.g. if A depends on B which depends on C which depends on D, then it becomes hard to write/read/maintain unit tests)
Using function records#
This approach decouples the business code from its dependency functions, either by storing dependency functions in the state - if any - or by taking them as additional parameters in each module function.
Note that this does not decouple types, only functions! If your module depends on a dependency type that can only be built by having side-effects, then you are forced to have those side effects too in your tests.
As an example, consider the following Counter
module (signature and implementation):
module Counter : sig
type t
val create_from_file : unit -> t
val increment_and_save_to_file : t -> t
end = struct
type t = {
counter : int;
}
let create_from_file () =
let counter = int_of_string (Files.read "/tmp/counter.txt") in
{counter}
let increment_and_save_to_file t =
let t = {counter = t.counter + 1} in
Files.write "/tmp/counter.txt" (string_of_int t.counter);
t
end
Counter
is hardly unit-testable because of the calls to Files.read
and Files.write
which have the side effect of reading from/writing into /tmp/counter.txt
file.
Thus we extract Files.read
and Files.write
into an argument that can be changed with a dumb (“mock”) function in tests.
Store dependencies in state#
One way to achieve this - when your module has a state, usually a type t
- is to store all dependency functions in a field, here dependencies
:
module Counter : sig
type t
val create_from_file : unit -> t
val increment_and_save_to_file : t -> t
module Internal_for_tests : sig
type dependencies = {
files_read : string -> string;
files_write : string -> string -> unit;
}
val create_from_file : dependencies -> unit -> t
end
end = struct
type dependencies = {
files_read : string -> string;
files_write : string -> string -> unit;
}
type t = {
counter : int;
dependencies : dependencies;
}
let create_from_file_internal dependencies () =
let counter = int_of_string (dependencies.files_read "/tmp/counter.txt") in
{counter; dependencies}
let create_from_file = create_from_file_internal {files_read = Files.read; files_write = Files.write}
let increment_and_save_to_file t =
let t = {t with counter = t.counter + 1} in
t.dependencies.files_write "/tmp/counter.txt" (string_of_int t.counter);
t
module Internal_for_tests = struct
type nonrec dependencies = dependencies = {
files_read : string -> string;
files_write : string -> string -> unit;
}
let create_from_file = create_from_file_internal
end
end
Note that the direct calls to Files.read
and Files.write
were replaced with indirect calls to dependencies.files_read
and field t.dependencies.files_write
:
They are set to
Files.[read|write]
in the business constructorCounter.create_from_file
They are changed at will in the test constructor
Counter.Internal_for_tests.create_from_file
Also note that while the API was extended with test artifacts under the Internal_for_tests
sub-module, the public API is otherwise unchanged, thus keeping this refactoring local - you do not need to change any call sites!
Now we can test this module without any side effect:
let test () =
let counter_value_written = ref "" in
let fake_files_read file_name = "41" in
let fake_files_write file_name text =
counter_value_written := text
in
let counter = Counter.Internal_for_tests.create_from_file {files_read = fake_files_read; files_write = fake_files_write} () in
let _ = Counter.increment_and_save_to_file counter in
Alcotest.(check string) "counter value was incremented in file" !counter_value_written "42"
Taking dependencies in function argument#
An alternative solution, more verbose but not requiring any “state” value available in each function, is to take the dependencies directly as an additional function argument:
module Counter : sig
type t
val create_from_file : unit -> t
val increment_and_save_to_file : t -> t
module Internal_for_tests : sig
type dependencies = {
files_read : string -> string;
files_write : string -> string -> unit;
}
val create_from_file : dependencies -> unit -> t
val increment_and_save_to_file : dependencies -> t -> t
end
end = struct
type dependencies = {
files_read : string -> string;
files_write : string -> string -> unit;
}
type t = {
counter : int;
}
let business_dependencies = {
files_read = Files.read;
files_write = Files.write;
}
let create_from_file_internal dependencies counter =
let counter = int_of_string (dependencies.files_read "/tmp/counter.txt") in
{counter}
let create_from_file = create_from_file_internal business_dependencies
let increment_and_save_to_file_internal dependencies t =
let t = {counter = t.counter + 1} in
dependencies.files_write "/tmp/counter.txt" (string_of_int t.counter);
t
let increment_and_save_to_file t = increment_and_save_to_file_internal business_dependencies t
module Internal_for_tests = struct
type nonrec dependencies = dependencies = {
files_read : string -> string;
files_write : string -> string -> unit;
}
let create_from_file = create_from_file_internal
let increment_and_save_to_file = increment_and_save_to_file_internal
end
end
Note that the direct calls to Files.read
and Files.write
were replaced with indirect calls to arguments dependencies.files_read
and dependencies.files_write
:
They are set to
Files.[read|write]
in each business function (create_from_file
andincrement_and_save_to_file
)They are changed at will in the test function
Counter.Internal_for_tests.[create_from_file|increment_and_save_to_file]
As in the previous solution, notice that the public API has not changed - save for additional APIs in Internal_for_tests
.
Now we can test this module without any side effect:
let test () =
let counter_value_written = ref "" in
let fake_files_read file_name = "41" in
let fake_files_write file_name text =
counter_value_written := text
in
let mock_dependencies = Counter.Internal_for_tests.{
files_read = fake_files_read;
files_write = fake_files_write;
} in
let counter = Counter.Internal_for_tests.create_from_file mock_dependencies () in
let _ = Counter.Internal_for_tests.increment_and_save_to_file mock_dependencies counter in
Alcotest.(check string) "counter value was incremented in file" !counter_value_written "42"
Pros and Cons for Function records#
Works best when:
Dependency types are not too hard to build
Pros:
No side-effecting function is called (they are replaced with mocks)
Enables validating the arguments passed to mock functions (e.g.
counter_value_written
) have the right valueIndependent of the dependency depth (for functions): if
A
callsB.f
which callsC.g
, your mock ofB.f
will never callC.g
Cons:
All dependency types remain, so if it is difficult/side-effectful to create those values, testing remains difficult/not so unitary
Adds a bit of boilerplate in
Internal_for_tests
moduleAdds a bit of indirection, by introducing indirect calls to dependency functions. The associated performance overhead should be negligible in most practical cases. There also is a slight decrease in code readability, but documenting this unit-testability pattern should avoid many headaches.
To choose between the field and the argument:
If your module already has a kind of “state” (usually a type
t
), then add adependencies
fieldElse add a
dependencies
argument - but this requires duplicating each function, which ends up being very verbose if you have several functionsIf your “state” value (usually a value of type
t
) is passed to a polymorphic function like=
orcompare
(which throw on function fields, and are famous for being an anti-pattern), and it is not possible for you to fix this anti-pattern, then either switch to function arguments, or wrap in an object.
Using functors#
This approach decouples the business code from its dependency modules. Note that unlike the Function records solution, this decouples both dependency functions and abstract types!
Consider the following code: it is similar to the previous Counter
example but this time, the Files
dependency module (which could be another module, a third party library, or even the Stdlib
) also has an abstract type t
:
(* The dependency *)
module Files : sig
type t
val openf : string -> t
val write : t -> string -> unit
val close : t -> unit
(* Many other functions and types *)
end = struct (* omitted implementation *) end
module Counter : sig
type t
val create : int -> t
val increment_and_save_to_file : t -> t
end = struct
type t = {
counter : int;
}
let create counter = {counter}
let increment_and_save_to_file t =
let t = {counter = t.counter + 1} in
let file = Files.openf "/tmp/counter.txt" in
Files.write file (string_of_int t.counter);
Files.close file;
t
end
The technique is to transform Counter
into a functor that takes a module looking like Files
in argument - but which can now be changed in tests!
module Counter : sig
module type S = sig
type t
val create : int -> t
val increment_and_save_to_file : t -> t
end
include S
module Internal_for_tests : sig
module type FILES = sig
type t
val openf : string -> t
val write : t -> string -> unit
val close : t -> unit
end
module Make (Files : FILES) : S
end
end = struct
module type S = sig
type t
val create : int -> t
val increment_and_save_to_file : t -> t
end
module type FILES = sig
type t
val openf : string -> t
val write : t -> string -> unit
val close : t -> unit
end
module Make (Files : FILES) = struct
type t = {
counter : int;
}
let create counter = {counter}
let increment_and_save_to_file t =
let t = {counter = t.counter + 1} in
let file = Files.openf "/tmp/counter.txt" in
Files.write file (string_of_int t.counter);
Files.close file;
t
end
(* Do not be mistaken: here [Files] refers to the real, business [Files] module! *)
include Make (Files)
module Internal_for_tests = struct
module type FILES = FILES
module Make = Make
end
end
As you can see, this is significantly more verbose!
However, now we can freely change/mock not only the dependency functions Files.[openf|close|write]
, but also the implementation of type Files.t
!
let test () =
let written_content = ref "" in
let module Counter = Counter.Internal_for_tests.Make (struct
type t = unit
let openf _ = ()
let close _ = ()
let write _ content = written_content := content
end) in
let counter = Counter.create 41 in
let _ = Counter.increment_and_save_to_file counter in
Alcotest.(check string) "counter value was incremented in file" !written_content "42"
While the real Files.t
type probably contained a file descriptor, our mock module has no side effect outside of the test!
Note on verbosity: some things are duplicated because of OCaml MLI syntax, e.g. module type declaration. This can be partially mitigated by using the _intf trick but this in turn induces a bit more complexity, use with caution.
Works best when:
You need to decouple (abstract) types, not only functions. For example, because building values of those types adds too much complexity, or requires side-effects.
There are linear dependencies in modules (
A
depends onB
which depends onC
, butA
does not depend onC
)
Pros:
Everything but exposed and private dependency types are mocked
Enables validating the arguments passed to mock functions (e.g.
counter_value_written
) have the right valueIndependent of the dependency depth (for functions): if
A
callsB
which callsC
, your mock ofB
will never callC
nor refer to its abstract types
Cons:
Verbosity
Additional complexity (functors, module types)
Does not scale well in more complex dependencies (
A
depends onB
andC
types, andB
also depends onC
types) as it induces a lot of destructive substitutions and module noise to convince the typechecker thatC.t
inA
is the same asC.t
inB
Does not work for exposed and private (exposed in read-only) dependency types