Testing strategy for my function using mock

Question

I have a rather complicated (for me anyway) set of functions that I copied from another project (specifically, from subuser). These functions do some checks on the system for the presence and status of a given binary. They work as is but I really want to write proper tests for them in my project. I'm using python 3.4 and unittest.mock for this. So in my checks.py module, I have these functions:

UPDATE: Changed some style items in function naming in final test code, see below.

import os

def is_executable(fpath):
    '''
    Returns true if the given filepath points to an executable file.
    '''
    return os.path.isfile(fpath) and os.access(fpath, os.X_OK)


# Origonally taken from: http://stackoverflow.com/questions/377017/test-if-executable-exists-in-python
def query_path(test):
    '''
    Search the PATH for an executable.

    Given a function which takes an absolute filepath and returns True when the
    filepath matches the query, return a list of full paths to matched files.
    '''
    matches = []

    def append_if_matches(exeFile):
        if is_executable(exeFile):
            if test(exeFile):
                matches.append(exeFile)

    for path in os.environ['PATH'].split(os.pathsep):
        path = path.strip('"')
        if os.path.exists(path):
            for fileInPath in os.listdir(path):
                exeFile = os.path.join(path, fileInPath)
                append_if_matches(exeFile)

    return matches


def which(program):
    '''
    Check for existence and executable state of program.
    '''
    fpath, fname = os.path.split(program)

    if not fpath == '':
        if is_executable(program):
            return program
    else:
        def matches_program(path):
            fpath, fname = os.path.split(path)
            return program == fname
    programMatches = query_path(matches_program)
    if len(programMatches) > 0:
        return programMatches[0]

    return None

And these perform well, they will check the PATH for a binary, see if it's executable, and return the first result. Basically recreating the Linux 'which' command.

My testing module so far looks like this:

NOTE: excuse different function-name style, updated in final result, see below.

import unittest
import unittest.mock as mock

from myproject import checks


class TestSystemChecks(unittest.TestCase):

    def setUp(self):
        pass

    def tearDown(self):
        pass

    # This test works great
    @mock.patch('os.path.isfile')
    @mock.patch('os.access')
    def test_isExecutable(self, mock_isfile, mock_access):
        # case 1
        mock_isfile.return_value = True
        mock_access.return_value = True

        self.assertTrue(
            checks.isExecutable('/some/executable/file'))

        # case 2
        mock_isfile.return_value = True
        mock_access.return_value = False

        self.assertFalse(
            checks.isExecutable('/some/non/executable/file'))

    # THIS ONE IS A MESS.
    @mock.patch('os.path.isfile')
    @mock.patch('os.path.exists')
    @mock.patch('os.access')
    @mock.patch('os.listdir')
    @mock.patch('os.environ')
    def test_queryPATH(
            self, mock_isfile, mock_access, mock_environ, mock_exists,
            mock_listdir):
        # case 1
        mock_isfile.return_value = True
        mock_access.return_value = True
        mock_exists.return_value = True
        mock_listdir.return_value = [
            'somebin',
            'another_bin',
            'docker']
        mock_environ.dict['PATH'] = \
            '/wrong:' +\
            '/wrong/path/two:' +\
            '/docker/path/one:' +\
            '/other/docker/path'
        target_paths = [
            '/docker/path/one/docker',
            '/other/docker/path/docker']

        def isPathToDockerCommand(path):
            return True

        self.assertEqual(
            target_paths,
            checks.queryPATH(isPathToDockerCommand))

    def test_which(self):
        pass

So the test for queryPATH() is my question here. Am I attempting to do too much in one function? Do I really need to recreate all these mock objects every time or is there a way to setup a meta-object (or set of objects) in setUp() for all these tests? Or, perhaps I still don't understand how the original code works and am just not setting up my test correctly (but the use of mock objects is correct). The result of running this test yields:

checks.queryPATH(isPathToDockerCommand))
AssertionError: Lists differ: ['/docker/path/one/docker', '/other/docker/path/docker'] != []

First list contains 2 additional elements.
First extra element 0:
/docker/path/one/docker
- ['/docker/path/one/docker', '/other/docker/path/docker']
+ []

Because of the complexity of the test and the function itself, I'm not sure why I can't design my test right. This is the first I'm extensively using mock in my unit testing and want to get it right before I proceed with my project so I can code TDD style from the get-go. Thanks!

UPDATE: SOLVED

Here is what my final result ended up looking like in all of it's glory for these three functions.

import unittest
import unittest.mock as mock

from myproject import checks

class TestSystemChecks(unittest.TestCase):

    def setUp(self):
        pass

    def tearDown(self):
        pass

    @mock.patch('os.access')
    @mock.patch('os.path.isfile')
    def test_is_executable(self,
                           mock_isfile,
                           mock_access):

        # case 1
        mock_isfile.return_value = True
        mock_access.return_value = True

        self.assertTrue(
            checks.is_executable('/some/executable/file'))

        # case 2
        mock_isfile.return_value = True
        mock_access.return_value = False

        self.assertFalse(
            checks.is_executable('/some/non/executable/file'))

    @mock.patch('os.listdir')
    @mock.patch('os.access')
    @mock.patch('os.path.exists')
    @mock.patch('os.path.isfile')
    def test_query_path(self,
                        mock_isfile,
                        mock_exists,
                        mock_access,
                        mock_listdir):
        # case 1
        # assume file exists, and is in all paths supplied
        mock_isfile.return_value = True
        mock_access.return_value = True
        mock_exists.return_value = True
        mock_listdir.return_value = ['docker']

        fake_path = '/path/one:' +\
                    '/path/two'

        def is_path_to_docker_command(path):
            return True

        with mock.patch.dict('os.environ', {'PATH': fake_path}):
            self.assertEqual(
                ['/path/one/docker', '/path/two/docker'],
                checks.query_path(is_path_to_docker_command))

        # case 2
        # assume file exists, but not in any paths
        mock_isfile.return_value = True
        mock_access.return_value = True
        mock_exists.return_value = False
        mock_listdir.return_value = ['docker']

        fake_path = '/path/one:' +\
                    '/path/two'

        def is_path_to_docker_command(path):
            return True

        with mock.patch.dict('os.environ', {'PATH': fake_path}):
            self.assertEqual(
                [],
                checks.query_path(is_path_to_docker_command))

        # case 3
        # assume file does not exist
        mock_isfile.return_value = False
        mock_access.return_value = False
        mock_exists.return_value = False
        mock_listdir.return_value = ['']

        fake_path = '/path/one:' +\
                    '/path/two'

        def is_path_to_docker_command(path):
            return True

        with mock.patch.dict('os.environ', {'PATH': fake_path}):
            self.assertEqual(
                [],
                checks.query_path(is_path_to_docker_command))

    @mock.patch('os.listdir')
    @mock.patch('os.access')
    @mock.patch('os.path.exists')
    @mock.patch('os.path.isfile')
    def test_which(self,
                   mock_isfile,
                   mock_exists,
                   mock_access,
                   mock_listdir):

        # case 1
        # file exists, only take first result
        mock_isfile.return_value = True
        mock_access.return_value = True
        mock_exists.return_value = True
        mock_listdir.return_value = ['docker']

        fake_path = '/path/one:' +\
                    '/path/two'

        with mock.patch.dict('os.environ', {'PATH': fake_path}):
            self.assertEqual(
                '/path/one/docker',
                checks.which('docker'))

        # case 2
        # file does not exist
        mock_isfile.return_value = True
        mock_access.return_value = True
        mock_exists.return_value = False
        mock_listdir.return_value = ['']

        fake_path = '/path/one:' +\
                    '/path/two'

        with mock.patch.dict('os.environ', {'PATH': fake_path}):
            self.assertEqual(
                None,
                checks.which('docker'))

Comments on @robjohncox points:

1. Order or decorator matters as he stated in his answer.
2. Patching a dictionary patch.dict using a decorator curiously doesn't need to pass any objects into the function as an argument like the other decorators. It must just modify the dict at the source or something.
3. For the sake of my tests, I decided to use the with method of changing context instead of the decorator so that I could easily test different cases using different paths.

Answer 1

On the question of "am I attempting to do too much in one function", I think the answer is no, you are testing a single unit here and it looks impossible to break the complexity down further - the complexity of the test is simply a result of the complex environmental setup required for your function. In fact I applaud your effort, most would look at that function and think "too hard, let's not bother with the tests".

With regards to what might be causing your tests to fail, there are two things that jump out:

Ordering of mock arguments in test method signatures: You must be careful of the positions of each mock in the signature, the mock objects will be passed in by the mocking framework in the reverse order to which you declare them in the decorators, for example:

@mock.patch('function.one')
@mock.patch('function.two')
def test_something(self, mock_function_two, mock_function_one):
    <test code>

I see in each of your functions you don't have the mock parameters in the correct order (although it works for your first example, test_isExecutable, because both mock return values are True.

Mocking the PATH in the environment dictionary: I don't think the approach taken there will work for mocking os.environ, as the way it is being set up I don't think will return what you expect when os.environ['PATH'] is called in the code under test (though I may be wrong). Fortunately, mock should have you covered here with the @mock.patch.dict decorator, shown below. This, combined with putting the mock arguments in the right order, should result in something like:

fake_path = '/wrong:' +\
            '/wrong/path/two:' +\
            '/docker/path/one:' +\
            '/other/docker/path'

@mock.patch.dict('os.environ', {'PATH': fake_path})
@mock.patch('os.listdir')
@mock.patch('os.access')
@mock.patch('os.path.exists')
@mock.patch('os.path.isfile')
def test_queryPATH(self,
                   mock_isfile,
                   mock_exists,
                   mock_access,
                   mock_listdir,
                   mock_environ):

    mock_isfile.return_value = True
    mock_access.return_value = True
    mock_exists.return_value = True
    mock_listdir.return_value = [
        'somebin',
        'another_bin',
        'docker']

    target_paths = [
        '/docker/path/one/docker',
        '/other/docker/path/docker']

    def isPathToDockerCommand(path):
        return True

    self.assertEqual(
        target_paths,
        checks.queryPATH(isPathToDockerCommand))

Disclaimer: I haven't actually tested this code, so please take it more as a guideline than a guaranteed working solution, but hopefully it can help you along in the right direction.