Skip to content

stubtest: add tests #8380

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

Merged
merged 22 commits into from
Feb 13, 2020
Merged

stubtest: add tests #8380

merged 22 commits into from
Feb 13, 2020

Conversation

hauntsaninja
Copy link
Collaborator

Following up from #8325

Test coverage is 91% from unit tests, and 97% if you count StubtestIntegration.test_typeshed.

@hauntsaninja hauntsaninja force-pushed the stubtesttest branch 2 times, most recently from 27499cb to 5e1e846 Compare February 7, 2020 21:03
@hauntsaninja
Copy link
Collaborator Author

hauntsaninja commented Feb 7, 2020

Hmmmmm.... more mypyc issues.

>   to_check.update(m for m in vars(runtime) if not m.startswith("_"))
1391E   TypeError: dict object expected; got mappingproxy

@hauntsaninja
Copy link
Collaborator Author

hauntsaninja commented Feb 7, 2020

So without any success I tried a) replacing vars with __dict__ and b)

-    to_check.update(m for m in vars(runtime) if not m.startswith("_"))
+    vars_runtime = cast(types.MappingProxyType[str, Any], vars(runtime))  # help mypyc out
+    to_check.update(m for m in vars_runtime if not m.startswith("_"))

Maybe we need to change typeshed stubs? eg https://github.com/python/typeshed/blob/master/stdlib/2and3/builtins.pyi#L42 (update: couldn't get changing this and the return type of vars to MappingProxyType to work)

Is there an easy mypyc workaround?

(Using tox -e py35 --notest; source .tox/py35/bin/activate; pip install -r mypy-requirements.txt; CC=clang MYPYC_OPT_LEVEL=0 python3 setup.py --use-mypyc build_ext --inplace; pytest -k test_arg_name; deactivate to repro)

@hauntsaninja
Copy link
Collaborator Author

Tried a couple things, gave up, came back and found the big off switch. Let me know if you know how to actually fix mypyc's complaints.

@JelleZijlstra JelleZijlstra self-assigned this Feb 8, 2020
Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is great!

As for the mypyc issue, I don't have any special insight. Maybe casting to Any works, or maybe @msullivan knows what's going on.

@hauntsaninja
Copy link
Collaborator Author

hauntsaninja commented Feb 8, 2020

Just took a look at the fresh mypyc errors (after adding stubtest to MYPYC_BLACKLIST). 3.5.1 and 3.7 have different failures.

I can't repro the 3.5.1 one (but I'm using 3.5.9). So maybe not a mypyc issue? Scanned through changelog for py35, but nothing popped up. Maybe one of you would recognise it if it were a Python version issue. I'll investigate more later.

mypy/stubtest.py:59: in Error
1302    runtime_desc: Optional[str] = None
1303/opt/python/3.5.1/lib/python3.5/typing.py:531: in __getitem__
1304    "Cannot subscript an existing Union. Use Union[u, t] instead.")
1305E   TypeError: Cannot subscript an existing Union. Use Union[u, t] instead.

The 3.7 one is a legitimate complaint. I abused LiteralType because a) it seemed to worked out of the box for other types, b) literals can sometimes give more informative errors, c) it type checked, because runtime is Any. One note is that LiteralValue seems to be typed wrong; as per https://mypy.readthedocs.io/en/latest/literal_types.html#parameterizing-literals it should at least accept bytes (and enums, which actually happens to be the unit test failure here)

mypy/stubtest.py:663: in verify_var
1075    runtime_type = get_mypy_type_of_runtime_value(runtime)
1076mypy/stubtest.py:893: in get_mypy_type_of_runtime_value
1077    fallback=mypy.types.Instance(type_info, [anytype() for _ in type_info.type_vars]),
1078_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
1079
1080>   def __init__(self, value: LiteralValue, fallback: Instance,
1081                 line: int = -1, column: int = -1) -> None:
1082E                TypeError: union[int, str, bool] object expected; got __future__._Feature
1083
1084mypy/types.py:1592: TypeError

Guess I'll have to update the comment before L893:

    # Technically, Literals are supposed to be only bool, int, str or bytes, but this
    # seems to work fine
    return mypy.types.LiteralType(
        value=runtime,
        fallback=mypy.types.Instance(type_info, [anytype() for _ in type_info.type_vars]),
    )

@hauntsaninja
Copy link
Collaborator Author

hauntsaninja commented Feb 8, 2020

I tried with 3.5.3 and couldn't repro either. Unfortunately, I can't get a working version of 3.5.2 or 3.5.1 (pyenv fails with ssl issues and manually built python runs into zlib issues).

Do we still need to be specifically compatible with 3.5.1? https://github.com/python/mypy/blob/master/.travis.yml#L26 (line is from 2016) 3.5.1 is over four years old, and EOL for 3.5 is in seven months.

Update: seems to work in travis using 3.5.3 as well, https://travis-ci.org/hauntsaninja/mypy/builds/647868830

hauntsaninja pushed a commit to hauntsaninja/mypy that referenced this pull request Feb 9, 2020
@hauntsaninja
Copy link
Collaborator Author

Looks like virtualenv had a new release today that's broken various different parts of the build. I'll attempt to fix in another PR.

@hauntsaninja
Copy link
Collaborator Author

Phew, all green now!

@@ -100,6 +100,9 @@ def run(self):
# We don't populate __file__ properly at the top level or something?
# Also I think there would be problems with how we generate version.py.
'version.py',

# Written by someone who doesn't know how to deal with mypyc
'stubtest.py',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I never figured out how to get vars / dict to work. This is the big off switch I mentioned in #8380 (comment)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, I inferred from some of the changes since that you'd figured out a workaround. I merged this change now and hopefully we'll eventually find a way to remove the special-casing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The workarounds were for the bug in early Python 3.5s and a valid mypyc complaint from another module.
I’ll try Sully’s suggestion to see if we can remove stubtest from the blacklist.

@JelleZijlstra JelleZijlstra merged commit a07dbd0 into python:master Feb 13, 2020
@msullivan
Copy link
Collaborator

Sorry, I had meant to double back on this and it slipped through the cracks. __dict__ is going to have trouble, since compiled objects don't have them. We do some hacks such as https://github.com/python/mypy/blob/master/mypy/util.py#L312-L321 to deal with some of these issues. mypyc compiled objects also all have a __getstate__ method that will return a dictionary.

mypyc really needs to implement a system for overriding lies in typeshed (as discussed mypyc/mypyc#676).

I think the workaround here would be to do cast(Any, vars)(runtime). Casting the return of vars is too late, since mypyc will have already checked that it matches the original type.

If you want to follow-up with another attempt to get it to work, I'll review it quickly. But it isn't particularly critical.

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

Successfully merging this pull request may close these issues.

3 participants