-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
modify pytype_test to run from within pytype too, and support python3 #1817
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
Conversation
Restarted mypy test, now that #1818 is in. |
@@ -1,5 +1,5 @@ | |||
#!/usr/bin/env python | |||
"""Test runner for typeshed. | |||
r"""Test runner for typeshed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"r"? Like, in a regexp?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r for raw string, since it has a literal backslash within it
default='/opt/python/3.6/bin/python3.6', | ||
help='Path to a python 3.6 interpreter.') | ||
|
||
Dirs = collections.namedtuple('Dirs', ['pytype', 'typeshed']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/Dirs/Config/ ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'll rename it if i add any more configuration options; right now it's just directories (saves having to name the fields config.pytype_dir and config.typeshed_dir)
tests/pytype_test.py
Outdated
@@ -83,34 +106,56 @@ def communicate(self): | |||
return self.results | |||
|
|||
|
|||
def _get_relative(filename): | |||
top = filename.find('stdlib') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding '/' after 'stdlib' might make this a tiny bit more robust.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
tests/pytype_test.py
Outdated
BinaryRun([pytd_exe, '-h']).communicate() | ||
except OSError: | ||
print('Cannot run pytd. Did you install pytype?') | ||
return 0, 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might be a bit more readable if you did a "can_run" method that does the try/BinaryRun/except part and returns True/False, since then you can do:
if can_run(config.pytype, 'pytd', '-h'):
pytd_exe = os.path.join(config.pytype, 'pytd')
elif can_run(config.pytype, 'pytd_tool', '-h'):
pytd_exe = os.path.join(config.pytype, 'pytd_tool')
else:
print ('Cannot run pytd. Did you install pytype?')
return 0, 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
tests/pytype_test.py
Outdated
'--typeshed-location=%s' % dirs.typeshed, | ||
'--module-name=%s' % _get_module_name(f), | ||
'--convert-to-pickle=%s' % os.devnull | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this below to where it's used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's used just below; the if block appends to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see below
tests/pytype_test.py
Outdated
errors += 1 | ||
bad.append((_get_relative(test_run.args[-1]), | ||
stderr.rstrip().rsplit('\n', 1)[-1])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This discards all lines but the last? Please add a comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
tests/pytype_test.py
Outdated
run_cmd += [ | ||
'-V 3.6', | ||
'--python_exe=%s' % args.python36_exe | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move the definition of run_cmd and the "if" block below, to where it's used? Right now, it's confusing that you're building up this command, but then you're not using it in the "f not in pytype_run" case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
tests/pytype_test.py
Outdated
'--typeshed-location=%s' % dirs.typeshed, | ||
'--module-name=%s' % _get_module_name(f), | ||
'--convert-to-pickle=%s' % os.devnull | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see below
@JelleZijlstra: Could you take a look whether you're OK with the changes to .travis.yml? |
|
||
install: | ||
# pytype needs py-2.7, mypy needs py-3.3+. Additional logic in runtests.py | ||
- if [[ $TRAVIS_PYTHON_VERSION == '3.6-dev' ]]; then pip install -U flake8==3.3.0 flake8-bugbear>=17.3.0 flake8-pyi>=17.1.0; fi | ||
- if [[ $TRAVIS_PYTHON_VERSION == '3.5' ]]; then pip install -U git+git://github.com/python/mypy git+git://github.com/python/typed_ast; fi | ||
- if [[ $TRAVIS_PYTHON_VERSION == '2.7' ]]; then pip install -U git+git://github.com/google/pytype; fi | ||
- if [[ $TRAVIS_PYTHON_VERSION == '2.7' ]]; then pip install -U git+git://github.com/google/pytype; wget https://s3.amazonaws.com/travis-python-archives/binaries/ubuntu/14.04/x86_64/python-3.6.tar.bz2; sudo tar xjf python-3.6.tar.bz2 --directory /; fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really like this use of wget. Could we instead run in a 3.6 environment, but then use the system python2.7 to run pytype?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to be running in a 2.7 venv otherwise pytype's C extensions try to build against the wrong python header files and fail. i tried a bunch of things, and wgetting the binaries and extracting them as root was the only thing that worked. (this is not an entirely new thing; travis runs the same wget command when we ask for python 3.5, which is how i found out which server to get the binaries from)
@@ -79,3 +79,13 @@ stdlib/3/tokenize.pyi # parse only | |||
stdlib/3/types.pyi # parse only | |||
stdlib/3/urllib/error.pyi # parse only | |||
stdlib/3/urllib/request.pyi # parse only | |||
stdlib/3/collections/abc.pyi # parse only |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, why were these added?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they're temporarily broken while i add some features to pytype; i just didn't want them to block this PR. i'll do a separate PR in a few days cleaning out a lot of the blacklist.
tests/pytype_test.py
Outdated
print('Cannot run pytd. Did you install pytype?') | ||
return 0, 0 | ||
|
||
skip, parse_only = load_blacklist() | ||
skip, parse_only = load_blacklist(dirs) | ||
wanted = re.compile(r'stdlib/.*\.pyi$') | ||
skipped = re.compile('(%s)$' % '|'.join(skip)) | ||
parse_only = re.compile('(%s)$' % '|'.join(parse_only)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not directly related to this PR, but while you're here: Could you handle the case of skip
or parse_only
being empty lists?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@JelleZijlstra: Let me know if you have any other ideas/comments about the |
I am fine with this version. |
* python/master: (269 commits) allow Optional[float] for asyncio.wait() timeout arg (python#1860) Clean up the pytype blacklist. (python#1851) filter function: make the callable of the first overload non-optional so that mypy is able to select the second overload for the case. (python#1855) Accept Text in distutils *Version classes (python#1854) Add attrs library (python#1838) Add type stub for the lzma module (python#1844) Add ImportError attributes name, path for Python 3.3+ (python#1849) Add 'message' to click.decorators.version_option (python#1845) PEP 553 and PEP 564 (python#1846) Adding py36 additions to datetime module (python#1848) Remove incomplete thrift stubs that cause false positives. (python#1827) adding curses.ascii, curses.panel and curses.textpad modules (python#1792) Fix requests session hooks type (python#1794) Add StringIO.name and BytesIO.name (python#1802) Fix werkzeug environ type (python#1831) Change the return type of unittest.TestCase.fail() to NoReturn (python#1843) _curses.tparm works on bytes, not str (python#1828) Refine types for distutils.version (python#1835) Add stub for stat.filemode (python#1837) modify pytype_test to run from within pytype too, and support python3 (python#1817) ...
pytype needs to run in a python2.7 venv, but also to have a python3.6 interpreter available to shell out to. this PR adds support for python3 stdlib testing to pytype_tests, and installs a python3.6 interpreter in the travis config.