flask-testing icon indicating copy to clipboard operation
flask-testing copied to clipboard

SQLAlchemy session not being removed between client requests

Open joshma opened this issue 12 years ago • 10 comments

Because of the pushed app context before each test case, multiple client requests within a test will see each other's (un-committed) sessions. This goes against the docs:

Another gotcha is that Flask-SQLAlchemy also removes the session instance at the end of every request (as should any threadsafe application using SQLAlchemy with scoped_session). Therefore the session is cleared along with any objects added to it every time you call client.get() or another client method.

Here's a test case that fails:

python from flask import Flask from flask.ext.sqlalchemy import SQLAlchemy from flask.ext.testing import TestCase

app = Flask(name) app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite:////tmp/test.db' db = SQLAlchemy(app) @app.route('/') def index(): u = db.session.query(User).first() u.name = 'bob' return ''

class User(db.Model): id = db.Column(db.Integer, primary_key=True) name = db.Column(db.String)

class SessionTest(TestCase):

def create_app(self): return app

def test_remove(self): db.drop_all() db.create_all()

u = User()
u.name = 'joe'
db.session.add(u)
db.session.commit()
client = app.test_client()
client.get('/')
assert u.name == 'joe' # fails!
assert u not in db.session # would fail if it reached here

joshma avatar Oct 21 '13 22:10 joshma

Bump - this seems like a pretty major issue, assuming I'm not doing anything wrong here. Is nobody else able to reproduce? Using

Flask==0.10.1
Flask-SQLAlchemy==0.16
Flask-Testing==0.4.1
SQLAlchemy==0.9.2
nose==1.1.2
$ nosetests broken_test.py
F
======================================================================
FAIL: test_remove (broken_test.SessionTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/joshma/aurelia/broken_test.py", line 33, in test_remove
    assert u.name == 'joe' # fails!
AssertionError

----------------------------------------------------------------------
Ran 1 test in 0.044s

FAILED (failures=1)

joshma avatar Mar 10 '14 20:03 joshma

this is working as intended as I understand. I use multiple client requests in a single test and they work find.

The documents for Flask-Testing does say that the session is removed at the end of the test. If you want to explicitly test the session stuff you would have to manually do what setUp and tearDown does between requests.

jamesonjlee avatar Mar 10 '14 21:03 jamesonjlee

Hm, I think the documentation says it's removed at the end of each request, not test:

Flask-SQLAlchemy also removes the session instance at the end of every request

And their example suggests this also. (Otherwise that last assertion should work.)

class SomeTest(MyTest):

    def test_something(self):

        user = User()
        db.session.add(user)
        db.session.commit()

        # this works
        assert user in db.session

        response = self.client.get("/")

        # this raises an AssertionError
        assert user in db.session

I also use multiple client requests per test, and they do not work fine for me. For example, if a request creates a SQLA model that has a foreign key, it gets attached to the session even if the request doesn't commit. If I then try to commit outside the request, because the session is shared I get IntegrityErrors when the uncommitted model tries to get committed with invalid fields.

joshma avatar Mar 10 '14 22:03 joshma

your IntegrityError seems like you need to call db.session.rollback on errors instead of waiting for session to close (or db.session.remove() manually).

but the documentation is incorrect (or is now incorrect). test_client does not create a new session, as I understand it's mainly for routing and parsing, afaik Flask-Testing doesn't dig into Werkzeug so I don't know how that can be changed.

the actual removal of session is at app/context teardown in flask-sqlalchemy: https://github.com/mitsuhiko/flask-sqlalchemy/blob/master/flask_sqlalchemy/init.py#L752

jamesonjlee avatar Mar 11 '14 02:03 jamesonjlee

My IntegrityError can be avoided, yes, but that would entail every view function doing db.session.rollback on any operation that doesn't commit. It's only a problem with the tests, because usually it's assumed there's one session per request so you never have to roll it back yourself.

This creates very unexpected behavior when you have more than one request per test. For example, in this gist I have a test where client.get('/') results in a different name being printed out the second time, even though the view function never commits.

It's a good point that this might be out of Flask-Testing's scope, though. I'll look a bit more into it.

joshma avatar Mar 11 '14 03:03 joshma

you can try @app.errorhandler or create a wrapped db.session.commitcall that will do the rollbacks. I am doing the latter.

I believe you can also try to do a post-app-view-but-before-teardown to trigger a commit() as well that a test_client might pickup.

jamesonjlee avatar Mar 11 '14 04:03 jamesonjlee

Flask-Testing messing up flask's normal way of separating requests from each other could easily lead to people's tests not testing what they think they are testing. This isn't just affecting the app context either, it's affecting flask.g as well.

eastein avatar May 28 '15 21:05 eastein

@eastein completely agree - I'm surprised more people aren't being bit by this. It's a very common assumption that a request can just be discarded without worry of affecting other requests

joshma avatar May 28 '15 23:05 joshma

It's not an assumption, it's in the Flask documentation. If Flask-Testing makes Flask disobey its own specification, how can we trust Flask-Testing?

eastein avatar May 29 '15 01:05 eastein

bumping this issue! doesn't clear the session!!

christopher-abastar avatar Jun 19 '15 10:06 christopher-abastar