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

Item.reportinfo is missing documentation #6689

Open
The-Compiler opened this issue Feb 7, 2020 · 3 comments
Open

Item.reportinfo is missing documentation #6689

The-Compiler opened this issue Feb 7, 2020 · 3 comments
Labels
type: docs documentation improvement, missing or needing clarification type: refactoring internal improvements to the code

Comments

@The-Compiler
Copy link
Member

In the documentation, we have an example for working with non-python tests. There, we have:

class YamlItem(pytest.Item):

    ...

    def reportinfo(self):
        return self.fspath, 0, "usecase: {}".format(self.name)

My first question when seeing this is "what does that tuple I return mean". Unfortunately, the API references for Item and Node don't help. Even reading the source doesn't help much:

    def reportinfo(self) -> Tuple[Union[py.path.local, str], Optional[int], str]:
        return self.fspath, None, ""

Finally, looking at the Python plugin in pytest, things get clearer:

        return fspath, lineno, modpath

but this should really be documented somewhere. Also, it's unclear to me why the example above uses 0 rather than None.

@Zac-HD
Copy link
Member

Zac-HD commented Feb 7, 2020

Seems like a perfect case for a namedtuple (in this case typing.NamedTuple) to me - backwards-compatible and much more descriptive!

@Zac-HD Zac-HD added type: docs documentation improvement, missing or needing clarification type: refactoring internal improvements to the code labels Feb 7, 2020
@nicoddemus
Copy link
Member

Seems like a perfect case for a namedtuple (in this case typing.NamedTuple) to me - backwards-compatible and much more descriptive!

Definitely, but unfortunately doing so would be an API breakage because reportinfo can be overriden by subclasses in plugins 😕.

For that reason I think we should stick to document it properly only.

@RonnyPfannschmidt
Copy link
Member

we can follow-up with documenting the return value in terms of it and then putting up a union for it to sort it out as a migration for consumers/implementers (aka if we need to cast a return value, warn)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: docs documentation improvement, missing or needing clarification type: refactoring internal improvements to the code
Projects
None yet
Development

No branches or pull requests

4 participants