Skip to content

Commit

Permalink
fixes after review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
jakkdl committed May 5, 2023
1 parent b1e2b8c commit 648cdfc
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 44 deletions.
2 changes: 1 addition & 1 deletion trio/_path.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ class Path(metaclass=AsyncAutoWrapperType):
def __init__(self, *args):
self._wrapped = pathlib.Path(*args)

# type checkers allow accessing any attributes on classes with `__getattr__`
def __getattr__(self, name):
if name in self._forward:
value = getattr(self._wrapped, name)
Expand Down Expand Up @@ -188,7 +189,6 @@ async def open(self, *args, **kwargs):
__truediv__ = pathlib.Path.__truediv__
__rtruediv__ = pathlib.Path.__rtruediv__

# Note: these are not parsed properly by jedi.
# It might be superior to just manually implement all the methods and get rid
# of all the magic wrapping stuff.

Expand Down
106 changes: 63 additions & 43 deletions trio/tests/test_exports.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,10 @@ def no_underscores(symbols):
pytest.skip("mypy not installed in tests on pypy")

# create py.typed file
(Path(trio.__file__).parent / "py.typed").write_text("")
py_typed_path = Path(trio.__file__).parent / "py.typed"
py_typed_exists = py_typed_path.exists()
if not py_typed_exists:
py_typed_path.write_text("")

# mypy behaves strangely when passed a huge semicolon-separated line with `-c`
# so we use a tmpfile
Expand All @@ -122,6 +125,10 @@ def no_underscores(symbols):

res = run(["--config-file=", "--follow-imports=silent", str(tmpfile)])

# clean up created py.typed file
if not py_typed_exists:
py_typed_path.unlink()

# check that there were no errors (exit code 0), otherwise print the errors
assert res[2] == 0, res[0]
return
Expand All @@ -146,7 +153,7 @@ def no_underscores(symbols):
# this could be sped up by only invoking mypy once per module, or even once for all
# modules, instead of once per class.
@slow
# see commend on test_static_tool_sees_all_symbols
# see comment on test_static_tool_sees_all_symbols
@pytest.mark.redistributors_should_skip
# pylint/jedi often have trouble with alpha releases, where Python's internals
# are in flux, grammar may not have settled down, etc.
Expand All @@ -167,13 +174,19 @@ def no_hidden(symbols):
if (not symbol.startswith("_")) or symbol.startswith("__")
}

py_typed_path = Path(trio.__file__).parent / "py.typed"
py_typed_exists = py_typed_path.exists()

if tool == "mypy":
if sys.implementation.name != "cpython":
pytest.skip("mypy not installed in tests on pypy")
# create py.typed file
(Path(trio.__file__).parent / "py.typed").write_text("")
# not marked with no-cover pragma, remove this logic when trio is marked
# with py.typed proper
if not py_typed_exists:
py_typed_path.write_text("")

errors: dict[str, Any] = {}
errors: dict[str, object] = {}
if module_name == "trio.tests":
return
for class_name, class_ in module.__dict__.items():
Expand All @@ -182,6 +195,7 @@ def no_hidden(symbols):
if module_name == "trio.socket" and class_name in dir(stdlib_socket):
continue
# Deprecated classes are exported with a leading underscore
# We don't care about errors in _MultiError as that's on its way out anyway
if class_name.startswith("_"): # pragma: no cover
continue

Expand Down Expand Up @@ -239,7 +253,7 @@ def no_hidden(symbols):
extra.remove("receive_stream")
extra.remove("send_stream")

if missing | extra:
if missing or extra:
errors[f"{module_name}.{class_name}"] = {
"missing": missing,
"extra": extra,
Expand All @@ -258,51 +272,57 @@ def no_hidden(symbols):
"--config-file=",
"--follow-imports=silent",
"--disable-error-code=operator",
"--soft-error-limit=-1",
"--no-error-summary",
str(tmpfile),
]
)
if res[2] != 0:
print(res)
it = iter(res[0].split("\n")[:-2])
for output_line in it:
# hack to get around windows path starting with <drive>:<path>
output_line = output_line[2:]

_, line, error_type, message = output_line.split(":")

# -2 due to lines being 1-indexed and to skip the import line
symbol = (
f"{module_name}.{class_name}."
+ sorted_runtime_names[int(line) - 2]
)

# The POSIX-only attributes get listed in `dir(trio.Path)` since
# they're in `dir(pathlib.Path)` on win32 cpython. This should *maybe*
# be fixed in the future, but for now we ignore it.
if (
symbol
in ("trio.Path.group", "trio.Path.owner", "trio.Path.is_mount")
and sys.platform == "win32"
and sys.implementation.name == "cpython"
):
continue

# a bunch of symbols have this error, e.g. trio.lowlevel.Task.context
# but I don't think that's a problem - and if it is it would likely
# be picked up by "proper" mypy tests elsewhere
if "conflicts with class variable access" in message:
continue

errors[symbol] = error_type + ":" + message
# no errors
if res[2] == 0:
continue

# get each line of output, containing an error for a symbol,
# stripping of trailing newline
it = iter(res[0].split("\n")[:-1])
for output_line in it:
# split out the three last fields to not have problems with windows
# drives or other paths with any `:`
_, line, error_type, message = output_line.rsplit(":", 3)

# -2 due to lines being 1-indexed and to skip the import line
symbol = (
f"{module_name}.{class_name}." + sorted_runtime_names[int(line) - 2]
)

# The POSIX-only attributes get listed in `dir(trio.Path)` since
# they're in `dir(pathlib.Path)` on win32 cpython. This should *maybe*
# be fixed in the future, but for now we ignore it.
if (
symbol
in ("trio.Path.group", "trio.Path.owner", "trio.Path.is_mount")
and sys.platform == "win32"
and sys.implementation.name == "cpython"
):
continue

# a bunch of symbols have this error, e.g. trio.lowlevel.Task.context
# It's not a problem: it's just complaining we're accessing
# instance-only attributes on a class!
# See this test for a minimized version that causes this error:
# https://github.com/python/mypy/blob/c517b86b9ba7487e7758f187cf31478e7aeaad47/test-data/unit/check-slots.test#L515-L523.

if "conflicts with class variable access" in message:
continue

errors[symbol] = error_type + ":" + message

else: # pragma: no cover
assert False
assert False, "unknown tool"

if errors: # pragma: no cover
from pprint import pprint
# clean up created py.typed file
if tool == "mypy" and not py_typed_exists:
py_typed_path.unlink()

print(f"\n{tool} can't see the following symbols in {module_name}:")
pprint(errors)
assert not errors


Expand Down

0 comments on commit 648cdfc

Please sign in to comment.