Skip to content

Commit

Permalink
Correct usage of Flask's app context during testing - fix #90
Browse files Browse the repository at this point in the history
Don't create an application context during testing.  See #90 for
more info on the reason why this was problematic.  Adjust test
fixtures to compensate.

BREAKING CHANGE
---------------

No longer initializing an app context means that flask.current_app
as well as keg.db.db operations may fail where they did not
previously.  You will need to adjust your test fixtures to either
get the app instance directly or create the app context as needed.

The changes to our tests in this commit will provide good examples
of different kinds of fixture usage that can be used to work around
this change.
  • Loading branch information
rsyring committed Oct 23, 2017
1 parent 1c9e741 commit bc27780
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 68 deletions.
21 changes: 11 additions & 10 deletions keg/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,26 +214,27 @@ def environ_key(cls, key):
@classmethod
def testing_prep(cls, **config):
"""
Make sure an instance of this class exists in a state that is ready for testing to
commence.
Trigger `signal.testing_run_start` the first time this method is called for an app.
1. Instantiate the app class.
2. Cache the app instance after creation so that it's only instantiated once per Python
process.
3. Trigger `signal.testing_run_start` the first time this method is called for an app
class.
"""
# For now, do the import here so we don't have a hard dependency on WebTest
from keg.testing import ContextManager
if cls is Keg:
raise TypeError('Don\'t use testing_prep() on Keg. Create a subclass first.')
cm = ContextManager.get_for(cls)

# if the context manager's app isn't ready, that means this will be the first time the app
# If the context manager's app isn't ready, that means this will be the first time the app
# is instantiated. That seems like a good indicator that tests are just beginning, so it's
# safe to trigger the signal. We don't want the signal to fire every time b/c
# testing_prep() can be called more than once per test run.
trigger_signal = not cm.is_ready()
cm.ensure_current(config)

if trigger_signal:
signals.testing_run_start.send(cm.app)
if not cm.is_ready():
app = cm.make_ready(config)
# Setup an app context so that DB operations will work without error.
with app.app_context():
signals.testing_run_start.send(app)

return cm.app

Expand Down
15 changes: 2 additions & 13 deletions keg/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from flask_webtest import TestApp
import six

from keg import current_app
from keg.utils import app_environ_get


Expand All @@ -26,23 +25,13 @@ class ContextManager(object):
def __init__(self, appcls):
self.appcls = appcls
self.app = None
self.ctx = None

def ensure_current(self, config):

def make_ready(self, config):
if not self.app:
self.app = self.appcls().init(use_test_profile=True, config=config)
self.ctx = self.app.app_context()
self.ctx.push()

if current_app._get_current_object is not self.app:
self.ctx.push()

return self.app

def cleanup(self):
self.ctx.pop()

def is_ready(self):
return self.app is not None

Expand Down Expand Up @@ -112,4 +101,4 @@ class WebBase(object):
@classmethod
def setup_class(cls):
cls.app = cls.appcls.testing_prep()
cls.testapp = TestApp(flask.current_app)
cls.testapp = TestApp(cls.app)
20 changes: 11 additions & 9 deletions keg/tests/test_assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,23 @@
import pytest
import six

import keg
from keg.assets import AssetManager, AssetException
from keg_apps.templating.app import TemplatingApp


def setup_module(module):
TemplatingApp.testing_prep()
@pytest.fixture(scope='module')
def app():
return TemplatingApp.testing_prep()


@pytest.fixture
def am(app):
return AssetManager(app)


class TestAssets(object):

def test_load_related(self):
am = AssetManager(keg.current_app)
def test_load_related(self, am):
am.load_related('assets_in_template.html')

assert len(am.content['js']) == 1
Expand All @@ -36,14 +40,12 @@ def test_load_related(self):
)
assert content.strip() == '/* assets_in_template css file */'

def test_load_related_none_found(self):
def test_load_related_none_found(self, am):
with pytest.raises(AssetException) as e:
am = AssetManager(keg.current_app)
am.load_related('_not_there.html')
assert six.text_type(e) == 'Could not find related assets for template: _not_there.html'

def test_load_single_asset(self):
am = AssetManager(keg.current_app)
def test_load_single_asset(self, am):
am.load_related('assets_include_single.html')

assert len(am.content['js']) == 1
Expand Down
8 changes: 4 additions & 4 deletions keg/tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ def test_default_exception_handling(self):

class TestCLI2(CLIBase):

@classmethod
def setup_class(cls):
CLIBase.setup_class()
@pytest.fixture(scope='class', autouse=True)
def setup_app(self):
# Use this, instead of setting app_cls on the testing class, in order to test that the
# machinery in place for using the current app is working.
CLI2App.testing_prep()
with CLI2App.testing_prep().app_context():
yield

def test_invoke(self):
result = self.invoke('hello1')
Expand Down
33 changes: 19 additions & 14 deletions keg/tests/test_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,17 @@
from keg_apps.db2 import DB2App


@pytest.fixture(autouse=True)
def db_session_prep():
"""
Rollback the session after every test.
"""
db.session.rollback()


class TestDB(object):

@classmethod
def setup_class(cls):
DBApp.testing_prep()
@pytest.fixture(scope="class", autouse=True)
def setup_app(self):
app = DBApp.testing_prep()
# Setup an app context that clears itself when this class's tests are finished.
with app.app_context():
yield

def teardown(self):
db.session.rollback()

def test_primary_db_entity(self):
assert ents.Blog.query.count() == 0
Expand Down Expand Up @@ -57,9 +55,16 @@ def test_init_without_db_binds(self):


class TestDatabaseManager(object):
@classmethod
def setup_class(cls):
DBApp.testing_prep()

@pytest.fixture(scope="class", autouse=True)
def setup_app(self):
app = DBApp.testing_prep()
# Setup an app context that clears itself when this class's tests are finished.
with app.app_context():
yield

def teardown(self):
db.session.rollback()

def test_db_init_with_clear(self):
# have to use self here to enable the inner functions to adjust an outer-context variable
Expand Down
13 changes: 6 additions & 7 deletions keg/tests/test_db_dialects.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,12 @@
from keg_apps.db.app import DBApp


def setup_module(module):
DBApp.testing_prep()
@pytest.fixture(scope="module", autouse=True)
def app():
app = DBApp.testing_prep()
# Setup an app context that clears itself when this module's tests are finished.
with app.app_context():
yield


@pytest.fixture(autouse=True)
Expand All @@ -24,11 +28,6 @@ class DialectExam(object):
bind_name = None
obj_names_sql = None

@classmethod
def setup_class(cls):
# passing None as the app just because on_testing_start() doesn't use it
current_app.db_manager.on_testing_start(None)

@classmethod
def dialect(cls):
return current_app.db_manager.bind_dialect(cls.bind_name)
Expand Down
26 changes: 15 additions & 11 deletions keg/tests/test_templating.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,32 @@
import pytest
import six

from keg import current_app
from keg_apps.templating.app import TemplatingApp


def setup_module(module):
TemplatingApp.testing_prep()
@pytest.fixture(scope="module")
def app():
return TemplatingApp.testing_prep()


class TestAssetsInclude(object):
def render(self, filename):
template = current_app.jinja_env.get_template(filename)
return template.render()

def setup_method(self, method):
self.ctx = current_app.test_request_context()
@pytest.fixture(autouse=True)
def setup_teardown(self, app):
# setup
self.ctx = app.test_request_context()
self.ctx.push()
self.assets = self.ctx.assets

def teardown_method(self, method):
self.app = app
yield
# teardown
self.ctx.pop()

def test_include(self):
def render(self, filename):
template = self.app.jinja_env.get_template(filename)
return template.render()

def test_include(self, app):
resp = self.render('assets_in_template.html')
assert resp.strip() == ''

Expand Down

0 comments on commit bc27780

Please sign in to comment.