search

coala / coala-bears

Bears for coala
https://coala.io/
GNU Affero General Public License v3.0
295 stars 580 forks source link

AttributeError: module 'unittest' has no attribute 'mock' #2862

Open jayvdb opened 5 years ago

jayvdb commented 5 years ago

Seen on Python 3.7

[   83s] ==================================== ERRORS ====================================
[   83s] ___________________ ERROR collecting tests/coalaBearTest.py ____________________
[   83s] tests/coalaBearTest.py:10: in <module>
[   83s]     class coalaBearTest(unittest.TestCase):
[   83s] tests/coalaBearTest.py:29: in coalaBearTest
[   83s]     @unittest.mock.patch('sys.version_info', tuple((2, 7, 11)))
[   83s] E   AttributeError: module 'unittest' has no attribute 'mock'
frextrite commented 5 years ago

unittest.mock.patch gives AttributeError on all versions of python 2.7+

However if the tests are run by pytest the above error is not thrown. This was likely fixed in the latest pytest resulting in the above error.

areebbeigh commented 5 years ago

It's a simple fix with 2 changes, from unittest.mock import patch for Python 3.

This could be a newcomer issue. If not, I want to be assigned.

jayvdb commented 5 years ago

@frextrite , if you want to explore your pytest theory, the versions used can be found in the logs at https://build.opensuse.org/package/show/home:jayvdb:coala/python-coala-bears , and you can 'branch' that package and edit the .spec to remove the rm tests/coalaBearTest.py which I did as a quick hack.

jayvdb commented 5 years ago

@areebbeigh, that sounds like it might be the fix, but how do we prevent it from occurring again? If you can think of a solution to that, this becomes a viable non-newcomer issue.

frextrite commented 5 years ago

@jayvdb We should go with the fix suggested by @areebbeigh, changing import unittest to from unittest.mock import patch.

but how do we prevent it from occurring again?

This would a hardbound fix and it would never occur again. The old method being used in coalaBearTest.py was bound to fail in every version of python but pytest with its magic made it work somehow. The new import method would never fail as it is the recommended(only correct) way of using the patch decorator.

jayvdb commented 5 years ago

was bound to fail in every version of python but pytest with its magic made it work somehow.

But we use pytest, and it does this magic, and it doesnt work with the pytest version that distros currently provide even in their rolling release. That means developers are likely going to try using the pytest from their OS and it will fail.

Someone can re-introduce this bad import and our CI wont fail. Expecting people to follow guidelines doesnt work -- if it did, this import wouldnt exist.

areebbeigh commented 5 years ago

Stumbled upon the following here:

Improved reporting of mock call assertion errors This plugin monkeypatches the mock library to improve pytest output for failures of mock call assertions like Mock.assert_called_with() by hiding internal traceback entries from the mock module.

The following config in pytest.ini disables this plugin and the AttributeError is detected for the bad import:

[pytest]
mock_traceback_monkeypatch = false

PyTest version: v3.6.4

__________________________________________ ERROR collecting tests/coalaBearTest.py ___________________________________________
tests/coalaBearTest.py:10: in <module>
    class coalaBearTest(unittest.TestCase):
tests/coalaBearTest.py:29: in coalaBearTest
    @unittest.mock.patch('sys.version_info', tuple((2, 7, 11)))
E   AttributeError: module 'unittest' has no attribute 'mock'
jayvdb commented 5 years ago

Lovely @areebbeigh . Please check that setting on coala repo also, and then submit a MR to https://github.com/coala/mobans with last line Related to https://github.com/coala/coala-bears/issues/2862 , adding it to the template for all coala python repos

areebbeigh commented 5 years ago

I think this might require a little more investigation. I'm running into strange behaviour now:

coalaDeleteOrigTest.py in the coala repo uses mock.patch in the same broken way but still passes when I run it with nosetests.

The line from coalib import coala_delete_orig for some reason passes the test. Tried pasting it into coalaBearTest.py and it causes the test to pass. - check the comment below

@jayvdb

areebbeigh commented 5 years ago

After looking into the requirements and test behaviours I found (oddly):

This fix is still applicable but not for coala-utils==0.7.0. Further investigation needed.

jayvdb commented 5 years ago

@areebbeigh , you may be seeing an example of an import changing state.

i.e. if from unittest.mock import patch is done first, from unittest import mock; mock.patch may become valid afterwards but not before. Just an idea. I've seen it happen before.

areebbeigh commented 5 years ago

It's testfixtures causing the problems. Turns out, coala-utils starting from 0.7.0 is using testfixtures therefore any import from coala-utils using testfixtures causes this behaviour. You've already created an issue: Remove dependency on textfixtures (Related: https://github.com/coala/coala/issues/5335)

TL;DR: For a fix, testfixtures should only be a test-requirement and never otherwise.

So we only have a fix for tests not depending on coala-utils==0.7.0 (directly or indirectly) for now. I assume this issue is blocked until coala-utils 0.8.0?

Currently the master branch for coala-utils isn't using this library out of tests (removed here).

To confirm:

This test will fail (AttributeError) if import testfixture is commented out and pass otherwise.

import unittest
import sys
import os

import testfixtures

class Test(unittest.TestCase):
    @unittest.mock.patch('sys.version_info', (2,7,0))
    def test_version(self):
        os.system(f"echo '{sys.version_info}'")
        assert(sys.version_info < (3,0,0))
jayvdb commented 5 years ago

I assume this issue is blocked until coala-utils 0.8.0?

Yup. Great analysis. I'll leave this assigned to you, and give you tasks related to getting coala-utils 0.8.0 released. It depends on PyPrint being released, and that is almost done, but there are a few more things which should be done.

areebbeigh commented 5 years ago

Needs a status/blocked @jayvdb

jayvdb commented 5 years ago

Please take a look upstream to see if testfixtures is doing anything funky which might be causing this.

We are using an old pinned version of testfixtures, so the problem might be fixed in more recent versions.

Relying on an import sequence isnt 'safe'.

This could be an extension of https://gitlab.com/coala/bears/coala-pyflakes/issues/10 , as it is easy to determine that unittest.mock is a sub-module by attempting to import it, and thus should not be accessed as a property of unittest.

But the problematic import needs to be fixed asap, so we cant block this. If we cant prevent it from happening quickly, we need to raise a separate issue about it, or include it as part of another issue (e.g. the one above) so it isnt forgotten.

areebbeigh commented 5 years ago

Please take a look upstream to see if testfixtures is doing anything funky which might be causing this.

We are using an old pinned version of testfixtures, so the problem might be fixed in more recent versions.

The latest version for testfixtures (6.5.0) is the same.

Relying on an import sequence isnt 'safe'.

Then I'll create a PR changing all the imports to the form below whereever necessary?

import unittest
import unittest.mock

But the problematic import needs to be fixed asap, so we cant block this. If we cant prevent it from happening quickly, we need to raise a separate issue about it or include it as part of another issue (e.g. the one above) so it isn't forgotten.

It can probably be fixed in all cases. I'll look through it (including the core repo).

Should I open a separate issue on the coala repo for these imports or I can just refer this one in a PR?

areebbeigh commented 5 years ago

About preventing from the errored importing from passing the tests in the future I'd like to recap that it'll be taken care of as follows:

jayvdb commented 5 years ago

Should I open a separate issue on the coala repo for these imports or I can just refer this one in a PR?

Yea, we only need one issue - you can do all the PRs.

An MR to the mobans repo has been made disabling monkeypatch for pytest

Im not sure about this yet - it sounds like it is useful ...

testfixture won't be a (non test) dependency anymore in coala-utils 0.8.0

And here is the real problem ... even if it is removed from coala-utils source, we still might do import testfixtures in the tests, and the problem can re-occur.

The problem in testfixtures needs to be found and fixed.

And a linter is still needed to catch this type of problem.

And if those two are done, we dont need to disable the monkey patch magic in pytest.

areebbeigh commented 5 years ago

Not an issue with testfixtures. Here's what's happening: From here and https://github.com/Simplistix/testfixtures/issues/108

when a submodule is loaded anywhere it is automatically added to the global namespace of the package

$ python3
>>> import logging
>>> logging.config
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'module' object has no attribute 'config'
$ echo "import logging.config" > weirdimport.py
$ python3
>>> import weirdimport
>>> import logging
>>> logging.config
<module 'logging.config' from '/usr/local/Cellar/python3/3.4.3/Frameworks/Python.framework/Versions/3.4/lib/python3.4/logging/config.py'>

So it's just the way imports are dealt with in Python.

jayvdb commented 5 years ago

Yup, which is why we need a linter to protect against this part of the Python runtime.