Fix fspath warnings in nonpython.rst example

[The-Compiler]

Looks like something we should fix in the example

Originally posted by @The-Compiler in #8814 (comment)

[The-Compiler]

So I'm looking at #8840 and using a private pytest helper in an example seems like an odd thing to me. After all, we'd want users to be able to do this kind of thing without having to reach into pytest internals...

I see there's a TODO about it in 77cb110 - but now we're in a situation where pytest.Item.reportinfo is public API, being implemented by some plugins, and our example implementation either needs to access private pytest APIs or deprecated API... 😟

What to do about this? I can see a couple of ways forward:

• Provide a public legacy_path helper and explain that pytest.legacy_path(self.path) should be used for the first return value of reportinfo at the moment. Downside: More churn once we have a proper solution to the problem.
• Un-deprecate the fspath attribute until pytest itself is fully pathlib aware.
• Make it possible for reportinfo to return Union[LEGACY_PATH, pathlib.Path, str] and handle it everywhere reportinfo is used, so that custom reportinfo implementations can return self.path. I'm wondering why there was this TODO left? Where there some problems with this approach? It looks like reportinfo is only used in three places in src/, and from a very quick glance, all of them just seem to turn the path into a string?
• Given that most projects only seem to want to customize the third item, suggest instead doing something like:

path, line, _name = super().reportinfo()
return path, line, f"usecase: {self.name}"

cc @RonnyPfannschmidt @bluetech @nicoddemus because I think those were involved in the pathlib migration mostly.

[bluetech]

Un-deprecate the fspath attribute until pytest itself is fully pathlib aware.

FWIW, this is what I think we should do. I think the py.path deprecations should all be done in one shot with proper documentation, guidance, migration plan etc. which is not ready yet. See #7259 for discussion. Unfortunately I haven't had time to work on it this last month.

[bluetech]

(reportinfo in particular is one of the hold outs because it's difficult to migrate - usual 90% work is done, 90% work remaining situation :) But I suggest using #7259 for discussion on how to deal with it)

[RonnyPfannschmidt]

Let's defer the warning

[nicoddemus]

Make it possible for reportinfo to return Union[LEGACY_PATH, pathlib.Path, str] and handle it everywhere reportinfo is used

I like that one actually, because it is future proof.

But I agree we should defer the warning until pytest itself is sorted out and we have complete documentation on how users can do it.

[bluetech]

It is not compatible for users/plugins which call reportinfo.

[nicoddemus]

Ahh true.

[The-Compiler]

It is not compatible for users/plugins which call reportinfo.

Is this really something we should have to worry about? I did a quick search in my almost complete corpus of plugins, and found:

$ rg -F .reportinfo
allure-pytest/allure/pytest_plugin.py
115:            description = item.reportinfo()[2]

python-pytest-cases/pytest_cases/case_parametrizer_new.py
1224:#             fspath, lineno, _ = item.reportinfo()

agent-python-pytest/pytest_reportportal/service.py
705:            return test_item.reportinfo()[2]

pytest-allure-adaptor2/allure/pytest_plugin.py
164:            description = item.reportinfo()[2]

The pytest-cases one is commented out (and only does str(fspath) for a sorting key); all others only actually need the description.

A grep.app search is similarly boring and mostly shows vendored pytest copies. Not sure if it gets used somewhere indirectly - but if it is, pytest could still turn the value into a legacy path for that.

(Decided to keep the discussion in here for now, as this issue is specifically about what to do with reportinfo, while #7259 is about a lot more things - though there are some previous discussions about reportinfo there)

[bluetech]

@The-Compiler according to this analysis, it seems like the way to go as the breakage (if any) will be minimal -- when I thought about it (not too much) I couldn't think of a nice way to do this in a backward compatible way.

Ideally we'd get rid of reportinfo entirely or modify it more substantially as it's not a very nice interface at all (not extensible, dubious types) and also hurts performance (calculating stuff that goes unused - I remember it showing up frequently in benchmarks). But that's probably not possible in one stroke.

[The-Compiler]

Yeah, and I feel like this is out of scope for 7.0 - right now I'd really like to focus on picking up the lose ends of whatever (great!) work we've done so far, so that we can finally get to another release.

So what's the way forward here? @bluetech do you want to open a PR to make it possible for reportinfo to return pathlib.Path (probably with a "breaking" changelog pointing that out)? (see below first)

However, this means plugins implementing reportinfo (of which there are around 40, from what I can tell) would still need to change to something like:

def reportinfo():
    try:
        path = self.path
    except AttributeError:
        path = self.fspath

(or the super() approach outlined above) if they want to stay compatible with both major versions. So perhaps if we know we want to re-think reportinfo anyways (for 8.0 potentially?), then the path of least friction would be to remove the warning for the fspath attribute and defer that until that's taken care of? Then current reportinfo implementations could stay as-is, and plugins wouldn't need to keep compatible with three different APIs if they want to support 6.x, 7.x and 8.x.

[nicoddemus]

then the path of least friction would be to remove the warning for the fspath attribute and defer that until that's taken care of?

I agree with removing the warning: that simple to do and we can reintroduce the warning later once we have a clear solution moving forward.

If everybody agrees I can work on a PR.