Add type hints to the remaining tests #1040
Conversation
| @@ -376,25 +386,11 @@ def can_write_dir(path): | |||
| return os.path.isdir(path) and os.access(path, os.R_OK | os.W_OK | os.X_OK) | |||
|
|
|||
|
|
|||
| def touch(file, times=None): | |||
There was a problem hiding this comment.
We never used the times kwarg. It's really tricky to get right with type hints, so I took the liberty to simplify this function.
I figure that we can add back the times part if we need it.
| def yield_files(directory): | ||
| for root, _, files in os.walk(directory): | ||
| for f in files: | ||
| filename = os.path.join(root, f) | ||
| rel_filename = os.path.relpath(filename, directory) | ||
| yield filename, rel_filename | ||
|
|
||
|
|
||
| def write_zipfile(directory, dest, reverse=False): | ||
| with open_zip(dest, "w") as zf: | ||
| for filename, rel_filename in sorted(yield_files(directory), reverse=reverse): | ||
| zf.write(filename, arcname=rel_filename) | ||
| return dest |
| @@ -135,7 +137,7 @@ def make_project( | |||
| os.path.join(name, "my_module.py"): 'def do_something():\n print("hello world!")\n', | |||
| os.path.join(name, "package_data/resource1.dat"): 1000, | |||
| os.path.join(name, "package_data/resource2.dat"): 1000, | |||
| } | |||
| } # type: Dict[str, Union[str, int]] | |||
There was a problem hiding this comment.
This is the best we can do with type casting. We can't use cast() because we can't use values like Dict at runtime.
There was a problem hiding this comment.
See below, but this is untrue, you just need to pass the type to cast as a string.
| @@ -957,6 +996,7 @@ def test_pex_multi_resolve_2(): | |||
|
|
|||
| @contextmanager | |||
| def pex_manylinux_and_tag_selection_context(): | |||
| # type: () -> Iterator[Tuple[Callable[[str, str, str, str, Optional[str]], None], Callable[[str, str, str, str], None]]] | |||
There was a problem hiding this comment.
Absolutely disgusting. But we can't use type aliases because those require setting variables at runtime, and that would break Python 2.
There was a problem hiding this comment.
This is untrue. You can define the inside the if TYPE_CHECKING guard, and, if you need to cast to them, pass those type aliases to cast as strings.
| assert b"This is an exception" in so, "Standard out was: %s" % so | ||
| assert b"This is an exception" in so, "Standard out was: %r" % so |
There was a problem hiding this comment.
MyPy warns that this will cause you to bring b"abc", with the b prefix, when using Py3. It wants us to be explicit about that.
Now that we have full type coverage for the tests, we'll get useful validation when adding hints to our source code that the hints line up.