Improve stub testing to cover all of typeshed #5051
Conversation
|
Would it be possible to replace this by an invocation of |
|
@JelleZijlstra IIRC you now want to do the opposite, replace the check in typeshed with this one? Is that right? Once this is merged it should be easy to just invoke this from typeshed. |
|
I don't feel very strongly about this, but apparently this uncovered bugs in typeshed that typeshed's own tests didn't find, so it would be better to run this test in typeshed itself. |
|
What is the status of this PR? |
|
test_stubs has moved in #5142, I think we should still change the tests to cover all of typeshed. I will modify this to change the pytest test instead of what is in runtests.py |
|
Okay this now targets the pytest test. I decided to split each version into its own test so pytest can parrallelize them. |
mypy/test/testsamples.py
Outdated
| def check_stubs(self, version: str, name: Optional[str] = None) -> None: | ||
| if name is None: | ||
| name = version | ||
| seen = {'__builtin__', } |
There was a problem hiding this comment.
Can you add a short comment explaining why we start by adding __builtin__ to the set?
Also, minor nit: I think just doing seen = {'__builtin__'} (without the comma) works.
mypy/test/testsamples.py
Outdated
| if name is None: | ||
| name = version | ||
| seen = {'__builtin__', } | ||
| modules = ['__builtin__'] |
There was a problem hiding this comment.
I think we can just have this list start by being empty -- that would let us remove the call to modules.remove('__builtin__') down below.
mypy/test/testsamples.py
Outdated
|
|
||
| def test_2and3_3(self) -> None: | ||
| sys_ver_str = '.'.join(map(str, sys.version_info[:2])) | ||
| self.check_stubs(sys_ver_str, name="2and3") |
There was a problem hiding this comment.
Rather then checking the "2and3" directory in only these two cases, would it maybe make sense to pass in a list of directories we want to check into check_stubs?
E.g. we could do self.check_stubs(["2.7", "2and3"]) inside test_2, do self.check_stubs(["2and3", "3", "3.5"]) inside test_35...
I think this would probably make our tests more thorough, though the tradeoff is that each individual test would take longer.
(This is just an idea though -- maybe the tradeoff isn't worth it?)
There was a problem hiding this comment.
Part of the reason I did this was we already run tests in a matrix of options on Travis, so we'd end up testing all the versions of Python we support if we use the system version info. I collapsed the 2/2and3 and 3/2and3 tests together, but I think otherwise, there isn't a point in adding 3/2and3 to each test.
This will fail until typeshed is synced with python/typeshed#2130 and python/typeshed#2121
Previously, we were only checking 2and3, 3.3-3.5 and 3. This checks 3.6, 3.7 and 2.