================================
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 :doc:`Tezt <tezt>` or :doc:`Python tests <python_testing_framework>` - 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 modules ``B``, ``C`` and ``D``, thus unit-testing ``A`` requires setting everything up for ``B``, ``C`` and ``D`` 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):

.. code:: ocaml

    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``:

.. code:: ocaml

    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 constructor ``Counter.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:

.. code:: ocaml

    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:

.. code:: ocaml

    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`` and ``increment_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:

.. code:: ocaml

    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 value
- Independent of the dependency depth (for functions): if ``A`` calls ``B.f`` which calls ``C.g``, your mock of ``B.f`` will never call ``C.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`` module
- Adds 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 a ``dependencies`` field
- Else add a ``dependencies`` argument - but this requires duplicating each function, which ends up being very verbose if you have several functions
- If your "state" value (usually a value of type ``t``) is passed to a polymorphic function like ``=`` or ``compare`` (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``:

.. code:: ocaml

    (* 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!

.. code:: ocaml

    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``!

.. code:: ocaml

    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 <https://www.craigfe.io/posts/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 on ``B`` which depends on ``C``, but ``A`` does not depend on ``C``)

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 value
- Independent of the dependency depth (for functions): if ``A`` calls ``B`` which calls ``C``, your mock of ``B`` will never call ``C`` nor refer to its abstract types

Cons:

- Verbosity
- Additional complexity (functors, module types)
- Does not scale well in more complex dependencies (``A`` depends on ``B`` and ``C`` types, and ``B`` also depends on ``C`` types) as it induces a lot of destructive substitutions and module noise to convince the typechecker that ``C.t`` in ``A`` is the same as ``C.t`` in ``B``
- Does not work for exposed and private (exposed in read-only) dependency types