add mypy_selftest.py #1102
add mypy_selftest.py #1102
Conversation
tests/mypy_selftest.py
Outdated
| shutil.copytree('stdlib', str(dirpath / 'mypy/typeshed/stdlib')) | ||
| shutil.copytree('third_party', str(dirpath / 'mypy/typeshed/third_party')) | ||
| try: | ||
| subprocess.run(['./runtests.py'], cwd=str(dirpath / 'mypy')) |
There was a problem hiding this comment.
The one place where you don't use check=True is where you're using try:except CalledProcessError: ;)
tests/mypy_selftest.py
Outdated
| subprocess.run(['./runtests.py'], cwd=str(dirpath / 'mypy')) | ||
| except subprocess.CalledProcessError: | ||
| print('mypy tests failed', file=sys.stderr) | ||
| sys.exit(1) |
There was a problem hiding this comment.
Would be rad to reuse the .returncode from the exception raised.
| import tempfile | ||
|
|
||
|
|
||
| if __name__ == '__main__': |
There was a problem hiding this comment.
nit: this is a bit of cargo culting. The point of ifmain is testability. Putting all code in this block doesn't achieve that, you're just losing one level of indentation. I'd just remove it for our simple script. Otherwise, prefer main() functions, they don't create unexpected globals :-)
There was a problem hiding this comment.
It also helps tools that try to import every file in a tree (e.g., stubgen). I prefer to just always avoid side-effects from importing files.
| subprocess.run(['git', 'clone', '--depth', '1', 'git://github.com/python/mypy', | ||
| str(dirpath / 'mypy')], check=True) | ||
| subprocess.run([sys.executable, '-m', 'pip', 'install', '-U', '-r', | ||
| str(dirpath / 'mypy/test-requirements.txt')], check=True) |
There was a problem hiding this comment.
Let's create nice error messages for both.
There was a problem hiding this comment.
I feel like that's overengineering. People submitting changes to typeshed can probably read a Python stack trace, and there's a chance we make it harder to find the underlying error if we wrap it in our own error handling.
| subprocess.run([sys.executable, '-m', 'pip', 'install', '-U', '-r', | ||
| str(dirpath / 'mypy/test-requirements.txt')], check=True) | ||
| shutil.copytree('stdlib', str(dirpath / 'mypy/typeshed/stdlib')) | ||
| shutil.copytree('third_party', str(dirpath / 'mypy/typeshed/third_party')) |
There was a problem hiding this comment.
Those two guys can also raise shutil.Error.
.travis.yml
Outdated
| env: TEST_CMD="flake8" | ||
| - python: "3.5" | ||
| env: TEST_CMD="./tests/mypy_test.py" | ||
| env: TEST_CMD="./tests/mypy_test.py && ./tests/mypy_selftest.py" |
There was a problem hiding this comment.
How about we spin up another job for this? We could cover a release Python version with this (e.g. "3.6"). Then you wouldn't need bash -c below.
There was a problem hiding this comment.
It would be nice to have a different job, but I don't like the current setup where we use python versions as the labels, but really the different Python versions do completely different things (running flake, mypy, and pytype). At least one PR contributor was confused because their PR was failing in "2.7" when it was really failing in pytype. Not sure how to achieve that in Travis though; I'll just use 3.6 for now.
|
The Travis failure is because a change to typeshed broke a mypy test; Guido is fixing in python/mypy#3058. |
|
Can you rebase so I can merge this? |
|
Why does it need a rebase? There's no merge conflict right now, but I can rebase the branch if you prefer that. Thanks for looking at this! |
|
Hm, now that I look at it, it doesn't say anything about conflicts. But it definitely did 3 days ago O_O |
2 participants
See discussion in #1090. I made it a Python script instead of shell so I can ensure the directory is cleaned up.