pathlib: Fix module path used when using importlib with namespaces

[isra17]

Closes #12044

Will do other steps from the template, but before I would like a confirmation that this fix make sense.

It does fix the issue of assertion rewriting mixed with namespace package on in my environment.

If fixes both the cases where the tests lives in a package with __init__.py and with the config consider_namespace_packages = true. I am not familiar at all with this part of the code (or any to be honest), so I'm unsure what are the consequence, but from my understanding the from_spec(module, path) path parameter should take the module parent path, not the source root.

From the doc:

path will be the value of path from the parent package.

@nicoddemus since you seem the main maintainer of this part.

Thanks!

[isra17]

On a local test:

src/
  p1/foo/p1/
    tests/
      test_p1.py
    __init__.py
    test_p1.py
  p2/foo/p2/
    __init__.py
    test_p2.py

Without the fix:

FAILED src/p1/foo/p1/test_p1.py::test_p1 - AssertionError
FAILED src/p1/foo/p1/tests/test_p1.py::test_p1 - AssertionError
FAILED src/p2/foo/p2/test_p2.py::test_p2 - AssertionError

With the fix:

FAILED src/p1/foo/p1/test_p1.py::test_p1 - assert False is True
 +  where False = p1()
FAILED src/p1/foo/p1/tests/test_p1.py::test_p1 - assert False is True
 +  where False = p1()
FAILED src/p2/foo/p2/test_p2.py::test_p2 - assert False is True
 +  where False = p2()

[jaraco]

Thanks @isra17 for the proof of concept. I do think at the very least we're going to want a regression test to capture expectation that was missed prior to your patch. Is that something that you'd be interested in developing?

[isra17]

Thanks @isra17 for the proof of concept. I do think at the very least we're going to want a regression test to capture expectation that was missed prior to your patch. Is that something that you'd be interested in developing?

Sure, I can work on this next week 👍

[tacaswell]

I can confirm that this un-breaks assertion re-writing for Matplotlib with an editable install.

[jnussbaum]

@jaraco @isra17 I very much like this PR, because it would unblock us. You said that a regression test is needed: Could you give me some more detailed instructions how such a test could/should look like, and where it should be located? Perhaps I would manage to draft such a test. Though, I have to admit that I have never contributed to pytest before.

[jaraco]

I'd model a test after some of the existing tests for importlib imports. Instead of checking that assert rewrites work, which would be much more involved, figure out what is the condition where import_path misbehaves (maybe inspired by the minimal repro in #12044 (comment)) and what that misbehavior looks like from the perspective of import_path. Then assert the correct behavior and it should fail. If you can do that on main and show the failure, then cherry-pick onto this branch and it succeeds, then you're golden.

[nicoddemus]

Thanks @isra17 for the PR, seems I have missed this notification.

As @jaraco mentions, it is important to have one or more regression tests for us to merge this in. Unfortunately the entire importlib mechanism feels somewhat brittle to me, given the scarce documentation and wild configuration variations in the wild where things can break or misbehave.