st2 icon indicating copy to clipboard operation
st2 copied to clipboard

[WIP] Add python3.9 to GHA

Open amanda11 opened this issue 3 years ago • 1 comments

Investigate if any changes needed for unit or integration tests to work with python 3.9.

amanda11 avatar Sep 16 '22 14:09 amanda11

The lint fails seem to do with astroid on python3.9 only, we are fixed to 2.5.6. Lots of fixes on later versions of astroid that mention python 3.9, but with newer versions of astroid our tests fail on both python3.8 and python 3.9. So not as simple as just upgrading astroid.

Test failure:

pylint_plugins/api_models_test.py ...FFF...                                                                      [100%]

======================================================= FAILURES =======================================================
__________________________________________________ test_copied_schema __________________________________________________

    def test_copied_schema():
        code = """
        import copy
    
        class ActionAPI(object):
            schema = {"properties": {"action": {}}}
    
        class ActionCreateAPI(object):
            schema = copy.deepcopy(ActionAPI.schema)
            schema["properties"]["default_files"] = {}
        """
    
        res = parse(code)
    
        assert isinstance(res, nodes.Module)
    
        class1_node: nodes.ClassDef = res.body[1]
        assert isinstance(class1_node, nodes.ClassDef)
    
        assert "schema" in class1_node.locals
    
        # action property added but not property from the other class
        assert "action" in class1_node.locals
        assert "default_files" not in class1_node.locals
    
        class2_node: nodes.ClassDef = res.body[2]
        assert isinstance(class2_node, nodes.ClassDef)
    
        # action (copied) and default_files (added) properties added
        assert "schema" in class2_node.locals
        assert "action" in class2_node.locals
>       assert "default_files" in class2_node.locals
E       AssertionError: assert 'default_files' in {'__module__': [<Const.str l.7 at 0x7f4ac62056a0>], '__qualname__': [<Const.str l.7 at 0x7f4ac62050a0>], 'action': [<AssignName.action l.7 at 0x7f4ac61ee220>], 'schema': [<AssignName.schema l.8 at 0x7f4ac6205490>]}
E        +  where {'__module__': [<Const.str l.7 at 0x7f4ac62056a0>], '__qualname__': [<Const.str l.7 at 0x7f4ac62050a0>], 'action': [<AssignName.action l.7 at 0x7f4ac61ee220>], 'schema': [<AssignName.schema l.8 at 0x7f4ac6205490>]} = <ClassDef.ActionCreateAPI l.7 at 0x7f4ac6205100>.locals

However if I use latest astroid/pylint combinations on python3.8 then the astroid parse on the pylint_plugins copy fails to find the property added after copy. (astroid 2.12.9). So with latest, test fails on both python3.8 and python3.9.

Rolled back down to 2.6.6 astroid and still failed (python 3.8 as well).

Can reproduce problem locally just with code:

from collections.abc import Collection
import astroid
from astroid import parse, nodes
import pylint.checkers.typecheck
import api_models
code="""
 
class ActionAPI(object):
     schema = {"properties": {"action": {}}}

class ActionCreateAPI(object):
    schema = copy.deepcopy(ActionAPI.schema)
    schema["properties"]["default_files"] = {}
"""
res=parse(code)
class2_node: nodes.ClassDef = res.body[2]

The test expects res.body[2].locals to contain both action and default_files properties. But the problem is that only action is present, not default_files property.

If I rolled python 3.8 back down to astroid 2.5.6, then the test passes on python3.8 but fails on python3.9. No joy either with astroid 2.5.8, 2.6.1 on python3.9.

Tried on python 3.9 astroid           2.8.6, pylint            2.11.1. With that combination still get the missing default_files property, but also instead of the res.body having 3 entries, it ends up only having 2.

amanda11 avatar Sep 16 '22 14:09 amanda11

I hit the similar issues with py3.10 and astroid 2.15.5.

============================================================================== short test summary info ==============================================================================
FAILED pylint_plugins/api_models_test.py::test_copied_schema - AssertionError: assert 'default_files' in {'__module__': [<Const.str l.7 at 0x7f118ae0fb20>], '__qualname__': [<Const.str l.7 at 0x7f118ae0f4c0>], 'action': [<AssignName.action...
FAILED pylint_plugins/api_models_test.py::test_copied_imported_schema - AssertionError: assert 'default_files' in {'__module__': [<Const.str l.5 at 0x7f118ae5feb0>], '__qualname__': [<Const.str l.5 at 0x7f118ae4ffa0>], 'description': [<AssignName.d...
FAILED pylint_plugins/api_models_test.py::test_indirect_copied_schema - AttributeError: 'str' object has no attribute 'value'
FAILED pylint_plugins/api_models_test.py::TestTypeChecker::test_finds_no_member_on_api_model_when_property_not_in_schema - AttributeError: module 'pylint.testutils' has no attribute 'Message'
============================================================================ 4 failed, 5 passed in 0.51s ============================================================================

nzlosh avatar Sep 08 '23 11:09 nzlosh

Bingo: py3.9 changed the AST (https://github.com/python/cpython/commit/13d52c268699f199a8e917a0f1dc4c51e5346c42), dropping an intermediate Slice type from something like schema["properties"], so I just pushed a commit that fixes that.

Oddly, the pants-based run did not fail with 3.9 even though it should have. I suspect that pants might be using the wrong python for our tests. I will figure out + address that when I update pants in another PR.

cognifloyd avatar Oct 11 '23 21:10 cognifloyd

@cognifloyd Great find :100:

nzlosh avatar Oct 12 '23 05:10 nzlosh