Test cases reuse sqlalchemy models during checks between request and this is unsafe

[heartsucker]

Bug

Description

During tests if we reuse a database object after a view function that calls db.session.commit, we may end up running assertions against a stale object.

Example

def test_foo(journalist_app, test_journo):
    with journalist_app.test_client() as app:
        # this is the sqlalchemy db object as defined in
        # the fixture `test_journo` in `conftest.py`
        db_object = test_journo['journalist']

        # grab some field we want to check
        some_attr = db_object.some_attr
        app.post('/endpoint-that-may-alter-some-attr',
                 data=dict(form_field='illegal value'))

        # the db object is stale here
        new_some_attr = db_object.some_attr
        # this check will always pass even if the endpoint changes the db object
        assert some_attr == new_some_attr

Expected Behavior

Our tests accurately check when there were or were not alterations in the database.

Actual Behavior

They do not.

Comments

We should requery the DB for the object between requests. Calling db.session.refresh(foo) breaks and yells because the session attached to the initial object has popped.

[heartsucker]

Note, this definitely is fixed with:

# manually refresh the object between requests
db_object = Journalist.query.get(db_object.id)

and may be fixed by changing the fixtures to this:

# conftest.py
@pytest.fixture(scope='function')
def journalist_app(config):
    app = create_journalist_app(config)
    with app.app_context():
        db.create_all()
        # note that we yield the app instead of return,
        # and we do it *inside* the pushed app context
        yield app

Though this may end up with cases where we've double pushed a context, and I have no idea what would happen there.