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

[WIP] Issue 424/add saferepr #460

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 7 additions & 3 deletions src/pluggy/_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from typing import Union

from ._result import Result
from ._tracing import saferepr


_T = TypeVar("_T")
Expand Down Expand Up @@ -456,7 +457,7 @@
self._hookimpls.insert(i + 1, hookimpl)

def __repr__(self) -> str:
return f"<HookCaller {self.name!r}>"
Copy link
Member

Choose a reason for hiding this comment

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

Why blindly replace all usages?

return f"<HookCaller {saferepr(self.name)}>"

def _verify_all_args_are_provided(self, kwargs: Mapping[str, object]) -> None:
# This is written to avoid expensive operations when not needed.
Expand Down Expand Up @@ -609,7 +610,7 @@
return self._orig._call_history

def __repr__(self) -> str:
return f"<_SubsetHookCaller {self.name!r}>"
return f"<_SubsetHookCaller {saferepr(self.name)}>"


@final
Expand Down Expand Up @@ -667,7 +668,10 @@
self.trylast: Final = hook_impl_opts["trylast"]

def __repr__(self) -> str:
return f"<HookImpl plugin_name={self.plugin_name!r}, plugin={self.plugin!r}>"
return (
f"<HookImpl plugin_name={saferepr(self.plugin_name)}, "
f"plugin={saferepr(self.plugin)}>"
)

Check warning on line 674 in src/pluggy/_hooks.py

View check run for this annotation

Codecov / codecov/patch

src/pluggy/_hooks.py#L674

Added line #L674 was not covered by tests


@final
Expand Down
100 changes: 100 additions & 0 deletions src/pluggy/_tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"""
from __future__ import annotations

import reprlib
from typing import Any
from typing import Callable
from typing import Sequence
Expand Down Expand Up @@ -60,6 +61,105 @@
self._tags2proc[tags] = processor


def _try_repr_or_str(obj: object) -> str:
try:
return repr(obj)
except (KeyboardInterrupt, SystemExit):
raise
except BaseException:
return f'{type(obj).__name__}("{obj}")'


Check warning on line 72 in src/pluggy/_tracing.py

View check run for this annotation

Codecov / codecov/patch

src/pluggy/_tracing.py#L71-L72

Added lines #L71 - L72 were not covered by tests
def _format_repr_exception(exc: BaseException, obj: object) -> str:
try:
exc_info = _try_repr_or_str(exc)
except (KeyboardInterrupt, SystemExit):
raise
except BaseException as exc:
exc_info = f"unpresentable exception ({_try_repr_or_str(exc)})"
return "<[{} raised in repr()] {} object at 0x{:x}>".format(
exc_info, type(obj).__name__, id(obj)
)


def _ellipsize(s: str, maxsize: int) -> str:
if len(s) > maxsize:
i = max(0, (maxsize - 3) // 2)
j = max(0, maxsize - 3 - i)

x = len(s) - j
return s[:i] + "..." + s[x:]
return s


class SafeRepr(reprlib.Repr):
"""
repr.Repr that limits the resulting size of repr() and includes
information on exceptions raised during the call.
"""

def __init__(self, maxsize: int | None, use_ascii: bool = False) -> None:
"""
:param maxsize:
If not None, will truncate the resulting repr to that specific size, using ellipsis
somewhere in the middle to hide the extra text.
If None, will not impose any size limits on the returning repr.
"""
super().__init__()
# ``maxstring`` is used by the superclass, and needs to be an int; using a
# very large number in case maxsize is None, meaning we want to disable
# truncation.
self.maxstring = maxsize if maxsize is not None else 1_000_000_000
self.maxsize = maxsize
self.use_ascii = use_ascii

def repr(self, x: object) -> str:
try:
if self.use_ascii:
s = ascii(x)
else:
s = super().repr(x)

except (KeyboardInterrupt, SystemExit):
raise
except BaseException as exc:
s = _format_repr_exception(exc, x)
if self.maxsize is not None:
s = _ellipsize(s, self.maxsize)
return s

def repr_instance(self, x: object, level: int) -> str:
try:
s = repr(x)
except (KeyboardInterrupt, SystemExit):
raise
except BaseException as exc:
s = _format_repr_exception(exc, x)
if self.maxsize is not None:
s = _ellipsize(s, self.maxsize)
return s


# Maximum size of overall repr of objects to display during assertion errors.
DEFAULT_REPR_MAX_SIZE = 240


def saferepr(
obj: object, maxsize: int | None = DEFAULT_REPR_MAX_SIZE, use_ascii: bool = False
) -> str:
"""Return a size-limited safe repr-string for the given object.

Failing __repr__ functions of user instances will be represented
with a short exception info and 'saferepr' generally takes
care to never raise exceptions itself.

This function is a wrapper around the Repr/reprlib functionality of the
stdlib.
"""

return SafeRepr(maxsize, use_ascii).repr(obj)


class TagTracerSub:
def __init__(self, root: TagTracer, tags: tuple[str, ...]) -> None:
self.root = root
Expand Down
5 changes: 3 additions & 2 deletions testing/benchmark.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from pluggy import PluginManager
from pluggy._callers import _multicall
from pluggy._hooks import HookImpl
from pluggy._tracing import saferepr


hookspec = HookspecMarker("example")
Expand Down Expand Up @@ -77,7 +78,7 @@ def __init__(self, num: int) -> None:
self.num = num

def __repr__(self) -> str:
return f"<Plugin {self.num}>"
return f"<Plugin {saferepr(self.num)}>"

@hookimpl
def fun(self, hooks, nesting: int) -> None:
Expand All @@ -89,7 +90,7 @@ def __init__(self, num: int) -> None:
self.num = num

def __repr__(self) -> str:
return f"<PluginWrap {self.num}>"
return f"<PluginWrap {saferepr(self.num)}>"

@hookimpl(wrapper=True)
def fun(self):
Expand Down
71 changes: 68 additions & 3 deletions testing/test_hookcaller.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
from pluggy import PluginValidationError
from pluggy._hooks import HookCaller
from pluggy._hooks import HookImpl
from pluggy._hooks import HookimplOpts
from pluggy._tracing import saferepr

hookspec = HookspecMarker("example")
hookimpl = HookimplMarker("example")
Expand Down Expand Up @@ -409,7 +411,8 @@

pm.add_hookspecs(Api)

# make sure a bad signature still raises an error when using specname
"""make sure a bad signature still raises an error when using specname"""

class Plugin:
@hookimpl(specname="hello")
def foo(self, arg: int, too, many, args) -> int:
Expand All @@ -418,8 +421,9 @@
with pytest.raises(PluginValidationError):
pm.register(Plugin())

# make sure check_pending still fails if specname doesn't have a
# corresponding spec. EVEN if the function name matches one.
"""make sure check_pending still fails if specname doesn't have a
corresponding spec. EVEN if the function name matches one."""

class Plugin2:
@hookimpl(specname="bar")
def hello(self, arg: int) -> int:
Expand Down Expand Up @@ -448,3 +452,64 @@
"Hook 'conflict' is already registered within namespace "
"<class 'test_hookcaller.test_hook_conflict.<locals>.Api1'>"
)


def test_hook_impl_initialization() -> None:
# Mock data
plugin = "example_plugin"
plugin_name = "ExamplePlugin"

def example_function(x):
return x

Check warning on line 463 in testing/test_hookcaller.py

View check run for this annotation

Codecov / codecov/patch

testing/test_hookcaller.py#L463

Added line #L463 was not covered by tests

hook_impl_opts: HookimplOpts = {
"wrapper": False,
"hookwrapper": False,
"optionalhook": False,
"tryfirst": False,
"trylast": False,
"specname": "",
}

# Initialize HookImpl
hook_impl = HookImpl(plugin, plugin_name, example_function, hook_impl_opts)

# Verify attributes are set correctly
assert hook_impl.function == example_function
Copy link
Member

Choose a reason for hiding this comment

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

that type of test adds no value as far as can tell - lets keep experiments out of the codebase

assert hook_impl.argnames == ("x",)
assert hook_impl.kwargnames == ()
assert hook_impl.plugin == plugin
assert hook_impl.opts == hook_impl_opts
assert hook_impl.plugin_name == plugin_name
assert not hook_impl.wrapper
assert not hook_impl.hookwrapper
assert not hook_impl.optionalhook
assert not hook_impl.tryfirst
assert not hook_impl.trylast


def test_hook_impl_representation() -> None:
# Mock data
plugin = "example_plugin"
plugin_name = "ExamplePlugin"

def example_function(x):
return x

Check warning on line 497 in testing/test_hookcaller.py

View check run for this annotation

Codecov / codecov/patch

testing/test_hookcaller.py#L497

Added line #L497 was not covered by tests

hook_impl_opts: HookimplOpts = {
"wrapper": False,
"hookwrapper": False,
"optionalhook": False,
"tryfirst": False,
"trylast": False,
"specname": "",
}

# Initialize HookImpl
hook_impl = HookImpl(plugin, plugin_name, example_function, hook_impl_opts)

# Verify __repr__ method
expected_repr = (
f"<HookImpl plugin_name={saferepr(plugin_name)}, " f"plugin={saferepr(plugin)}>"
Copy link
Member

Choose a reason for hiding this comment

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

we shouldnt add safe-repr all over the place for everything

)
assert repr(hook_impl) == expected_repr