Skip to content
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

stubtest: fix literal type construction #11931

Merged
merged 7 commits into from Jan 7, 2022
Merged

stubtest: fix literal type construction #11931

merged 7 commits into from Jan 7, 2022

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Jan 7, 2022

I've started with a new unit test that illustrates the problem.

Closes python/typeshed#6845

@sobolevn sobolevn changed the title Literal[b] was not properly checked by stubtest Literal[bytes] was not properly checked by stubtest Jan 7, 2022
@sobolevn
Copy link
Member Author

sobolevn commented Jan 7, 2022

Failure!

 =================================== FAILURES ===================================
__________________________ StubtestUnit.test_literal ___________________________
[gw1] linux -- Python 3.10.1 /home/runner/work/mypy/mypy/.tox/py/bin/python

args = (<mypy.test.teststubtest.StubtestUnit testMethod=test_literal>,)
kwargs = {}, cases = [<mypy.test.teststubtest.Case object at 0x7f33ebc511b0>]
expected_errors = set()
c = <mypy.test.teststubtest.Case object at 0x7f33ebc511b0>, @py_assert1 = False
@py_format3 = "{'test_module..._module.BYT2'} == set()\n~Extra items in the left set:\n~'test_module.BYT1'\n~'test_module.BYT2'\n~Use -v to get the full diff"
@py_format5 = "test_module.BYT1\n~test_module.BYT2\n~\n>assert {'test_module..._module.BYT2'} == set()\n~Extra items in the left set:\n~'test_module.BYT1'\n~'test_module.BYT2'\n~Use -v to get the full diff"
output = 'test_module.BYT1\ntest_module.BYT2\n'
actual_errors = {'test_module.BYT1', 'test_module.BYT2'}

    def test(*args: Any, **kwargs: Any) -> None:
        cases = list(fn(*args, **kwargs))
        expected_errors = set()
        for c in cases:
            if c.error is None:
                continue
            expected_error = "{}.{}".format(TEST_MODULE_NAME, c.error)
            assert expected_error not in expected_errors, (
                "collect_cases merges cases into a single stubtest invocation; we already "
                "expect an error for {}".format(expected_error)
            )
            expected_errors.add(expected_error)
        output = run_stubtest(
            stub="\n\n".join(textwrap.dedent(c.stub.lstrip("\n")) for c in cases),
            runtime="\n\n".join(textwrap.dedent(c.runtime.lstrip("\n")) for c in cases),
            options=["--generate-allowlist"],
        )
    
        actual_errors = set(output.splitlines())
>       assert actual_errors == expected_errors, output
E       AssertionError: test_module.BYT1
E         test_module.BYT2
E         
E       assert {'test_module..._module.BYT2'} == set()
E         Extra items in the left set:
E         'test_module.BYT1'
E         'test_module.BYT2'
E         Use -v to get the full diff

mypy/test/teststubtest.py:128: AssertionError
=========================== short test summary info ============================
FAILED mypy/test/teststubtest.py::StubtestUnit::test_literal - AssertionError...
====== 1 failed, 9679 passed, 371 skipped, 7 xfailed in 684.78s (0:11:24) ======

Link: https://github.com/python/mypy/runs/4736271970?check_suite_focus=true

Now I will try to fix it.

@sobolevn
Copy link
Member Author

sobolevn commented Jan 7, 2022

Done! CC @hauntsaninja

@@ -942,6 +942,15 @@ def is_subtype_helper(left: mypy.types.Type, right: mypy.types.Type) -> bool:
):
# Pretend Literal[0, 1] is a subtype of bool to avoid unhelpful errors.
Copy link
Member Author

Choose a reason for hiding this comment

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

Btw, I am almost sure that it does not work anymore, because bool() now is Union[Literal[False], Literal[True]].

I can address this in a new PR.

@sobolevn sobolevn mentioned this pull request Jan 7, 2022
@sobolevn
Copy link
Member Author

sobolevn commented Jan 7, 2022

Looks like mypyc-compiled mypy fails. Here's the reason why: https://github.com/python/mypy/blame/e8cf960e8579e5cee2db27212efad4870e5106bd/mypy/stubtest.py#L1032-L1041

Any ideas?

@hauntsaninja
Copy link
Collaborator

Looking, I can push a fix to your branch!

@sobolevn
Copy link
Member Author

sobolevn commented Jan 7, 2022

Let's wait for my new solution to build. Maybe I've found a good workaround. But, mypyc builds need to finish.

I can push a fix to your branch!

Yes, Allow edits and access to secrets by maintainers is enabled.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Jan 7, 2022

Oh just saw your message. Our fixes are quite similar. We should be able to change enum handling as well and get rid of that hacky try catch completely (what I pushed isn't quite right). I also collate the tests... it makes running stubtest tests much faster (since each test method involves writing files to disk)

Copy link
Member Author

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

LGTM!

@hauntsaninja hauntsaninja changed the title Literal[bytes] was not properly checked by stubtest stubtest: fix literal type construction Jan 7, 2022
@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Jan 7, 2022

As a result, this should be fixed as well https://github.com/python/typeshed/blob/032e6ee90cbb938259a2aa2183966502de19108e/tests/stubtest_allowlists/py3_common.txt#L108 :-)

@sobolevn
Copy link
Member Author

sobolevn commented Jan 7, 2022

Awesome! I will send a PR to fix it, when new mypy will be out!

@hauntsaninja hauntsaninja merged commit e40877d into python:master Jan 7, 2022
JukkaL pushed a commit that referenced this pull request Jan 7, 2022
tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this pull request Jan 20, 2022
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.

None yet

2 participants