[RFC] --import-mode: allow for "importlib" [ci skip]

[blueyed]

Ref: pytest-dev/py#203

[codecov]

Codecov Report

Merging #5352 into features will decrease coverage by 8.71%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##           features    #5352      +/-   ##
============================================
- Coverage     93.96%   85.24%   -8.72%     
============================================
  Files           115      115              
  Lines         26384    26384              
  Branches       2607     2607              
============================================
- Hits          24792    22492    -2300     
- Misses         1269     3514    +2245     
- Partials        323      378      +55

Impacted Files	Coverage Δ
src/_pytest/python.py	90.18% <ø> (-1.94%)	⬇️
testing/test_reports.py	10.49% <0%> (-89.51%)	⬇️
testing/test_session.py	17.72% <0%> (-82.28%)	⬇️
testing/test_runner_xunit.py	29.82% <0%> (-70.18%)	⬇️
testing/test_unittest.py	33.48% <0%> (-65.62%)	⬇️
testing/test_runner.py	34.7% <0%> (-61.61%)	⬇️
testing/test_pdb.py	41.24% <0%> (-57.2%)	⬇️
testing/test_argcomplete.py	20.28% <0%> (-47.83%)	⬇️
testing/test_resultlog.py	53.6% <0%> (-46.4%)	⬇️
src/_pytest/_argcomplete.py	35.13% <0%> (-40.55%)	⬇️
... and 40 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 693c3b7...4cec123. Read the comment docs.

[nicoddemus]

Thanks for picking this up again!

We should merge this after 4.6 is released, I suspect you already plan to do that though. 😁

Things missing I think:

• Require py>=1.8.0
• Integration test(s)
• Docs

[blueyed]

I do not think we should bump the requirement because of it.
I've more thought about primarily enabling it - I am not using it myself after all, since it does not allow for relative imports.

[asottile]

I think we should find out a way to enable this and make it the default -- from playing with this it seems to make the import structure of tests way more sane -- though we should figure out that relative import issue 🤔

[nicoddemus]

I do not think we should bump the requirement because of it.

You mean py? Unless I'm missing something, doesn't that depend on the importlib support added in py 1.8?

I think we should find out a way to enable this and make it the default -- from playing with this it seems to make the import structure of tests way more sane -- though we should figure out that relative import issue

Doesn't that change mean that pytest doesn't have to change sys.path to import test modules and conftest.py files? If that's right, we are making a big change here, because I'm sure there are a ton of projects out there which (wrongly) rely on this behavior to run their test suites in non-src setups.

[asottile]

yeah it would have to be a big shift, I'm still not even sure the full fallout of such a change but from some basic playing with it it seems to make the "duplicated test names problem go away"

[nicoddemus]

I agree, but at the same time I fear a lot of test suites would simply stop working, and worse, without any guidance from the issuing error message... not sure if we should change that to be the default, TBH. Definitely an option, but just making that the default seems risky... 🤔

[blueyed]

You mean py? Unless I'm missing something, doesn't that depend on the importlib support added in py 1.8?

Yes, but it is not required if not used.
So for completeness it should probably add the choice only for a recent enough py maybe, but could also have a comment/note in the help instead (or a runtime check when being used).

[blueyed]

I also do not think it should be made a default (as of now) - like said before, not being able to use relative imports is a big bummer for me. I've only looked at it briefly, but maybe there's an option to support this somehow.

[nicoddemus]

Yes, but it is not required if not used.
So for completeness it should probably add the choice only for a recent enough py maybe, but could also have a comment/note in the help instead (or a runtime check when being used).

Not sure, what's the problem with bumping the requirement to py>=1.8? Seems simpler and straightforward, or are there any drawbacks I'm not seeing here?

I also do not think it should be made a default (as of now)

Agreed. This gives me the idea that we should update the error message about duplicated modules to mention this option as an alternative, as well as to point to the appropriate docs.

[graingert]

I think this help string should be updated

[blueyed]

Yeah.
But initially this was just meant to allow for using this easily already (i.e. experimental).

[nicoddemus]

I also do not think it should be made a default (as of now) - like said before, not being able to use relative imports is a big bummer for me.

I suppose we can include the option, then we can try it using it more and ask people to do the same. If there are no major show-stoppers, we can change the default in pytest 6.0, while issuing a warning about the upcoming change in defaults.

[blueyed]

Using as a default is not an option really (according to when I've tested it).
But I agree that it makes testing and getting feedback easier if you can use it without patching pytest.. (the point of this PR). Feel free to re-open / take over.