Try not to assume a module's file extension is .py

[Kodiologist]

Fixes #2369.

Any hints on how to write a test for this?

[coveralls]

Coverage decreased (-0.01%) to 92.68% when pulling 3c3c6fd on Kodiologist:getmodpath-file-ext into 581b463 on pytest-dev:master.

[nicoddemus]

Awesome, thanks a ton @Kodiologist! Please take a look at my comments and tell me what do you think.

[nicoddemus]

About a test for this:

I was considering writing a unittest that basically creates a Module instance and calls getmodpath.

But after thinking some more about it I think we need an integration test that demonstrates it is possible to collect a file without a .py extension as a Module object and the entire machinery works, otherwise we might end up introducing a regression later in some other part of the code and breaking this use case without realizing it.

I suggest to create a test that has a conftest.py file which implements the pytest_collect_file hook like this (untested):

def test_collects_different_extensions_as_python(testdir):
    """Ensure we can collect files with different extensions as .py files (#2369)""".
    testdir.makeconftest("""
        def pytest_collect_file(path, parent):
            ext = path.ext
            if ext == ".hy":
                return Module(path, parent)
    """)
    testdir.writefile("""
        # ... 1 test here in Hy ...
    """, ext=".hy")
    result = testdir.runpytest()
    result.stdout.fnmatch_lines('*1 passed*')

[Kodiologist]

All right, thanks, a test is in.

[nicoddemus]

I would write this to reflect the use case instead of the internal implementation detail that changed, perhaps:

Changed some internal details slightly so it is possible to collect files of any extension as Python modules.

[nicoddemus]

Clever solution. 😁

I notice though that you are using sys.meta_path.append, and because we run this test in the same process as the main process running pytest, we are leaving that finder there for the rest of pytest's test run.

The simplest solution I can find now is to just use testdir.runpytest_subprocess() to run that test in a separated process to ensure isolation. What do you think?

[Kodiologist]

Sounds good to me. Done.

[RonnyPfannschmidt]

for a different protection scheme you can use pytest_configure and pytest_unconfigure to push/pop the extra finder

[nicoddemus]

Good point. Do you think this should be changed, or leaving that as it is is fine?

I'm OK either way TBH.

[RonnyPfannschmidt]

both ways work, i just wanted to document here

[nicoddemus]

Fair enough! 👍

Would you like to merge this then?

[coveralls]

Coverage decreased (-0.01%) to 92.68% when pulling 836b3ee on Kodiologist:getmodpath-file-ext into 581b463 on pytest-dev:master.

[coveralls]

Coverage decreased (-0.01%) to 92.68% when pulling dcefb28 on Kodiologist:getmodpath-file-ext into 581b463 on pytest-dev:master.

[nicoddemus]

Thanks a lot @Kodiologist! 👍