coverage fails with mocked os.path.islink

Issue #518 wontfix
Simon Ruderich
created an issue

Hi,

The following code (also attached) fails when run under python3-coverage 4.2 on Debian sid with python3 3.5.2 but works fine when run normally. It looks related to #416.

#!/usr/bin/python3

import unittest
import unittest.mock


class TestMe(unittest.TestCase):
    @unittest.mock.patch('os.path.islink')
    def test_handle_series_simple(self, mock_os_islink):
        print(mock_os_islink.call_args_list)

if __name__ == '__main__':
    unittest.main()
$ python3-coverage run ./test.py
E
======================================================================
ERROR: test_handle_series_simple (__main__.TestMe)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.5/unittest/mock.py", line 1159, in patched
    return func(*args, **keywargs)
  File "./test.py", line 10, in test_handle_series_simple
    print(mock_os_islink.call_args_list)
  File "/usr/lib/python3.5/unittest/mock.py", line 323, in __repr__
    return pprint.pformat(list(self))
  File "/usr/lib/python3/dist-packages/coverage/control.py", line 605, in _should_trace
    disp = self._should_trace_internal(filename, frame)
  File "/usr/lib/python3/dist-packages/coverage/control.py", line 507, in _should_trace_internal
    canonical = files.canonical_filename(filename)
  File "/usr/lib/python3/dist-packages/coverage/files.py", line 69, in canonical_filename
    cf = abs_file(filename)
  File "/usr/lib/python3/dist-packages/coverage/files.py", line 150, in abs_file
    path = os.path.abspath(os.path.realpath(path))
  File "/usr/lib/python3.5/posixpath.py", line 373, in realpath
    path, ok = _joinrealpath(filename[:0], filename, {})
  File "/usr/lib/python3.5/posixpath.py", line 421, in _joinrealpath
    path, ok = _joinrealpath(path, os.readlink(newpath), seen)
OSError: [Errno 22] Invalid argument: '/usr'

----------------------------------------------------------------------
Ran 1 test in 0.002s

FAILED (errors=1)

Regards Simon

Comments (9)

  1. Ned Batchelder repo owner

    In #416 I isolated coverage.py from a mocked os module. I now think that was the wrong approach. Mocking works best when you mock an object where it is used, rather than where it is defined. This prevents the kind of global mischief that you see here.

    If you can share your real code, I can help change the mocking to prevent the problem.

  2. Simon Ruderich reporter

    It took a while to reproduce it with a minimal example, but my real code looks similar to the test case, except the the tested module uses os.path.link to check a file.

    import os
    
    def foo():
        return os.path.islink('Test me')
    
    #!/usr/bin/python3
    
    import unittest
    import unittest.mock
    
    import my_module
    
    class TestMe(unittest.TestCase):
        @unittest.mock.patch('os.path.islink')
        def test_handle_series_simple(self, mock_os_islink):
            my_module.foo()
            print(mock_os_islink.call_args_list)
    
    
    if __name__ == '__main__':
        unittest.main()
    
    E
    ======================================================================
    ERROR: test_handle_series_simple (__main__.TestMe)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/usr/lib/python3.5/unittest/mock.py", line 1159, in patched
        return func(*args, **keywargs)
      File "./test.py", line 12, in test_handle_series_simple
        print(mock_os_islink.call_args_list)
      File "/usr/lib/python3.5/unittest/mock.py", line 323, in __repr__
        return pprint.pformat(list(self))
      File "/usr/lib/python3/dist-packages/coverage/control.py", line 605, in _should_trace
        disp = self._should_trace_internal(filename, frame)
      File "/usr/lib/python3/dist-packages/coverage/control.py", line 507, in _should_trace_internal
        canonical = files.canonical_filename(filename)
      File "/usr/lib/python3/dist-packages/coverage/files.py", line 69, in canonical_filename
        cf = abs_file(filename)
      File "/usr/lib/python3/dist-packages/coverage/files.py", line 150, in abs_file
        path = os.path.abspath(os.path.realpath(path))
      File "/usr/lib/python3.5/posixpath.py", line 373, in realpath
        path, ok = _joinrealpath(filename[:0], filename, {})
      File "/usr/lib/python3.5/posixpath.py", line 421, in _joinrealpath
        path, ok = _joinrealpath(path, os.readlink(newpath), seen)
    OSError: [Errno 22] Invalid argument: '/usr'
    
    ----------------------------------------------------------------------
    Ran 1 test in 0.002s
    
    FAILED (errors=1)
    

    Interestingly I can't reproduce the issue when I use .mock_os_link.return_value = True or use it slightly differently. My real code for example doesn't use mock_os_islink.call_args_list but instead uses .side_effect to collect argument calls and the corresponding return value; but I couldn't get a minimal example.

  3. Ned Batchelder repo owner

    I don't think coverage.py should try to fix this problem. As I mentioned before, I think this mock is over-zealous, and it would be better to adjust the test code to mock more precisely.

  4. Loic Dachary

    I agree with @Ned Batchelder mocking functions where they are defined is asking for trouble. Even if coverage.py was able to deal with this specific issue, there is no guarantee that it won't fail in the future because os.path.islink is used indirectly when running the test. The following will do the same in a more robust way and succeed with and without coverage:

    from os.path import islink
    
    def foo():
        return islink('Test me')
    
    import unittest
    import unittest.mock
    
    import my_module
    
    class TestMe(unittest.TestCase):
        @unittest.mock.patch('my_module.islink')
        def test_handle_series_simple(self, mock_os_islink):
            my_module.foo()
            print(mock_os_islink.call_args_list)
    
    
    if __name__ == '__main__':
        unittest.main()
    
  5. Log in to comment