Skip to content

A few tiny micro-optimizations/simplifications #7238

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
May 23, 2020
Merged

A few tiny micro-optimizations/simplifications #7238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8d841ab
nodes: remove unused argument from FSHookProxy
bluetech May 19, 2020
139a029
terminal: remove a redundant line
bluetech May 19, 2020
796fba6
terminal: remove redundant write_fspath_result call
bluetech May 19, 2020
f1f9c77
Import `packaging` package lazily
bluetech May 13, 2020
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
4 changes: 3 additions & 1 deletion src/_pytest/config/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

import attr
import py
from packaging.version import Version
from pluggy import HookimplMarker
from pluggy import HookspecMarker
from pluggy import PluginManager
Expand Down Expand Up @@ -1059,6 +1058,9 @@ def _checkversion(self):

minver = self.inicfg.get("minversion", None)
if minver:
# Imported lazily to improve start-up time.
from packaging.version import Version

if Version(minver) > Version(pytest.__version__):
raise pytest.UsageError(
"%s:%d: requires pytest-%s, actual pytest-%s'"
Expand Down
7 changes: 2 additions & 5 deletions src/_pytest/nodes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,10 +456,7 @@ def _check_initialpaths_for_relpath(session, fspath):


class FSHookProxy:
def __init__(
self, fspath: py.path.local, pm: PytestPluginManager, remove_mods
) -> None:
self.fspath = fspath
def __init__(self, pm: PytestPluginManager, remove_mods) -> None:
self.pm = pm
self.remove_mods = remove_mods

Expand Down Expand Up @@ -510,7 +507,7 @@ def _gethookproxy(self, fspath: py.path.local):
remove_mods = pm._conftest_plugins.difference(my_conftestmodules)
if remove_mods:
# one or more conftests are not in use at this fspath
proxy = FSHookProxy(fspath, pm, remove_mods)
proxy = FSHookProxy(pm, remove_mods)
else:
# all plugins are active for this fspath
proxy = self.config.hook
Expand Down
5 changes: 3 additions & 2 deletions src/_pytest/outcomes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
from typing import Optional
from typing import TypeVar

from packaging.version import Version

TYPE_CHECKING = False # avoid circular import through compat

if TYPE_CHECKING:
Expand Down Expand Up @@ -217,6 +215,9 @@ def importorskip(
return mod
verattr = getattr(mod, "__version__", None)
if minversion is not None:
# Imported lazily to improve start-up time.
from packaging.version import Version

if verattr is None or Version(verattr) < Version(minversion):
raise Skipped(
"module %r has __version__ %r, required is: %r"
Expand Down
8 changes: 2 additions & 6 deletions src/_pytest/terminal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,8 +443,7 @@ def pytest_runtest_logstart(self, nodeid, location):
self.write_ensure_prefix(line, "")
self.flush()
elif self.showfspath:
fsid = nodeid.split("::")[0]
self.write_fspath_result(fsid, "")
Copy link
Member

Choose a reason for hiding this comment

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

This is now different, correct? Or does is it have the same outcome?

Copy link
Member Author

Choose a reason for hiding this comment

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

write_fspath_result does this:

fspath = self.config.rootdir.join(nodeid.split("::")[0])

(and doesn't use nodeid otherwise) so the split is redundant here.

self.write_fspath_result(nodeid, "")
self.flush()

def pytest_runtest_logreport(self, report: TestReport) -> None:
Expand Down Expand Up @@ -474,10 +473,7 @@ def pytest_runtest_logreport(self, report: TestReport) -> None:
else:
markup = {}
if self.verbosity <= 0:
if not running_xdist and self.showfspath:
self.write_fspath_result(rep.nodeid, letter, **markup)
Copy link
Member

Choose a reason for hiding this comment

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

This is never reached?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is reached, but has the same outcome as just the write.

What write_fspath_result does is check whether the path (extracted from the nodeid) is different than the last call to write_fspath_result, and if so, starts a new line and prints the path, before printing the argument.

The reason it's not needed here is that pytest_runtest_logstart already calls write_fspath_result, so the call here (logreport) doesn't.

Because the check it performs is mildly expensive, it's good to avoid it when possible.

else:
self._tw.write(letter, **markup)
self._tw.write(letter, **markup)
else:
self._progress_nodeids_reported.add(rep.nodeid)
line = self._locationline(rep.nodeid, *rep.location)
Expand Down