Testsuite fails on python 3.7

[eli-schwartz]

Trying to rebuild astroid master (due to the lack of a release and multiple failures with the stable release -- I was trying to see if the impending 2.0.0 release would fix it) for python 3.7 on Arch Linux, I hit the following testsuite error:

=================================== FAILURES ===================================
______________ GetModuleFilesTest.test_load_module_set_attribute _______________

self = <astroid.tests.unittest_modutils.GetModuleFilesTest testMethod=test_load_module_set_attribute>

    def test_load_module_set_attribute(self):
        import xml.etree.ElementTree
        import xml
        del xml.etree.ElementTree
        del sys.modules['xml.etree.ElementTree']
>       m = modutils.load_module_from_modpath(['xml', 'etree', 'ElementTree'])

astroid/tests/unittest_modutils.py:311: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
astroid/modutils.py:231: in load_module_from_modpath
    module = imp.load_module(curname, mp_file, mp_filename, mp_desc)
/usr/lib/python3.7/imp.py:235: in load_module
    return load_source(name, filename, file)
/usr/lib/python3.7/imp.py:172: in load_source
    module = _load(spec)
<frozen importlib._bootstrap>:696: in _load
    ???
<frozen importlib._bootstrap>:677: in _load_unlocked
    ???
<frozen importlib._bootstrap_external>:724: in exec_module
    ???
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <imp._LoadSourceCompatibility object at 0x7fe448e1acf8>
fullname = 'xml.etree.ElementTree'

>   ???
E   TypeError: a bytes-like object is required, not 'str'

<frozen importlib._bootstrap_external>:838: TypeError
======== 1 failed, 801 passed, 48 skipped, 10 xfailed in 21.08 seconds =========

I actually get more errors trying to rebuild master on python 3.6:

=================================== FAILURES ===================================
___________________________ test_type_comments_with ____________________________

    @pytest.mark.skipif(not HAS_TYPED_AST, reason="requires typed_ast")
    def test_type_comments_with():
        module = builder.parse('''
        with a as b: # type: int
            pass
        with a as b: # type: ignore
            pass
        ''')
        node = module.body[0]
        ignored_node = module.body[1]
>       assert isinstance(node.type_annotation, astroid.Name)
E       AssertionError: assert False
E        +  where False = isinstance(None, <class 'astroid.node_classes.Name'>)
E        +    where None = <With l.2 at 0x7f8ab1ddb400>.type_annotation
E        +    and   <class 'astroid.node_classes.Name'> = astroid.Name

astroid/tests/unittest_nodes.py:863: AssertionError
____________________________ test_type_comments_for ____________________________

    @pytest.mark.skipif(not HAS_TYPED_AST, reason="requires typed_ast")
    def test_type_comments_for():
        module = builder.parse('''
        for a, b in [1, 2, 3]: # type: List[int]
            pass
        for a, b in [1, 2, 3]: # type: ignore
            pass
        ''')
        node = module.body[0]
        ignored_node = module.body[1]
>       assert isinstance(node.type_annotation, astroid.Subscript)
E       AssertionError: assert False
E        +  where False = isinstance(None, <class 'astroid.node_classes.Subscript'>)
E        +    where None = <For l.2 at 0x7f8ab1e6bcf8>.type_annotation
E        +    and   <class 'astroid.node_classes.Subscript'> = astroid.Subscript

astroid/tests/unittest_nodes.py:878: AssertionError
___________________________ test_type_coments_assign ___________________________

    @pytest.mark.skipif(not HAS_TYPED_AST, reason="requires typed_ast")
    def test_type_coments_assign():
        module = builder.parse('''
        a, b = [1, 2, 3] # type: List[int]
        a, b = [1, 2, 3] # type: ignore
        ''')
        node = module.body[0]
        ignored_node = module.body[1]
>       assert isinstance(node.type_annotation, astroid.Subscript)
E       AssertionError: assert False
E        +  where False = isinstance(None, <class 'astroid.node_classes.Subscript'>)
E        +    where None = <Assign l.2 at 0x7f8ab1dd07b8>.type_annotation
E        +    and   <class 'astroid.node_classes.Subscript'> = astroid.Subscript

astroid/tests/unittest_nodes.py:892: AssertionError
_________________________ test_type_comments_function __________________________

    @pytest.mark.skipif(not HAS_TYPED_AST, reason="requires typed_ast")
    def test_type_comments_function():
        module = builder.parse('''
        def func():
            # type: (int) -> str
            pass
        def func1():
            # type: (int, int, int) -> (str, str)
            pass
        def func2():
            # type: (int, int, str, List[int]) -> List[int]
            pass
        ''')
        expected_annotations = [
            (["int"], astroid.Name, "str"),
            (["int", "int", "int"], astroid.Tuple, "(str, str)"),
            (["int", "int", "str", "List[int]"], astroid.Subscript, "List[int]"),
        ]
        for node, (
                expected_args,
                expected_returns_type,
                expected_returns_string
            ) in zip(module.body, expected_annotations):
>           assert node.type_comment_returns is not None
E           assert None is not None
E            +  where None = <FunctionDef.func l.2 at 0x7f8ab1ea7860>.type_comment_returns

astroid/tests/unittest_nodes.py:950: AssertionError
======== 4 failed, 803 passed, 43 skipped, 10 xfailed in 23.47 seconds =========

[PCManticore]

I just released astroid 2.0 and was working on Pylint 2.0 as well. Not exactly sure why you're getting these errors though.

[PCManticore]

From a quick look it seems most of these are related to typed_ast and the fact that the type annotations are not extracted. Is typed_ast installed on that Python version?

[eli-schwartz]

Hmm, I missed this suddenly being necessary. It fixes the python 3.6 failures, but not the python 3.7 error.

[PCManticore]

It should be installed by default though, wasn't that the case? (at least for Python 3.6, not for 3.7 where a compatible version has not been released yet). Taking a look at the other test failure.

[eli-schwartz]

I see 6 tests are marked skipif(not HAS_TYPED_AST, reason="requires typed_ast") but this still errored...

[eli-schwartz]

It should be installed by default though, wasn't that the case?

Not when you're manually assembling test dependencies without test_requires in order to run pytest in a distro packaging environment I guess.

[PCManticore]

Yeah, that makes sense though. Curious why you're not using tox though?

Regarding the other test failure, I cannot reproduce it locally nor on CI. What Python 3.7 build do you have? If you do the following in a shell, does it still reproduces?

from astroid import modutils
m = modutils.load_module_from_modpath(['xml', 'etree', 'ElementTree'])

[eli-schwartz]

Using tox means we only test your code, which you already do.

Not using tox, means we check the package we're creating, specifically that it works okay with the distro packages we've added to the dependency tree.

So for distro packaging purposes, tox is actively unhelpful and downright bad/counterproductive. :p

[eli-schwartz]

Yes, systemd-nspawn into an Arch Linux container running the staging repositories (which is where our python 3.7 package is currently sitting while we try to rebuild everything), cd to an astroid git checkout, and run:

[builduser@eschwartz astroid]$ python
Python 3.7.0 (default, Jun 29 2018, 16:28:53) 
[GCC 8.1.1 20180531] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from astroid import modutils
>>> m = modutils.load_module_from_modpath(['xml', 'etree', 'ElementTree'])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/build/python-astroid/src/astroid/astroid/modutils.py", line 231, in load_module_from_modpath
    module = imp.load_module(curname, mp_file, mp_filename, mp_desc)
  File "/usr/lib/python3.7/imp.py", line 235, in load_module
    return load_source(name, filename, file)
  File "/usr/lib/python3.7/imp.py", line 172, in load_source
    module = _load(spec)
  File "<frozen importlib._bootstrap>", line 696, in _load
  File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 724, in exec_module
  File "<frozen importlib._bootstrap_external>", line 838, in get_code
TypeError: a bytes-like object is required, not 'str'

[PCManticore]

What I don't understand is what exactly is raising the TypeError and why. I have a July 4 build and doesn't error at all. Do you have by any chance any modifications to the CPython interpreter? Also if you do import xml.etree.ElementTree I assume that works, right?

Thanks for the tox explanation! that makes sense!

[eli-schwartz]

I can import xml.etree.ElementTree just fine, yes.

Also running tox just in case, fails too (it's only testing the actual python interpreter while grabbing fresh packages from PyPI etc. but indeed, this is failing anyway so something about the interpreter itself is obviously failing to work as expected by astroid.modutils).

The python interpreter is built using this buildscript: https://git.archlinux.org/svntogit/packages.git/tree/repos/staging-x86_64/PKGBUILD?h=packages/python

We shouldn't be doing anything controversial I don't think...

[yan12125]

I cloned the CPython repo from GitHub and build the tag v3.7.0. modutils.load_module_from_modpath(['this']) works just fine, but the same statement fails for the official Arch Linux package from [staging]. There's something strange in PKGBUILD and my first guess is mis-optimization. - see below :)

[yan12125]

Oh, a just fixed CPython bug (after 3.7.0 release) has the same traceback as this issue - https://bugs.python.org/issue34056. Let me try that patch.

[eli-schwartz]

So basically the bug is "why does this still use imp"? :p

[PCManticore]

Thanks for finding that bug @yan12125 ! Ideally we'd move away from imp now that we support Python 3 only, so we can use importlib and friends, but it's not going to happen in the day of the release. :)

[PCManticore]

In which case I imagine you'd like to backport that CPython patch back to Arch, until we completely remove imp from the codebase?

[eli-schwartz]

Yes, that would (both) indeed be a good idea.

Closing in favor of the new issue.