Skip to content

Testing Flask apps breaks with 5.0.0 #5532

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
aberres opened this issue Jul 1, 2019 · 12 comments
Closed

Testing Flask apps breaks with 5.0.0 #5532

aberres opened this issue Jul 1, 2019 · 12 comments
Labels
topic: rewrite related to the assertion rewrite mechanism type: bug problem that needs to be addressed

Comments

@aberres
Copy link

aberres commented Jul 1, 2019

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
@RonnyPfannschmidt RonnyPfannschmidt added topic: rewrite related to the assertion rewrite mechanism type: bug problem that needs to be addressed labels Jul 1, 2019
@RemiCardona
Copy link

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
Copy link

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

@asottile
Copy link
Member

asottile commented Jul 1, 2019

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
Copy link
Member

asottile commented Jul 1, 2019

Handling this in flask: pallets/flask#3275

@nicoddemus
Copy link
Member

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
Copy link
Member

asottile commented Jul 1, 2019

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
Copy link
Member

I see, thanks!

@asottile
Copy link
Member

asottile commented Jul 1, 2019

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

@asottile asottile closed this as completed Jul 1, 2019
@nicoddemus
Copy link
Member

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

@asottile
Copy link
Member

asottile commented Jul 1, 2019

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
Copy link
Member

Ahh OK, thanks!

@asottile
Copy link
Member

asottile commented Jul 6, 2019

The latest version of flask (released yesterday) contains the fix (1.0.4 / 1.1.0)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: rewrite related to the assertion rewrite mechanism type: bug problem that needs to be addressed
Projects
None yet
Development

No branches or pull requests

5 participants