Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python3 + pytest + pytest-mock: Mocks leaking into other test functions breaking assertions? #84

Closed
bossjones opened this issue Apr 29, 2017 · 15 comments

Comments

@bossjones
Copy link

bossjones commented Apr 29, 2017

Hi guys! First of all, thank you for everything that you do! It is really appreciated!

My Question

Apologies in advance if this issue is blatant, but i've been wrestling with it for several days now. Hopefully someone can shed some new light.

I'm in the process of converting unit tests for my personal project from unittest -> pytest. Previously I was using the built-in unittest.mock module, but now i'm trying to use the pytest-mock plugin instead.

I have a sneaking feeling that my tests are leaking mock objects into one another.

Here's why:
NOTE: All Details about my setup (python version, modules etc) listed at bottom of question.

High-level details:

# Python version
Python 3.5.2

# Pytest version ( and plugins )
pytest==3.0.7
pytest-benchmark==3.1.0a2
pytest-catchlog==1.2.2
pytest-cov==2.4.0
pytest-ipdb==0.1.dev2
pytest-leaks==0.2.2
pytest-mock==1.6.0
pytest-rerunfailures==2.1.0
pytest-sugar==0.8.0
pytest-timeout==1.2.0
python-dateutil==2.6.0
python-dbusmock==0.16.7

When I run my tests using the following command:

py.test --pdb --showlocals -v -R : -k test_subprocess.py

Everything is fine till we get to test_subprocess_check_command_type. At which point I get the following error:

            # Set mock return types
            # mock_map_type_to_command.return_value = int
    
            # action
            with pytest.raises(TypeError) as excinfo:
                scarlett_os.subprocess.Subprocess(test_command,
                                                  name=test_name,
                                                  fork=test_fork,
    >                                             run_check_command=True)
    E           Failed: DID NOT RAISE <class 'TypeError'>
    
    excinfo    = <[AttributeError("'ExceptionInfo' object has no attribute 'typename'") raised in repr()] ExceptionInfo object at 0x7f8c380f9dc0>
    mock_fork  = <Mock name='mock_fork' id='140240122195184'>
    mock_logging_debug = <Mock name='mock_logging_debug' id='140240128747640'>
    mock_map_type_to_command = <Mock name='mock_map_type_to_command' id='140240122785112'>
    mocker     = <pytest_mock.MockFixture object at 0x7f8c329f07a8>
    monkeypatch = <_pytest.monkeypatch.MonkeyPatch object at 0x7f8c329f0810>
    self       = <tests.test_subprocess.TestScarlettSubprocess object at 0x7f8c32aaac20>
    test_command = ['who', '-b']
    test_fork  = False
    test_name  = 'test_who'
    
    tests/test_subprocess.py:267: Failed
    
     tests/test_subprocess.py::TestScarlettSubprocess.test_subprocess_check_command_type ⨯                                                           100% ██████████

BUT!

If I filter out all of the other tests except for the problematic one then I get:

via py.test --pdb --showlocals -v -R : -k test_subprocess_check_command_type

    pi@0728af726f1f:~/dev/bossjones-github/scarlett_os$ py.test --pdb --showlocals -v -R : -k test_subprocess_check_command_type
    /usr/local/lib/python3.5/site-packages/_pdbpp_path_hack/pdb.py:4: ResourceWarning: unclosed file <_io.TextIOWrapper name='/usr/local/lib/python3.5/site-packages/pdb.py' mode='r' encoding='UTF-8'>
      os.path.dirname(os.path.dirname(__file__)), 'pdb.py')).read(), os.path.join(
    Test session starts (platform: linux, Python 3.5.2, pytest 3.0.7, pytest-sugar 0.8.0)
    cachedir: .cache
    benchmark: 3.1.0a2 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
    rootdir: /home/pi/dev/bossjones-github/scarlett_os, inifile: setup.cfg
    plugins: timeout-1.2.0, sugar-0.8.0, rerunfailures-2.1.0, mock-1.6.0, leaks-0.2.2, ipdb-0.1.dev2, cov-2.4.0, catchlog-1.2.2, benchmark-3.1.0a2
    timeout: 60.0s method: signal
    NOTE: DBUS_SESSION_BUS_ADDRESS environment var not found!
    [DBUS_SESSION_BUS_ADDRESS]: unix:path=/tmp/dbus_proxy_outside_socket
    
     tests/test_subprocess.py::TestScarlettSubprocess.test_subprocess_check_command_type ✓                                                                                                                                                                           100% ██████████
    
    Results (8.39s):
           1 passed
         190 deselected
    pi@0728af726f1f:~/dev/bossjones-github/scarlett_os$

I also tried manually commenting out the following 2 tests and they allowed me to successfully run all the tests again:

  • test_subprocess_init
  • test_subprocess_map_type_to_command

Can anyone see anything blatently wrong with my setup? I've read several blog posts on "where to mock", and looked at the docs themselves several times, not sure what i'm missing. https://docs.python.org/3/library/unittest.mock.html

My Setup Details

Here is everything that might be required to solve this. Let me know if I need to provide any more information!

Also ... please excuse how messy my code looks and all of the comment blocks. I'm a big note taker when i'm learning something new ... I'll make everything more pythonic and cleaner in the near future :)

My code:

    #!/usr/bin/env python
    # -*- coding: utf-8 -*-

    """Scarlett Dbus Service. Implemented via MPRIS D-Bus Interface Specification."""

    from __future__ import with_statement, division, absolute_import

    import os
    import sys
    from scarlett_os.exceptions import SubProcessError
    from scarlett_os.exceptions import TimeOutError
    import logging
    from scarlett_os.internal.gi import GObject
    from scarlett_os.internal.gi import GLib

    logger = logging.getLogger(__name__)


    def check_pid(pid):
        """Check For the existence of a unix pid."""
        try:
            os.kill(pid, 0)
        except OSError:
            return False
        else:
            return True


    class Subprocess(GObject.GObject):
        """
        GObject API for handling child processes.

        :param command: The command to be run as a subprocess.
        :param fork: If `True` this process will be detached from its parent and
                     run independent. This means that no excited-signal will be emited.

        :type command: `list`
        :type fork: `bool`
        """

        __gtype_name__ = 'Subprocess'
        __gsignals__ = {
            'exited': (GObject.SignalFlags.RUN_LAST, None, (GObject.TYPE_INT, GObject.TYPE_INT))
        }

        def __init__(self, command, name=None, fork=False, run_check_command=True):
            """Create instance of Subprocess."""

            GObject.GObject.__init__(self)

            self.process = None
            self.pid = None

            if not fork:
                self.stdout = True
                self.stderr = True
            else:
                self.stdout = False
                self.stderr = False

            self.forked = fork

            # Verify that command is properly formatted 
            # and each argument is of type str
            if run_check_command:
                self.check_command_type(command)

            self.command = command
            self.name = name

            logger.debug("command: {}".format(self.command))
            logger.debug("name: {}".format(self.name))
            logger.debug("forked: {}".format(self.forked))
            logger.debug("process: {}".format(self.process))
            logger.debug("pid: {}".format(self.pid))

            if fork:
                self.fork()

        # TODO: Add these arguments so we can toggle stdout
        # def spawn_command(self, standard_input=False, standard_output=False, standard_error=False):
        def spawn_command(self):
            # DO_NOT_REAP_CHILD
            # Don't reap process automatically so it is possible to detect when it is closed.
            return GLib.spawn_async(self.command,
                                    flags=GLib.SpawnFlags.SEARCH_PATH | GLib.SpawnFlags.DO_NOT_REAP_CHILD
                                    )

        def map_type_to_command(self, command):
            """Return: Map after applying type to several objects in an array"""
            # NOTE: In python3, many processes that iterate over iterables return iterators themselves. 
            # In most cases, this ends up saving memory, and should make things go faster.
            # cause of that, we need to call list() over the map object
            return list(map(type, command))

        def check_command_type(self, command):

            types = self.map_type_to_command(command)

            if type(types) is not list:
                raise TypeError("Variable types should return a list in python3. Got: {}".format(types))

            # NOTE: str is a built-in function (actually a class) which converts its argument to a string. 
            # string is a module which provides common string operations.
            # source: http://stackoverflow.com/questions/2026038/relationship-between-string-module-and-str
            for t in types:
                if t is not str:
                    raise TypeError("Executables and arguments must be str objects. types: {}".format(t))

            logger.debug("Running Command: %r" % " ".join(command))
            return True

        def run(self):
            """Run the process."""

            # NOTE: DO_NOT_REAP_CHILD: the child will not be automatically reaped;
            # you must use g_child_watch_add yourself (or call waitpid or handle `SIGCHLD` yourself),
            # or the child will become a zombie.
            # source:
            # http://valadoc.org/#!api=glib-2.0/GLib.SpawnFlags.DO_NOT_REAP_CHILD

            # NOTE: SEARCH_PATH: argv[0] need not be an absolute path, it will be looked for in the user's PATH
            # source:
            # http://lazka.github.io/pgi-docs/#GLib-2.0/flags.html#GLib.SpawnFlags.SEARCH_PATH

            self.pid, self.stdin, self.stdout, self.stderr = self.spawn_command()

            logger.debug("command: {}".format(self.command))
            logger.debug("stdin: {}".format(self.stdin))
            logger.debug("stdout: {}".format(self.stdout))
            logger.debug("stderr: {}".format(self.stderr))
            logger.debug("pid: {}".format(self.pid))

            # close file descriptor
            self.pid.close()

            print(self.stderr)

            # NOTE: GLib.PRIORITY_HIGH = -100
            # Use this for high priority event sources.
            # It is not used within GLib or GTK+.
            watch = GLib.child_watch_add(GLib.PRIORITY_HIGH, 
                                         self.pid, 
                                         self.exited_cb)

            return self.pid

        def exited_cb(self, pid, condition):
            if not self.forked:
                self.emit('exited', pid, condition)

        def fork(self):
            """Fork the process."""
            try:
                # first fork
                pid = os.fork()
                if pid > 0:
                    logger.debug('pid greater than 0 first time')
                    sys.exit(0)
            except OSError as e:
                logger.error('Error forking process first time')
                sys.exit(1)

            # Change the current working directory to path.
            os.chdir("/")

            # Description: setsid() creates a new session if the calling process is not a process group leader. 
            # The calling process is the leader of the new session, 
            # the process group leader of the new process group, 
            # and has no controlling terminal. 
            # The process group ID and session ID of the calling process are set to the PID of the calling process. 
            # The calling process will be the only process in this new process group and in this new session.

            # Return Value: On success, the (new) session ID of the calling process is returned. 
            # On error, (pid_t) -1 is returned, and errno is set to indicate the error.
            os.setsid()

            # Set the current numeric umask and return the previous umask.
            os.umask(0)

            try:
                # second fork
                pid = os.fork()
                if pid > 0:
                    logger.debug('pid greater than 0 second time')
                    sys.exit(0)
            except OSError as e:
                logger.error('Error forking process second time')
                sys.exit(1)

My Test:

    #!/usr/bin/env python
    # -*- coding: utf-8 -*-

    """
    test_subprocess
    ----------------------------------
    """

    import os
    import sys
    import pytest

    import scarlett_os
    # import signal
    # import builtins
    # import re

    class TestScarlettSubprocess(object):
        '''Units tests for Scarlett Subprocess, subclass of GObject.Gobject.'''

        def test_check_pid_os_error(self, mocker):
            # Feels like mocks are leaking into other tests, 
            # stop mock before starting each test function
            mocker.stopall()

            # Setup mock objects
            kill_mock = mocker.MagicMock(name=__name__ + "_kill_mock_OSError")
            kill_mock.side_effect = OSError

            # patch things
            mocker.patch.object(scarlett_os.subprocess.os, 'kill', kill_mock)

            # When OSError occurs, throw False
            assert not scarlett_os.subprocess.check_pid(4353634632623)
            # Verify that os.kill only called once
            assert kill_mock.call_count == 1

        def test_check_pid(self, mocker):
            # Feels like mocks are leaking into other tests, 
            # stop mock before starting each test function
            mocker.stopall()

            # Setup mock objects
            kill_mock = mocker.MagicMock(name=__name__ + "_kill_mock")

            mocker.patch.object(scarlett_os.subprocess.os, 'kill', kill_mock)

            result = scarlett_os.subprocess.check_pid(123)
            assert kill_mock.called
            # NOTE: test against signal 0
            # sending the signal 0 to a given PID just checks if any
            # process with the given PID is running and you have the
            # permission to send a signal to it.
            kill_mock.assert_called_once_with(123, 0)
            assert result is True

        # FIXME: I THINK THIS GUYS IS LEAKING MOCK OBJECTS
        def test_subprocess_init(self, mocker):
            # Feels like mocks are leaking into other tests, 
            # stop mock before starting each test function
            mocker.stopall()

            mock_check_command_type = MagicMock(name="mock_check_command_type")
            mock_check_command_type.return_value = True
            mock_fork = mocker.MagicMock(name="mock_fork")
            mock_logging_debug = mocker.MagicMock(name="mock_logging_debug")

            # mock
            mocker.patch.object(scarlett_os.subprocess.logging.Logger, 'debug', mock_logging_debug)
            mocker.patch.object(scarlett_os.subprocess.Subprocess, 'check_command_type', mock_check_command_type)
            mocker.patch.object(scarlett_os.subprocess.Subprocess, 'fork', mock_fork)

            # NOTE: On purpose this is an invalid cmd. Should be of type array
            test_command = ['who']

            test_name = 'test_who'
            test_fork = False

            s_test = scarlett_os.subprocess.Subprocess(test_command,
                                                       name=test_name,
                                                       fork=test_fork)

            # action
            assert s_test.check_command_type(test_command) is True
            mock_check_command_type.assert_called_with(['who'])
            assert not s_test.process
            assert not s_test.pid
            assert s_test.name == 'test_who'
            assert not s_test.forked
            assert s_test.stdout is True
            assert s_test.stderr is True

            mock_logging_debug.assert_any_call("command: ['who']")
            mock_logging_debug.assert_any_call("name: test_who")
            mock_logging_debug.assert_any_call("forked: False")
            mock_logging_debug.assert_any_call("process: None")
            mock_logging_debug.assert_any_call("pid: None")
            mock_fork.assert_not_called()

        # FIXME: I THINK THIS GUYS IS LEAKING MOCK OBJECTS
        def test_subprocess_map_type_to_command(self, mocker):
            """Using the mock.patch decorator (removes the need to import builtins)"""
            # Feels like mocks are leaking into other tests, 
            # stop mock before starting each test function
            mocker.stopall()

            mock_check_command_type = mocker.MagicMock(name="mock_check_command_type")
            mock_check_command_type.return_value = True
            mock_fork = mocker.MagicMock(name="mock_fork")
            mock_logging_debug = mocker.MagicMock(name="mock_logging_debug")

            # mock
            mocker.patch.object(scarlett_os.subprocess.logging.Logger, 'debug', mock_logging_debug)
            mocker.patch.object(scarlett_os.subprocess.Subprocess, 'check_command_type', mock_check_command_type)
            mocker.patch.object(scarlett_os.subprocess.Subprocess, 'fork', mock_fork)

            # NOTE: On purpose this is an invalid cmd. Should be of type array
            test_command = ["who", "-b"]
            test_name = 'test_who'
            test_fork = False

            # create subprocess object
            s_test = scarlett_os.subprocess.Subprocess(test_command,
                                                       name=test_name,
                                                       fork=test_fork)
            mocker.spy(s_test, 'map_type_to_command')
            assert isinstance(s_test.map_type_to_command(test_command), list)
            assert s_test.map_type_to_command.call_count == 1

            assert s_test.check_command_type(test_command)
            assert s_test.check_command_type(
                test_command) == mock_check_command_type.return_value

        def test_subprocess_check_command_type(self, mocker):
            """Using the mock.patch decorator (removes the need to import builtins)"""
            # Feels like mocks are leaking into other tests, 
            # stop mock before starting each test function
            mocker.stopall()

            test_command = ["who", "-b"]
            test_name = 'test_who'
            test_fork = False

            # mock
            mock_map_type_to_command = mocker.MagicMock(name="mock_map_type_to_command")
            # mock_map_type_to_command.return_value = int
            mock_map_type_to_command.side_effect = [int, [int, int]]
            mock_fork = mocker.MagicMock(name="mock_fork")
            mock_logging_debug = mocker.MagicMock(name="mock_logging_debug")

            mocker.patch.object(scarlett_os.subprocess.logging.Logger, 'debug', mock_logging_debug)
            mocker.patch.object(scarlett_os.subprocess.Subprocess, 'map_type_to_command', mock_map_type_to_command)
            mocker.patch.object(scarlett_os.subprocess.Subprocess, 'fork', mock_fork)


            # action
            with pytest.raises(TypeError) as excinfo:
                scarlett_os.subprocess.Subprocess(test_command,
                                                  name=test_name,
                                                  fork=test_fork,
                                                  run_check_command=True)
            assert str(
                excinfo.value) == "Variable types should return a list in python3. Got: <class 'int'>"

            with pytest.raises(TypeError) as excinfo:
                scarlett_os.subprocess.Subprocess(test_command,
                                                  name=test_name,
                                                  fork=test_fork,
                                                  run_check_command=True)

            assert str(
                excinfo.value) == "Executables and arguments must be str objects. types: <class 'int'>"

My folder structure( Note I removed a couple things since it was overly verbose ):

    pi@0728af726f1f:~/dev/bossjones-github/scarlett_os$ tree -I *.pyc
    .
    ├── requirements_dev.txt
    ├── requirements_test_experimental.txt
    ├── requirements_test.txt
    ├── requirements.txt
    ├── scarlett_os
    │   ├── automations
    │   │   ├── __init__.py
    │   │   └── __pycache__
    │   ├── commands.py
    │   ├── compat.py
    │   ├── config.py
    │   ├── const.py
    │   ├── core.py
    │   ├── emitter.py
    │   ├── exceptions.py
    │   ├── __init__.py
    │   ├── internal
    │   │   ├── debugger.py
    │   │   ├── deps.py
    │   │   ├── encoding.py
    │   │   ├── formatting.py
    │   │   ├── gi.py
    │   │   ├── __init__.py
    │   │   ├── path.py
    │   │   ├── __pycache__
    │   │   └── system_utils.py
    │   ├── listener.py
    │   ├── loader.py
    │   ├── logger.py
    │   ├── log.py
    │   ├── __main__.py
    │   ├── mpris.py
    │   ├── player.py
    │   ├── __pycache__
    │   ├── receiver.py
    │   ├── speaker.py
    │   ├── subprocess.py
    │   ├── tasker.py
    │   ├── tools
    │   │   ├── __init__.py
    │   │   ├── package.py
    │   │   ├── __pycache__
    │   │   └── verify.py
    │   └── utility
    │       ├── audio.py
    │       ├── dbus_runner.py
    │       ├── dbus_utils.py
    │       ├── distance.py
    │       ├── dt.py
    │       ├── file.py
    │       ├── generators.py
    │       ├── gnome.py
    │       ├── __init__.py
    │       ├── location.py
    │       ├── __pycache__
    │       ├── temperature.py
    │       ├── threadmanager.py
    │       ├── thread.py
    │       ├── unit_system.py
    │       └── yaml.py
    ├── setup.cfg
    ├── setup.py
    ├── tests
    │   ├── common_integration.py
    │   ├── common.py
    │   ├── helpers
    │   │   ├── __init__.py
    │   │   ├── __pycache__
    │   │   ├── test_config_validation.py
    │   │   ├── test_entity.py
    │   │   └── test_init.py
    │   ├── __init__.py
    │   ├── integration
    │   │   ├── baseclass.py
    │   │   ├── conftest.py
    │   │   ├── __init__.py
    │   │   ├── __pycache__
    │   │   ├── README.md
    │   │   ├── stubs.py
    │   │   ├── test_integration_end_to_end.py
    │   │   ├── test_integration_listener.py
    │   │   ├── test_integration_mpris.py
    │   │   ├── test_integration_player.py
    │   │   ├── test_integration_tasker.py
    │   │   ├── test_integration_tasker.py.enable_sound.diff
    │   │   └── test_integration_threadmanager.py
    │   ├── internal
    │   │   ├── __init__.py
    │   │   ├── __pycache__
    │   │   ├── test_deps.py
    │   │   ├── test_encoding.py
    │   │   └── test_path.py
    │   ├── performancetests
    │   │   ├── baseclass.py
    │   │   ├── __init__.py
    │   │   └── __pycache__
    │   ├── __pycache__
    │   ├── run_all_tests
    │   ├── run_dbus_tests.sh
    │   ├── test_cli.py
    │   ├── test_commands.py
    │   ├── testing_config
    │   │   └── custom_automations
    │   │       ├── light
    │   │       │   └── test.py
    │   │       └── switch
    │   │           └── test.py
    │   ├── test_listener.py
    │   ├── test_mpris.py
    │   ├── test_player.py
    │   ├── test_scarlett_os.py
    │   ├── test_speaker.py
    │   ├── test_subprocess.py
    │   ├── test_tasker.py
    │   ├── test_threadmanager.py
    │   ├── tools_common.py
    │   ├── unit_scarlett_os.py
    │   └── utility
    │       ├── __init__.py
    │       ├── __pycache__
    │       ├── test_dbus_utils.py
    │       ├── test_distance.py
    │       ├── test_dt.py
    │       ├── test_gnome.py
    │       ├── test_init.py
    │       ├── test_location.py
    │       ├── test_unit_system.py
    │       └── test_yaml.py
    67 directories, 256 files
    pi@0728af726f1f:~/dev/bossjones-github/scarlett_os$

Other details( Extended pip freeze just in case of incompatibilities ):

    # Python version
    Python 3.5.2

    # Pytest version ( and plugins )
    pytest==3.0.7
    pytest-benchmark==3.1.0a2
    pytest-catchlog==1.2.2
    pytest-cov==2.4.0
    pytest-ipdb==0.1.dev2
    pytest-leaks==0.2.2
    pytest-mock==1.6.0
    pytest-rerunfailures==2.1.0
    pytest-sugar==0.8.0
    pytest-timeout==1.2.0
    python-dateutil==2.6.0
    python-dbusmock==0.16.7


    # Pip Freeze ( Just in case )
    alabaster==0.7.10
    appdirs==1.4.3
    argh==0.26.2
    asn1crypto==0.22.0
    astroid==1.5.2
    Babel==2.4.0
    bleach==2.0.0
    bumpversion==0.5.3
    cffi==1.10.0
    click==6.7
    click-plugins==1.0.3
    colorama==0.3.7
    colorlog==2.10.0
    coverage==4.3.4
    coveralls==1.1
    cryptography==1.8.1
    Cython==0.25.2
    decorator==4.0.11
    docopt==0.6.2
    docutils==0.13.1
    ecdsa==0.13
    entrypoints==0.2.2
    Fabric3==1.12.post1
    fancycompleter==0.7
    fields==5.0.0
    flake8==3.3.0
    flake8-docstrings==1.0.3
    flake8-polyfill==1.0.1
    freezegun==0.3.8
    gnureadline==6.3.3
    graphviz==0.6
    html5lib==0.999999999
    hunter==1.4.1
    idna==2.5
    imagesize==0.7.1
    ipdb==0.10.2
    ipykernel==4.6.1
    ipython==6.0.0
    ipython-genutils==0.2.0
    ipywidgets==6.0.0
    isort==4.2.5
    jedi==0.10.2
    Jinja2==2.9.6
    jsonschema==2.6.0
    jupyter==1.0.0
    jupyter-client==5.0.1
    jupyter-console==5.1.0
    jupyter-core==4.3.0
    lazy-object-proxy==1.2.2
    MarkupSafe==1.0
    mccabe==0.6.1
    mistune==0.7.4
    mock==2.0.0
    mock-open==1.3.1
    mypy-lang==0.4.6
    nbconvert==5.1.1
    nbformat==4.3.0
    notebook==5.0.0
    objgraph==3.1.0
    ordereddict==1.1
    packaging==16.8
    pandocfilters==1.4.1
    paramiko==1.18.2
    pathtools==0.1.2
    pbr==1.10.0
    pdbpp==0.8.3
    pexpect==4.2.1
    pickleshare==0.7.4
    pluggy==0.4.0
    plumbum==1.6.3
    prompt-toolkit==1.0.14
    psutil==5.2.2
    ptyprocess==0.5.1
    py==1.4.33
    py-cpuinfo==3.2.0
    pyasn1==0.2.3
    pycodestyle==2.3.1
    pycparser==2.17
    pycrypto==2.6.1
    pydbus==0.6.0
    pydocstyle==2.0.0
    pyflakes==1.5.0
    pygal==2.3.1
    pygaljs==1.0.1
    Pygments==2.2.0
    pygobject==3.22.0
    pylint==1.7.1
    pyparsing==2.2.0
    pystuck==0.8.5
    pytest==3.0.7
    pytest-benchmark==3.1.0a2
    pytest-catchlog==1.2.2
    pytest-cov==2.4.0
    pytest-ipdb==0.1.dev2
    pytest-leaks==0.2.2
    pytest-mock==1.6.0
    pytest-rerunfailures==2.1.0
    pytest-sugar==0.8.0
    pytest-timeout==1.2.0
    python-dateutil==2.6.0
    python-dbusmock==0.16.7
    pytz==2017.2
    PyYAML==3.12
    pyzmq==16.0.2
    qtconsole==4.3.0
    requests==2.13.0
    requests-mock==1.3.0
    rpyc==3.3.0
    -e git+git@github.com:bossjones/scarlett_os.git@c14ffcde608da12f5c2d4d9b81a63c7e618b3eed#egg=scarlett_os
    simplegeneric==0.8.1
    six==1.10.0
    snowballstemmer==1.2.1
    Sphinx==1.5.5
    stevedore==1.18.0
    termcolor==1.1.0
    terminado==0.6
    testpath==0.3
    tornado==4.5.1
    tox==2.7.0
    traitlets==4.3.2
    typing==3.6.1
    virtualenv==15.0.3
    virtualenv-clone==0.2.6
    virtualenvwrapper==4.7.2
    voluptuous==0.9.3
    watchdog==0.8.3
    wcwidth==0.1.7
    webencodings==0.5.1
    widgetsnbextension==2.0.0
    wmctrl==0.3
    wrapt==1.10.10
    xdot==0.7
@nicoddemus
Copy link
Member

Hi, sorry for not answering earlier, I was out of town.

Looking at the code I don't see anything obvious, but because I can't run the code I can't dig deeper. You think test_subprocess_map_type_to_command is leaking the mock to check_command_type right? Your usage seems OK. You also shouldn't need to call mocker.stopall().

Here some suggestions to try to sort the issue:

  1. Try replacing the usages of the mocker fixture by the builtin unittest.mock module; if it works then it is definitely a bug in pytest-mock and I would be very interested in trying to reproduce and fix this;
  2. Step into the debugger right before the call to scarlett_os.subprocess.Subprocess inside the with pytest.raises block in test_subprocess_check_command_type and see where that lands. You can enable the debugger at that point by adding this line: import pdb; pdb.set_trace() or using an IDE with builtin debugger like PyCharm or PyDev.

Other than that I don't know how to help further, but please keep me posted if you discover anything.

@bossjones
Copy link
Author

bossjones commented May 3, 2017

Hi @nicoddemus ! Your timing is amazing, thank you for responding.

So, I haven't fully fixed things yet( though part of it might be from some weird crap I was trying ), but you're spot on about differences between unittest.mock and the separate mock module. After digging through some older issues and other peoples source code online, I realized I had this setting in my setup.cfg ( again overly verbose so that I would remember what exactly everything does ):

[tool:pytest]
timeout = 60
testpaths = tests
norecursedirs = .git testing_config
addopts =  --cov=scarlett_os --cov-report term-missing
mock_use_standalone_module = True

I set mock_use_standalone_module to False and explicitly imported unittest.mock. That alone didn't fix it though ... but when I added mocker.stopall() to the beginning and end of each test ... the tests passed (all of them!). Crazy right?! I came to the same conclusion as you above re: needing some code to dig deeper. Tonight i'm going to spend some time making a separate repo that "hopefully" reproduces this issue that might make it easier to see what's going on. I'll make sure everything is runnable from Docker so there will be no need to figure out how to setup a "outlier" environment from scratch. Thank you very much for your response! Hopefully i'll be back in a couple hours w/ a github repo of the reproduction :)

@bossjones
Copy link
Author

@nicoddemus okay! Repoduced it in a "simpler" separate repo. Going to copy and paste the readme I made as well here: https://github.com/bossjones/reproduce_pytest_mock_issue_84

If I can help in anyway, please let me know. Maybe this isn't even pytest-mock ?

Readme:


reproduce_pytest_mock_issue_84

reproduce_pytest_mock_issue_84

Disclaimer

I put this repo together pretty quickly, in between the work week in an effort to repoduce the issues I saw while working w/ pytest and pytest-mock for my personal project, scarlett_os. I left a lot of the modules that I originally used in this repo, along w/ some other things that might be a bit unecessary. The purpose of that was to create an environment that was as close as possible to what I'm actually using ... just minus some of the complexity.

Requirements

  • Docker For Mac ( Also tested on fedora 24 in virtualbox )

Specifically i'm using:

$ docker --version
Docker version 1.12.5, build 7392c3b

$ docker-compose --version
docker-compose version 1.9.0, build 2585387

 |2.2.3|   Malcolms-MBP-3 in ~/dev
○ →

Setup

1. Start docker container via docker-compose

make docker-compose

2. Exec into the container using bash

make docker-exec

3. Once in the container, run bootstrap command to install dependencies

# from inside container
make bootstrap

4. Enable virtualenv and run tests

A. How to repoduce error

# from inside container
workon repoduce_pytest_mock_issue_84
make test

B. Run tests w/ mocker.stopall() to fix "leak"

When you set this environment variable, mocker.stopall() runs at the beginning and end of each test case.

# from inside container

workon repoduce_pytest_mock_issue_84
ENABLE_STOPALL=1 make test

@dimaqq
Copy link

dimaqq commented May 9, 2017

Exception text seems very similar to http://programtalk.com/python-examples/qibuild.build.BuildFailed/ example 2 "test_qibuild_make.py". Perhaps there is a bug?

@bossjones
Copy link
Author

bossjones commented May 10, 2017

Very interesting... For the sake of transparency, the comment @dimaqq is referring to is:

def test_codegen_fail_when_generating_command_fails(qibuild_action):
    qibuild_action.add_test_project("codegen")
    qibuild_action("configure", "codegen", "-DFAIL=TRUE")
    # Bug in pytest, with pytest.raises() pytest fails with:
    # ExceptionInfo object has no attribute 'typename'
    # (but only when used with the xdist plugin on Windows)
    try:
        qibuild_action("make", "codegen", "--verbose-make")
        # pylint:disable-msg=E1101
        pytest.fail("Build should have fail")
    except qibuild.build.BuildFailed:
        pass

I definitely have pytest-xdist installed in my environment. Im not leveraging it currently, but would like to in the future.

@lohrmann
Copy link

I'm experiencing the same issue — I'm currently working around it by keeping my tests in reverse order, i.e. the ones testing full functionality first, then those that rely on mocking out certain functionality.

@nicoddemus
Copy link
Member

nicoddemus commented Oct 16, 2017

@lohrmann which issue, the original leaks or the one mentioned by @dimaqq?

About calling mocker.stopall and this fixing the issue, it is all very weird given that the mocker fixture calls that explicitly at the end of each test:

pytest-mock/pytest_mock.py

Lines 157 to 165 in 433378c

@pytest.yield_fixture
def mocker(pytestconfig):
"""
return an object that has the same interface to the `mock` module, but
takes care of automatically undoing all patches after each test method.
"""
result = MockFixture(pytestconfig)
yield result
result.stopall()

So I'm at a loss regarding that.

I thought pytest-dev/pytest#2798 might be related, but doesn't seem the case because there are no failures.

@fogo
Copy link
Contributor

fogo commented Oct 17, 2017

In the last case @bossjones mentioned, can't the issue be that there is a conftest module importing other conftest here, @nicoddemus? Isn't that ill-advised and may result in weird problems w/ tests?

@nicoddemus
Copy link
Member

@fogo that is not recommended and might cause problems specially done like that link, thanks.

@lohrmann
Copy link

lohrmann commented Oct 19, 2017

@nicoddemus Seems to be the first issue to me. I'm a bit out of my depth here as I'm just a 'hobby coder' but wanted to make sure to let you know. If there is any extra information that would be helpful for you let me know.

If a function (from the module I'm testing) is called in a test but was previously mocked out it will still reference the mock. The other way around works.

@nicoddemus
Copy link
Member

If a function (from the module I'm testing) is called in a test but was previously mocked out it will still it will still reference the mock.

That shouldn't happen, the mocker fixture is supposed to "undo" all the mocking it did during the first test. Can you show a minimal example that demonstrates the problem?

@lohrmann
Copy link

Sure — I have put the following together which reproduces the problem on my system. Perhaps I am just using the functionality incorrectly though?

min.py

class ClassTest(object):
    def fun_test(self):
        return True

test_min.py

import min as m

class TestMock(object):
    def test_no_mock1(self):
        c = m.ClassTest()
        assert c.fun_test() is True  # This passes

    def test_mock(self, mocker):
        m.ClassTest.fun_test = mocker.MagicMock()
        c = m.ClassTest()
        c.fun_test()
        print(c.fun_test())  # This is a mock
        assert 1==2

    def test_no_mock2(self):
        c = m.ClassTest()
        print(c.fun_test())  # This is still a mock
        assert c.fun_test() is True  # This fails

    def test_no_mock3(self, mocker):
        mocker.stopall()
        c = m.ClassTest()
        print(c.fun_test())  # This is still a mock
        assert c.fun_test() is True  # This fails

@nicoddemus
Copy link
Member

The problem is that you are assigning the mock object directly:

m.ClassTest.fun_test = mocker.MagicMock()

mocker can't know that it should undo that assignment in that case. Mocking like this for instances is fine because you would be changing only that instance, but by doing in the class you are effectively doing that globally and permanently.

Use mocker.patch.object instead:

mocker.patch.object(m.ClassTest, 'fun_test')

Then your example works as expected for me:

collected 4 items

foo.py .F..

================================== FAILURES ===================================
_____________________________ TestMock.test_mock ______________________________

self = <foo.TestMock object at 0x000001D10A7C1160>
mocker = <pytest_mock.MockFixture object at 0x000001D10A7C12E8>

    def test_mock(self, mocker):
        mocker.patch.object(ClassTest, 'fun_test')
        c = ClassTest()
        c.fun_test()
        print(c.fun_test())  # This is a mock
>       assert 1==2
E       assert 1 == 2

foo.py:16: AssertionError
---------------------------- Captured stdout call -----------------------------
<MagicMock name='fun_test()' id='1997335725616'>
===================== 1 failed, 3 passed in 0.17 seconds ======================

@lohrmann
Copy link

Ahh, I see. PEBKAC after all. Thanks for the explanation!

@nicoddemus
Copy link
Member

TIL what PEBKAC means 😉

You're welcome!

heilaaks added a commit to heilaaks/snippy that referenced this issue Jan 31, 2018
The sys.argv is not needed anymore because the arguments
are passed from main() as a parameter. This eases testing.

There are few exceptions that are not sorted yet. The help
command has additional parameters 'tests' and 'examples'
which requires checking of the sys.argv.

There is one test commented out. It may be that the exception
set by the side_effect for ImportError turns this setting
global. This affects to following test.

/1/ pytest-dev/pytest-mock#84

Signed-off-by: Heikki Laaksonen <laaksonen.heikki.j@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants