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

pytest_report_teststatus documentation is contradictory #10872

Closed
samuelcolvin opened this issue Apr 5, 2023 · 6 comments
Closed

pytest_report_teststatus documentation is contradictory #10872

samuelcolvin opened this issue Apr 5, 2023 · 6 comments
Labels
good first issue easy issue that is friendly to new contributor type: docs documentation improvement, missing or needing clarification

Comments

@samuelcolvin
Copy link
Member

samuelcolvin commented Apr 5, 2023

See here:

image

The example return value is "rerun", "R", ("RERUN", {"yellow": True}), while the return type is documented as

Tuple[str, str, Union[str, Mapping[str, bool]]]

which don't match.

I'm assuming the return type should be

Tuple[str, str, Union[str, Tuple[str, Mapping[str, bool]]]]

Or something?

Also some clearer documentation on how the third element of the tuple is sued would be great.

@nicoddemus
Copy link
Member

nicoddemus commented Apr 5, 2023

Indeed the docs are wrong, and the annotation in the code is correct:

res: Tuple[
str, str, Union[str, Tuple[str, Mapping[str, bool]]]
] = self.config.hook.pytest_report_teststatus(report=rep, config=self.config)

The return value can be, for example:

("rerun", "R", "RERUN")

Or:

("rerun", "R", ("RERUN", {"yellow": True}))

This signature is like that for historical reasons unfortunately, nowadays we would use probably a dataclass.

@nicoddemus nicoddemus added type: docs documentation improvement, missing or needing clarification good first issue easy issue that is friendly to new contributor labels Apr 5, 2023
@nicoddemus
Copy link
Member

The possible values that can be used in the mapping are not really documented I think, they are defined here:

_esctable = dict(
black=30,
red=31,
green=32,
yellow=33,
blue=34,
purple=35,
cyan=36,
white=37,
Black=40,
Red=41,
Green=42,
Yellow=43,
Blue=44,
Purple=45,
Cyan=46,
White=47,
bold=1,
light=2,
blink=5,
invert=7,
)

@samuelcolvin
Copy link
Member Author

makes sense, thanks for the details.

@The-Compiler
Copy link
Member

This signature is like that for historical reasons unfortunately, nowadays we would use probably a dataclass.

I feel like we should do just that, and figure out a way to make it backwards compatible.

If pytest itself would be the only place invoking the hook, it'd just be a matter of an isinstance - if we want to keep compatibility with some 15-20 projects invoking the hook themselves, returning an object with a __getitem__ and __iter__ implemented (perhaps emitting a DeprecationWarning) would probably work smoothly.

@nicoddemus
Copy link
Member

Perhaps a named tuple would work too?

@RonnyPfannschmidt
Copy link
Member

I wonder if we could try to mirror rich styled spans and supply easy conversation

This is one of the rarely used apis that tie us into the legacy terminalwriter apis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue easy issue that is friendly to new contributor type: docs documentation improvement, missing or needing clarification
Projects
None yet
Development

No branches or pull requests

4 participants