Testing Flask apps breaks with 5.0.0

[aberres]

I just upgraded to 5.0.0 and even the most simplistic Flask app cannot be tested anymore.

It seems as if this might be related to an issue from 2013: #317

Minimalistic test case

from flask import Flask

def test_example():
    app = Flask(__name__)
    assert True

Backtrace

planningsvc/tests/flaskjsonapi/test_a.py:5 (test_example)
def test_example():
>       app = Flask(__name__)

test_a.py:7: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/Users/berres/venv/mpptool/lib/python3.7/site-packages/flask/app.py:381: in __init__
    instance_path = self.auto_find_instance_path()
/Users/berres/venv/mpptool/lib/python3.7/site-packages/flask/app.py:678: in auto_find_instance_path
    prefix, package_path = find_package(self.import_name)
/Users/berres/venv/mpptool/lib/python3.7/site-packages/flask/helpers.py:826: in find_package
    loader, root_mod_name):
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

loader = <_pytest.assertion.rewrite.AssertionRewritingHook object at 0x10e8c96a0>
mod_name = 'test_a'

    def _matching_loader_thinks_module_is_package(loader, mod_name):
        """Given the loader that loaded a module and the module this function
        attempts to figure out if the given module is actually a package.
        """
        # If the loader can tell us if something is a package, we can
        # directly ask the loader.
        if hasattr(loader, 'is_package'):
            return loader.is_package(mod_name)
        # importlib's namespace loaders do not have this functionality but
        # all the modules it loads are packages, so we can take advantage of
        # this information.
        elif (loader.__class__.__module__ == '_frozen_importlib' and
              loader.__class__.__name__ == 'NamespaceLoader'):
            return True
        # Otherwise we need to fail with an error that explains what went
        # wrong.
        raise AttributeError(
            ('%s.is_package() method is missing but is required by Flask of '
             'PEP 302 import hooks.  If you do not use import hooks and '
             'you encounter this error please file a bug against Flask.') %
>           loader.__class__.__name__)
E       AttributeError: AssertionRewritingHook.is_package() method is missing but is required by Flask of PEP 302 import hooks.  If you do not use import hooks and you encounter this error please file a bug against Flask.

/Users/berres/venv/mpptool/lib/python3.7/site-packages/flask/helpers.py:789: AttributeError

[RemiCardona]

So I tried to tackle this, but my import/module/package/loader knowledge is very limited. The following did allow my Flask tests to pass.

class AssertionRewritingHook:
    """PEP302/PEP451 import hook which rewrites asserts."""

    def is_package(self, fullname):
        raise ImportError

and

class AssertionRewritingHook:
    """PEP302/PEP451 import hook which rewrites asserts."""

    def is_package(self, fullname):
        if self._marked_for_rewrite_cache.get(fullname):
            return True
        raise ImportError

I'm not too sure what the proper thing to do here is. The first one works and is_package() is called by flask on my own code modules so really Flask doesn't really care all that much (it's used to find a default directory for an "instance dir", probably some sort of default location for static data, and it looks like Flask can cope with this directory being missing, but it does need the proper APIs to exist).

The second one is probably more in line with what the API requires, but it's a shot in the dark for me. I really need a proper pytest adult here :)

Cheers

[RemiCardona]

Oh and it looks like it's a regression from #5468

[asottile]

flask is sadly incorrect here, is_package is optional since PEP 451

I believe the replacement is to check submodule_search_locations being truthy on the spec object

let's see if I can't fix it from the flask side

[asottile]

Handling this in flask: pallets/flask#3275

[nicoddemus]

Thanks @asottile!

Should we implement is_package anyway? It would allow users to choose not to update their Flask version to use pytest 5.0. This might also happen with other libraries.

If it is a two liner, I think it is worth it to avoid the small pain this can cause, even though it is not pytest's fault.

[asottile]

It is very difficult to implement correctly I'm afraid (since it gets queried before the module spec inference has run) -- this was the gnarly recursive somewhat-broken code I had before I switched to the PEP 451 loader approach

[nicoddemus]

I see, thanks!

[asottile]

closing since we fixed this up(down? I can never remember which direction is right)stream -- thanks again for the report @aberres and @RemiCardona 🎉 !

[nicoddemus]

If this affects all flask users, perhaps a tweet commenting on this issue would be good?

[asottile]

it shouldn't affect all (or even many I would think?) flask users, only those which define flask apps inside of a test module (for instance, my tested flask things all appear to be fine 🤷‍♂)

[nicoddemus]

Ahh OK, thanks!