How to get DRYer tests for simple function using python glob?

Question

I have a function that searches for a file in current location, then in Download folder and if not found, raises and error. Using pytest and pytest-mock, I was able to test the code with a code much bigger than the tested one. Is there a way to make this tighter/DRYer?

Tested code:

# cei.py
import glob
import os

def get_xls_filename() -> str:
    """ Returns first xls filename in current folder or Downloads folder """
    csv_filenames = glob.glob("InfoCEI*.xls")
    if csv_filenames:
        return csv_filenames[0]
    home = os.path.expanduser("~")
    csv_filenames = glob.glob(home + "/Downloads/InfoCEI*.xls")
    if csv_filenames:
        return csv_filenames[0]
    return sys.exit(
        "Error: file not found."
    )

There are three test scenarios here. Found in current, found in downloads and not found. Test code:

# test_cei.py
from unittest.mock import Mock

import pytest
from pytest_mock import MockFixture

import cei

@pytest.fixture
def mock_glob_glob_none(mocker: MockFixture) -> Mock:
    """Fixture for mocking glob.glob."""
    mock = mocker.patch("glob.glob")
    mock.return_value = []
    return mock

@pytest.fixture
def mock_os_path_expanduser(mocker: MockFixture) -> Mock:
    """Fixture for mocking os.path.expanduser."""
    mock = mocker.patch("os.path.expanduser")
    mock.return_value = "/home/user"
    return mock

def test_get_xls_filename_not_found(mock_glob_glob_none, mock_os_path_expanduser) -> None:
    with pytest.raises(SystemExit):
        assert cei.get_xls_filename()
        mock_glob_glob_none.assert_called()
        mock_os_path_expanduser.assert_called_once()

@pytest.fixture
def mock_glob_glob_found(mocker: MockFixture) -> Mock:
    """Fixture for mocking glob.glob."""
    mock = mocker.patch("glob.glob")
    mock.return_value = ["/my/path/InfoCEI.xls"]
    return mock

def test_get_xls_filename_current_folder(mock_glob_glob_found) -> None:
    assert cei.get_xls_filename() == "/my/path/InfoCEI.xls"
    mock_glob_glob_found.assert_called_once()

@pytest.fixture
def mock_glob_glob_found_download(mocker: MockFixture) -> Mock:
    """Fixture for mocking glob.glob."""
    values = {
        "InfoCEI*.xls": [],
        "/home/user/Downloads/InfoCEI*.xls": ["/home/user/Downloads/InfoCEI.xls"],
    }

    def side_effect(arg):
        return values[arg]
    mock = mocker.patch("glob.glob")
    mock.side_effect = side_effect
    return mock


def test_get_xls_filename_download_folder(
    mock_glob_glob_found_download, mock_os_path_expanduser
) -> None:
    assert cei.get_xls_filename() == "/home/user/Downloads/InfoCEI.xls"
    mock_os_path_expanduser.assert_called_once()
    mock_glob_glob_found_download.assert_called_with(
        "/home/user/Downloads/InfoCEI*.xls"
    )

Answer 1

This is obviously a bit opinion-based, but I'll try.

First, there is nothing wrong with the tests being larger than the tested code. Depending on the number of tested use cases, this can easily happen, and I wouldn't use this a criterion for test quality.

That being said, your tests shall usually test the API / interface, which in this case is the returned file path under different conditions. Testing if os.path.expanduser has been called is part of the internal implementation that may not be stable - I would not consider that a good thing at least in this case. You already test the most relevant use cases (a test for having files in both locations might be added), where these internals are used.

Here is what I would probably do:

import os
import pytest
from cei import get_xls_filename


@pytest.fixture
def cwd(fs, monkeypatch):
    fs.cwd = "/my/path"
    monkeypatch.setenv("HOME", "/home/user")


def test_get_xls_filename_not_found(fs, cwd) -> None:
    with pytest.raises(SystemExit):
        assert get_xls_filename()


def test_get_xls_filename_current_folder(fs, cwd) -> None:
    fs.create_file("/my/path/InfoCEI.xls")
    assert get_xls_filename() == "InfoCEI.xls"  # adapted to your implementation


def test_get_xls_filename_download_folder(fs, cwd) -> None:
    path = os.path.join("/home/user", "Downloads", "InfoCEI.xls")
    fs.create_file(path)
    assert get_xls_filename() == path

Note that I used the pyfakefs fixture fs to mock the fs (I'm a contributor to pyfakefs, so this is what I'm used to, and it makes the code a bit shorter), but this may be overkill for you.

Basically, I try to test only the API, put the common setup (here cwd and home path location) into a fixture (or in a setup method for xUnit-like tests), and add the test-specific setup (creation of the test file) to the test itself.