From fb845b90328278f4a557f898024a53800752fd53 Mon Sep 17 00:00:00 2001 From: David Lord Date: Fri, 14 Jul 2017 19:27:45 -0700 Subject: [PATCH] allow local packages in FLASK_APP don't require .py extension in FLASK_APP add tests for nested package loading parametrize cli loading tests --- CHANGES | 3 + flask/cli.py | 129 ++++++------- tests/test_apps/cliapp/factory.py | 8 +- tests/test_apps/cliapp/inner1/__init__.py | 3 + .../cliapp/inner1/inner2/__init__.py | 0 tests/test_apps/cliapp/inner1/inner2/flask.py | 3 + tests/test_apps/cliapp/message.txt | 1 + tests/test_cli.py | 170 ++++++++++-------- 8 files changed, 162 insertions(+), 155 deletions(-) create mode 100644 tests/test_apps/cliapp/inner1/__init__.py create mode 100644 tests/test_apps/cliapp/inner1/inner2/__init__.py create mode 100644 tests/test_apps/cliapp/inner1/inner2/flask.py create mode 100644 tests/test_apps/cliapp/message.txt diff --git a/CHANGES b/CHANGES index cc763166..dc11d5e4 100644 --- a/CHANGES +++ b/CHANGES @@ -52,6 +52,8 @@ Major release, unreleased - FLASK_APP=myproject.app:create_app('dev') support. - ``FLASK_APP`` can be set to an app factory, with arguments if needed, for example ``FLASK_APP=myproject.app:create_app('dev')``. (`#2326`_) +- ``FLASK_APP`` can point to local packages that are not installed in dev mode, + although `pip install -e` should still be preferred. (`#2414`_) - ``View.provide_automatic_options = True`` is set on the view function from ``View.as_view``, to be detected in ``app.add_url_rule``. (`#2316`_) - Error handling will try handlers registered for ``blueprint, code``, @@ -127,6 +129,7 @@ Major release, unreleased .. _#2373: https://github.com/pallets/flask/pull/2373 .. _#2385: https://github.com/pallets/flask/issues/2385 .. _#2412: https://github.com/pallets/flask/pull/2412 +.. _#2414: https://github.com/pallets/flask/pull/2414 Version 0.12.2 -------------- diff --git a/flask/cli.py b/flask/cli.py index ddf3c323..3568c10f 100644 --- a/flask/cli.py +++ b/flask/cli.py @@ -130,7 +130,7 @@ def find_app_by_string(string, script_info, module): if isinstance(app, Flask): return app else: - raise RuntimeError('Failed to find application in module ' + raise NoAppException('Failed to find application in module ' '"{name}"'.format(name=module)) except TypeError as e: new_error = NoAppException( @@ -147,85 +147,61 @@ def find_app_by_string(string, script_info, module): 'or function expression.'.format(string=string)) -def prepare_exec_for_file(filename): +def prepare_import(path): """Given a filename this will try to calculate the python path, add it to the search path and return the actual module name that is expected. """ - module = [] + path = os.path.realpath(path) - # Chop off file extensions or package markers - if os.path.split(filename)[1] == '__init__.py': - filename = os.path.dirname(filename) - elif filename.endswith('.py'): - filename = filename[:-3] - else: - raise NoAppException('The file provided (%s) does exist but is not a ' - 'valid Python file. This means that it cannot ' - 'be used as application. Please change the ' - 'extension to .py' % filename) - filename = os.path.realpath(filename) - - dirpath = filename - while 1: - dirpath, extra = os.path.split(dirpath) - module.append(extra) - if not os.path.isfile(os.path.join(dirpath, '__init__.py')): + if os.path.splitext(path)[1] == '.py': + path = os.path.splitext(path)[0] + + if os.path.basename(path) == '__init__': + path = os.path.dirname(path) + + module_name = [] + + # move up until outside package structure (no __init__.py) + while True: + path, name = os.path.split(path) + module_name.append(name) + + if not os.path.exists(os.path.join(path, '__init__.py')): break - if sys.path[0] != dirpath: - sys.path.insert(0, dirpath) + if sys.path[0] != path: + sys.path.insert(0, path) - return '.'.join(module[::-1]) + return '.'.join(module_name[::-1]) -def locate_app(script_info, app_id, raise_if_not_found=True): +def locate_app(script_info, module_name, app_name, raise_if_not_found=True): """Attempts to locate the application.""" __traceback_hide__ = True - if ':' in app_id: - module, app_obj = app_id.split(':', 1) - else: - module = app_id - app_obj = None - try: - __import__(module) + __import__(module_name) except ImportError: # Reraise the ImportError if it occurred within the imported module. # Determine this by checking whether the trace has a depth > 1. if sys.exc_info()[-1].tb_next: - stack_trace = traceback.format_exc() raise NoAppException( - 'There was an error trying to import the app ({module}):\n' - '{stack_trace}'.format( - module=module, stack_trace=stack_trace - ) + 'While importing "{name}", an ImportError was raised:' + '\n\n{tb}'.format(name=module_name, tb=traceback.format_exc()) ) elif raise_if_not_found: raise NoAppException( - 'The file/path provided ({module}) does not appear to exist.' - ' Please verify the path is correct. If app is not on' - ' PYTHONPATH, ensure the extension is .py.'.format( - module=module) + 'Could not import "{name}"."'.format(name=module_name) ) else: return - mod = sys.modules[module] + module = sys.modules[module_name] - if app_obj is None: - return find_best_app(script_info, mod) + if app_name is None: + return find_best_app(script_info, module) else: - return find_app_by_string(app_obj, script_info, mod) - - -def find_default_import_path(): - app = os.environ.get('FLASK_APP') - if app is None: - return - if os.path.isfile(app): - return prepare_exec_for_file(app) - return app + return find_app_by_string(app_name, script_info, module) def get_version(ctx, param, value): @@ -308,15 +284,8 @@ class ScriptInfo(object): """ def __init__(self, app_import_path=None, create_app=None): - if create_app is None: - if app_import_path is None: - app_import_path = find_default_import_path() - self.app_import_path = app_import_path - else: - app_import_path = None - #: Optionally the import path for the Flask application. - self.app_import_path = app_import_path + self.app_import_path = app_import_path or os.environ.get('FLASK_APP') #: Optionally a function that is passed the script info to create #: the instance of the application. self.create_app = create_app @@ -335,37 +304,39 @@ class ScriptInfo(object): if self._loaded_app is not None: return self._loaded_app + app = None + if self.create_app is not None: - rv = call_factory(self.create_app, self) + app = call_factory(self.create_app, self) else: if self.app_import_path: - rv = locate_app(self, self.app_import_path) + path, name = (self.app_import_path.split(':', 1) + [None])[:2] + import_name = prepare_import(path) + app = locate_app(self, import_name, name) else: - for module in ['wsgi.py', 'app.py']: - import_path = prepare_exec_for_file(module) - rv = locate_app( - self, import_path, raise_if_not_found=False + for path in ('wsgi.py', 'app.py'): + import_name = prepare_import(path) + app = locate_app( + self, import_name, None, raise_if_not_found=False ) - if rv: + if app: break - if not rv: - raise NoAppException( - 'Could not locate Flask application. You did not provide ' - 'the FLASK_APP environment variable, and a wsgi.py or ' - 'app.py module was not found in the current directory.\n\n' - 'For more information see ' - 'http://flask.pocoo.org/docs/latest/quickstart/' - ) + if not app: + raise NoAppException( + 'Could not locate a Flask application. You did not provide ' + 'the "FLASK_APP" environment variable, and a "wsgi.py" or ' + '"app.py" module was not found in the current directory.' + ) debug = get_debug_flag() if debug is not None: - rv._reconfigure_for_run_debug(debug) + app._reconfigure_for_run_debug(debug) - self._loaded_app = rv - return rv + self._loaded_app = app + return app pass_script_info = click.make_pass_decorator(ScriptInfo, ensure=True) diff --git a/tests/test_apps/cliapp/factory.py b/tests/test_apps/cliapp/factory.py index b0d4771e..2e8598e8 100644 --- a/tests/test_apps/cliapp/factory.py +++ b/tests/test_apps/cliapp/factory.py @@ -4,12 +4,12 @@ from flask import Flask def create_app(): - return Flask('create_app') + return Flask('app') def create_app2(foo, bar): - return Flask("_".join(['create_app2', foo, bar])) + return Flask('_'.join(['app2', foo, bar])) -def create_app3(foo, bar, script_info): - return Flask("_".join(['create_app3', foo, bar])) +def create_app3(foo, script_info): + return Flask('_'.join(['app3', foo, script_info.data['test']])) diff --git a/tests/test_apps/cliapp/inner1/__init__.py b/tests/test_apps/cliapp/inner1/__init__.py new file mode 100644 index 00000000..8330f6e0 --- /dev/null +++ b/tests/test_apps/cliapp/inner1/__init__.py @@ -0,0 +1,3 @@ +from flask import Flask + +application = Flask(__name__) diff --git a/tests/test_apps/cliapp/inner1/inner2/__init__.py b/tests/test_apps/cliapp/inner1/inner2/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/test_apps/cliapp/inner1/inner2/flask.py b/tests/test_apps/cliapp/inner1/inner2/flask.py new file mode 100644 index 00000000..d7562aac --- /dev/null +++ b/tests/test_apps/cliapp/inner1/inner2/flask.py @@ -0,0 +1,3 @@ +from flask import Flask + +app = Flask(__name__) diff --git a/tests/test_apps/cliapp/message.txt b/tests/test_apps/cliapp/message.txt new file mode 100644 index 00000000..fc2b2cf0 --- /dev/null +++ b/tests/test_apps/cliapp/message.txt @@ -0,0 +1 @@ +So long, and thanks for all the fish. diff --git a/tests/test_cli.py b/tests/test_cli.py index 0e0a56ad..5fba5229 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -11,7 +11,8 @@ # the Revised BSD License. # Copyright (C) 2015 CERN. # -from __future__ import absolute_import, print_function +from __future__ import absolute_import + import os import sys from functools import partial @@ -19,11 +20,10 @@ from functools import partial import click import pytest from click.testing import CliRunner -from flask import Flask, current_app -from flask.cli import cli, AppGroup, FlaskGroup, NoAppException, ScriptInfo, \ - find_best_app, locate_app, with_appcontext, prepare_exec_for_file, \ - find_default_import_path, get_version +from flask import Flask, current_app +from flask.cli import AppGroup, FlaskGroup, NoAppException, ScriptInfo, \ + find_best_app, get_version, locate_app, prepare_import, with_appcontext @pytest.fixture @@ -125,78 +125,104 @@ def test_find_best_app(test_apps): pytest.raises(NoAppException, find_best_app, script_info, Module) -def test_prepare_exec_for_file(test_apps): - """Expect the correct path to be set and the correct module name to be returned. - - :func:`prepare_exec_for_file` has a side effect, where - the parent directory of given file is added to `sys.path`. +cwd = os.getcwd() +test_path = os.path.abspath(os.path.join( + os.path.dirname(__file__), 'test_apps' +)) + + +@pytest.mark.parametrize('value,path,result', ( + ('test', cwd, 'test'), + ('test.py', cwd, 'test'), + ('a/test', os.path.join(cwd, 'a'), 'test'), + ('test/__init__.py', cwd, 'test'), + ('test/__init__', cwd, 'test'), + # nested package + ( + os.path.join(test_path, 'cliapp', 'inner1', '__init__'), + test_path, 'cliapp.inner1' + ), + ( + os.path.join(test_path, 'cliapp', 'inner1', 'inner2'), + test_path, 'cliapp.inner1.inner2' + ), + # dotted name + ('test.a.b', cwd, 'test.a.b'), + (os.path.join(test_path, 'cliapp.app'), test_path, 'cliapp.app'), + # not a Python file, will be caught during import + ( + os.path.join(test_path, 'cliapp', 'message.txt'), + test_path, 'cliapp.message.txt' + ), +)) +def test_prepare_import(request, value, path, result): + """Expect the correct path to be set and the correct import and app names + to be returned. + + :func:`prepare_exec_for_file` has a side effect where the parent directory + of the given import is added to :data:`sys.path`. This is reset after the + test runs. """ - realpath = os.path.realpath('/tmp/share/test.py') - dirname = os.path.dirname(realpath) - assert prepare_exec_for_file('/tmp/share/test.py') == 'test' - assert dirname in sys.path - - realpath = os.path.realpath('/tmp/share/__init__.py') - dirname = os.path.dirname(os.path.dirname(realpath)) - assert prepare_exec_for_file('/tmp/share/__init__.py') == 'share' - assert dirname in sys.path + original_path = sys.path[:] + + def reset_path(): + sys.path[:] = original_path + + request.addfinalizer(reset_path) + + assert prepare_import(value) == result + assert sys.path[0] == path + + +@pytest.mark.parametrize('iname,aname,result', ( + ('cliapp.app', None, 'testapp'), + ('cliapp.app', 'testapp', 'testapp'), + ('cliapp.factory', None, 'app'), + ('cliapp.factory', 'create_app', 'app'), + ('cliapp.factory', 'create_app()', 'app'), + # no script_info + ('cliapp.factory', 'create_app2("foo", "bar")', 'app2_foo_bar'), + # trailing comma space + ('cliapp.factory', 'create_app2("foo", "bar", )', 'app2_foo_bar'), + # takes script_info + ('cliapp.factory', 'create_app3("foo")', 'app3_foo_spam'), +)) +def test_locate_app(test_apps, iname, aname, result): + info = ScriptInfo() + info.data['test'] = 'spam' + assert locate_app(info, iname, aname).name == result + + +@pytest.mark.parametrize('iname,aname', ( + ('notanapp.py', None), + ('cliapp/app', None), + ('cliapp.app', 'notanapp'), + # not enough arguments + ('cliapp.factory', 'create_app2("foo")'), + # nested import error + ('cliapp.importerrorapp', None), + # not a Python file + ('cliapp.message.txt', None), + # space before arg list + ('cliapp.factory', 'create_app ()'), +)) +def test_locate_app_raises(test_apps, iname, aname): + info = ScriptInfo() with pytest.raises(NoAppException): - prepare_exec_for_file('/tmp/share/test.txt') + locate_app(info, iname, aname) -def test_locate_app(test_apps): - """Test of locate_app.""" - script_info = ScriptInfo() - assert locate_app(script_info, "cliapp.app").name == "testapp" - assert locate_app(script_info, "cliapp.app:testapp").name == "testapp" - assert locate_app(script_info, "cliapp.factory").name == "create_app" - assert locate_app( - script_info, "cliapp.factory").name == "create_app" - assert locate_app( - script_info, "cliapp.factory:create_app").name == "create_app" - assert locate_app( - script_info, "cliapp.factory:create_app()").name == "create_app" - assert locate_app( - script_info, "cliapp.factory:create_app2('foo', 'bar')" - ).name == "create_app2_foo_bar" - assert locate_app( - script_info, "cliapp.factory:create_app2('foo', 'bar', )" - ).name == "create_app2_foo_bar" - assert locate_app( - script_info, "cliapp.factory:create_app3('baz', 'qux')" - ).name == "create_app3_baz_qux" - assert locate_app(script_info, "cliapp.multiapp:app1").name == "app1" - pytest.raises( - NoAppException, locate_app, script_info, "notanpp.py") - pytest.raises( - NoAppException, locate_app, script_info, "cliapp/app") - pytest.raises( - RuntimeError, locate_app, script_info, "cliapp.app:notanapp") - pytest.raises( - NoAppException, locate_app, - script_info, "cliapp.factory:create_app2('foo')") - pytest.raises( - NoAppException, locate_app, - script_info, "cliapp.factory:create_app ()") - pytest.raises( - NoAppException, locate_app, script_info, "cliapp.importerrorapp") - assert locate_app( - script_info, "notanpp.py", raise_if_not_found=False - ) is None - - -def test_find_default_import_path(test_apps, monkeypatch, tmpdir): - """Test of find_default_import_path.""" - monkeypatch.delitem(os.environ, 'FLASK_APP', raising=False) - assert find_default_import_path() == None - monkeypatch.setitem(os.environ, 'FLASK_APP', 'notanapp') - assert find_default_import_path() == 'notanapp' - tmpfile = tmpdir.join('testapp.py') - tmpfile.write('') - monkeypatch.setitem(os.environ, 'FLASK_APP', str(tmpfile)) - expect_rv = prepare_exec_for_file(str(tmpfile)) - assert find_default_import_path() == expect_rv +def test_locate_app_suppress_raise(): + info = ScriptInfo() + app = locate_app(info, 'notanapp.py', None, raise_if_not_found=False) + assert app is None + + # only direct import error is suppressed + with pytest.raises(NoAppException): + locate_app( + info, 'cliapp.importerrorapp', None, raise_if_not_found=False + ) def test_get_version(test_apps, capsys):