6
\$\begingroup\$

I have a function that constructs a path based on a function of inputs and creates the parent directory.

When writing a unit test for this function, I want to mock the I/O part of the function, namely the .exists() and .mkdir() methods of the pathlib.Path objects. However, since both are not called until the path has been constructed, I find myself reconstructing the implementation using a long chain of return_value.__truediv__ accesses, such as below:

import pathlib
import unittest.mock
import datetime


def get(base, timestamp, channel):
    base = pathlib.Path(base)
    path = base / "tofu" / timestamp.strftime("%Y%m%d") / f"{channel:>02d}" / "quinoa.txt"
    if path.exists():
        print(path, "exists")
    else:
        path.parent.mkdir(exist_ok=True, parents=True)
    return path


@unittest.mock.patch("pathlib.Path", autospec=True)
def test_get(pP):
    pP.return_value.__truediv__.return_value.__truediv__.return_value.\
                    __truediv__.return_value.__truediv__.return_value.\
                    exists.return_value = False
    get("quorn", datetime.datetime.now(), 42)
    pP.return_value.__truediv__.return_value.__truediv__.return_value.\
                    __truediv__.return_value.__truediv__.return_value.\
                    parent.mkdir.assert_called_once()

This test passes, but I have several concerns:

  • It is testing implementation rather than functionality. If I were to rewrite the second line in get to read path = base / timestamp.strftime("tofu%Y%m%d") / f{channel:>02d} / "quinoa.txt", the functionality would be identical, but the test would fail as I wouldn't be mocking the right value anymore.
  • It is hard to read and easy to miscount the depth of __truediv__.return_value chain.

Apart from moving the I/O out of get, can I mock this in another way, that does not share those limitations? Or does the difficulty of writing a good test (with mock) telling me my function is too complex, despite it's small size?

\$\endgroup\$
0
5
\$\begingroup\$

I'm no professional when it comes to testing. But it looks like your code is a perfect example on non-test friendly code. And is why SRP, single responsibility principle is such a big thing.

From my perspective, your function is doing two things:

  1. Making path.
  2. Ensuring path exists.

If you change your code so that those are two distinct functions than you can test more easily. And finally have get as a helper function.

def generate_path(base, timestamp, channel):
    return (
        pathlib.Path(base)
        / "tofu"
        / timestamp.strftime("%Y%m%d")
        / f"{channel:>02d}"
        / "quinoa.txt"
    )


def ensure_exists(path):
    if path.exists():
        print(path, "exists")
    else:
        path.parent.mkdir(exist_ok=True, parents=True)


def get(base, timestamp, channel):
    path = generate_path()
    ensure_exists(path)
    return path

I believe someone more versed in testing would be able to say something more enlightening about what you should unittest and what you should mock. But from my POV I don't see why the implementation details of generate_path should matter, all that matters is you get the correct result. The implementation details of ensure_exists, however, do matter, as it can be hard to test file interactions. But with etene's changes this statement can suddenly become false.

Overall I agree with etene's answer, but I think my answer should help when designing your code for testing.

import pathlib
import unittest.mock
import datetime


def generate_path(base, timestamp, channel):
    return (
        pathlib.Path(base)
        / timestamp.strftime("tofu/%Y%m%d")
        / f"{channel:>02d}"
        / "quinoa.txt"
    )


def ensure_exists(path):
    if path.exists():
        print(path, "exists")
    else:
        path.parent.mkdir(exist_ok=True, parents=True)
    return path


class Tests(unittest.TestCase):
    BASE = "quorn"
    TIME = datetime.datetime(2020, 2, 11, 0, 0, 0, 0)
    CHANNEL = 42
    PATH = pathlib.Path("quorn/tofu/20200211/42/quinoa.txt")

    def test_generate_path(self):
        self.assertEqual(
            generate_path(self.BASE, self.TIME, self.CHANNEL),
            self.PATH,
        )

    @unittest.mock.patch("pathlib.Path", autospec=True)
    def test_ensure_exists(self, pP):
        pP.exists.return_value = False
        ensure_exists(pP)
        pP.parent.mkdir.assert_called_once()


if __name__ == '__main__':
    unittest.main()
\$\endgroup\$
2
  • \$\begingroup\$ I wholeheartedly agree with your suggestion of refactoring the code to make it more testable. "Do one thing and do it well", as goes the saying. I just didn't think of this angle when writing my answer. \$\endgroup\$ – etene Feb 11 '20 at 13:36
  • 1
    \$\begingroup\$ Don't sweat it @etene I didn't think of your solution either ;P Things like this are what make the site great IMO \$\endgroup\$ – Peilonrayz Feb 11 '20 at 14:20
5
\$\begingroup\$

Mocking system calls, especially those with side effects (mkdir()), is difficult and error prone.

I would rather use a temporary directory for test purposes. This approach has the added advantage of really testing your FS manipluation code.

import datetime
import os
import pathlib

from contextlib import contextmanager
from tempfile import TemporaryDirectory


@contextmanager
def temporary_test_dir():
    oldpwd = os.getcwd()
    with TemporaryDirectory("test-path") as td:
        try:
            os.chdir(td)
            yield
        finally:
            os.chdir(oldpwd)


def get(base, timestamp, channel):
    base = pathlib.Path(base)
    path = base / "tofu" / timestamp.strftime("%Y%m%d") / f"{channel:>02d}" / "quinoa.txt"
    if path.exists():
        print(path, "exists")
    else:
        path.parent.mkdir(exist_ok=True, parents=True)
    return path


@temporary_test_dir()
def test_get():
    # All relative FS operations will be made in a temporary directory
    result = get("quorn", datetime.datetime.now(), 42)
    assert result.parent.exists()

I'm not sure what is the point of the tested function, but it doesn't matter for this example's sake.

If you really need to mock the filesystem part, I would suggest using an existing library such as pyfakefs, although I have never had such a need.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you for posting this as an answer. With added details to boot! :) \$\endgroup\$ – Peilonrayz Feb 11 '20 at 11:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.