Skip to content
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

Export test fixes #2887

Merged
merged 9 commits into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/trio/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
"""
from __future__ import annotations

from typing import TYPE_CHECKING

# General layout:
#
# trio/_core/... is the self-contained core library. It does various
Expand Down Expand Up @@ -111,7 +113,7 @@

# Not imported by default, but mentioned here so static analysis tools like
# pylint will know that it exists.
if False:
if TYPE_CHECKING:
from . import testing

from . import _deprecate as _deprecate
Expand Down Expand Up @@ -154,3 +156,4 @@
fixup_module_metadata(from_thread.__name__, from_thread.__dict__)
fixup_module_metadata(to_thread.__name__, to_thread.__dict__)
del fixup_module_metadata
del TYPE_CHECKING
23 changes: 5 additions & 18 deletions src/trio/_tests/test_exports.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@
ast = linter.get_ast(module.__file__, modname)
static_names = no_underscores(ast) # type: ignore[arg-type]
elif tool == "jedi":
if sys.implementation.name != "cpython":
pytest.skip("jedi does not support pypy")

Check warning on line 161 in src/trio/_tests/test_exports.py

View check run for this annotation

Codecov / codecov/patch

src/trio/_tests/test_exports.py#L161

Added line #L161 was not covered by tests
Comment on lines +160 to +161
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if sys.implementation.name != "cpython":
pytest.skip("jedi does not support pypy")
if sys.implementation.name != "cpython": # pragma: no branch
pytest.skip("jedi does not support pypy")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels weird though, like I don't think codecov should be complaining about this :(

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, this is the issue where we have double codecov runs on some PRs - and the inline errors are taken from the "incorrect" codecov run, I've seen this a couple times.
That's also the same as #2887 (comment) not getting updated.

I was gonna try and link them .. when I noticed that one set links to https://app.codecov.io/gh/python-trio/trio/commit/c74f51bf891c940d4ec7ed5859e55ab91a2a1809 and the other links to https://github.com/python-trio/trio/pull/2887/checks?check_run_id=19006357499 in the "details" link in the summary. Which is maybe a good hint to what's causing it?

We need some more messing around with codecov 🙃, see also #2689


try:
import jedi
except ImportError as error:
Expand Down Expand Up @@ -195,7 +198,8 @@
)
elif tool == "pyright_verifytypes":
if not RUN_SLOW: # pragma: no cover
pytest.skip("use --run-slow to check against mypy")
pytest.skip("use --run-slow to check against pyright")

try:
import pyright # noqa: F401
except ImportError as error:
Expand All @@ -213,26 +217,9 @@
for x in current_result["typeCompleteness"]["symbols"]
if x["name"].startswith(modname)
}

# pyright ignores the symbol defined behind `if False`
if modname == "trio":
static_names.add("testing")

# these are hidden behind `if sys.platform != "win32" or not TYPE_CHECKING`
# so presumably pyright is parsing that if statement, in which case we don't
# care about them being missing.
if modname == "trio.socket" and sys.platform == "win32":
ignored_missing_names = {"if_indextoname", "if_nameindex", "if_nametoindex"}
assert static_names.isdisjoint(ignored_missing_names)
static_names.update(ignored_missing_names)

else: # pragma: no cover
raise AssertionError()

# mypy handles errors with an `assert` in its branch
if tool == "mypy":
return

# It's expected that the static set will contain more names than the
# runtime set:
# - static tools are sometimes sloppy and include deleted names
Expand Down
12 changes: 9 additions & 3 deletions src/trio/socket.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,19 @@
ntohs as ntohs,
)

# TODO: update python docs to reflect this
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mostly just this that's a blocker

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I made a typeshed PR thinking "oh well the stdlib docs are the spec, so PyPy is deviating from it and should not have the docs update for it" but then now I realize sys.implementation.name isn't a guard typeshed allows :(

And it looks like these functions just never exist on PyPy. Lol.

if sys.implementation.name == "cpython":
from socket import (
if_indextoname as if_indextoname,
if_nameindex as if_nameindex,
if_nametoindex as if_nametoindex,
)


# not always available so expose only if
if sys.platform != "win32" or not _t.TYPE_CHECKING:
with _suppress(ImportError):
from socket import (
if_indextoname as if_indextoname,
if_nameindex as if_nameindex,
if_nametoindex as if_nametoindex,
sethostname as sethostname,
)

Expand Down