testing that a session is committed correctly

[Chris Withers]

Hi All,

I'm working with the pattern described at https://docs.sqlalchemy.org/en/13/orm/session_transaction.html#joining-a-session-into-an-external-transaction-such-as-for-test-suites along with pytest and FastAPI, an async web app framework with good support for running blocking code.

So, I have my fixtures:

@pytest.fixture(scope='session')
def db(client):
    engine = Session.kw['bind']
    conn = engine.connect()
    transaction = conn.begin()
    try:
        Base.metadata.create_all(bind=conn, checkfirst=False)
        yield conn
    finally:
        transaction.rollback()
        Session.configure(bind=engine)

@pytest.fixture()
def transaction(db):
    transaction = db.begin_nested()
    try:
        Session.configure(bind=db)
        yield transaction
    finally:
        transaction.rollback()

@pytest.fixture()
def session(transaction):
    return Session()

And a test:

def test_create_full_data(transaction, session, client):
    response = client.post('/events/', json={
        'date': '2019-06-02',
        'type': 'DONE',
        'text': 'some stuff got done'
    })
    transaction.rollback()
    actual = session.query(Event).one()
    compare(actual.date, expected=date(2019, 6, 2))
    compare(type(actual), expected=Done)
    compare(response.json(), expected={
        'id': actual.id,
        'date': '2019-06-02',
        'type': 'DONE',
        'text': 'some stuff got done'
    })
    compare(response.status_code, expected=201)

And some code:

@router.post("/", response_model=EventRead, status_code=201)
def create_object(
    *,
    session: Session = Depends(db_session),
    event: EventCreate,
):
    """
    Create new Event.
    """
    with session.transaction:
        event = Event(**event.dict())
        session.add(event)
        session.flush()
        session.expunge(event)
    return event

Now, what I'm trying to test is that I haven't forgotten to include the "with session.transaction". The problem is that, without the transaction.rollback(), the test passes regardless of whether the "with session.transaction" is there, and with it there, it always fails.

What's the best way for writing tests that have the database setup DDL run in a transaction that's rolled back at the end of the session, each test in a subtransaction that gets rolled back at the end of each test, and also test that code under test is committing or rolling back as required?

cheers,

Chris

[Mike Bayer]

So when you make the Session it has a .transaction that you start with, when you come out of your tests, that should be the same .transaction when you get the Session back.  If it's different, then the test did not leave the Session in the same state.  does that work ?

cheers,

Chris

--

SQLAlchemy -

The Python SQL Toolkit and Object Relational Mapper

 

http://www.sqlalchemy.org/

 

To post example code, please provide an MCVE: Minimal, Complete, and Verifiable Example. See http://stackoverflow.com/help/mcve for a full description.

---

You received this message because you are subscribed to the Google Groups "sqlalchemy" group.

To unsubscribe from this group and stop receiving emails from it, send an email to sqlalchemy+...@googlegroups.com.

To post to this group, send email to sqlal...@googlegroups.com.

Visit this group at https://groups.google.com/group/sqlalchemy.

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/430d4156-d9ff-4349-5be5-62bee6ea4627%40withers.org.

For more options, visit https://groups.google.com/d/optout.

[Chris Withers]

On 04/06/2019 14:49, Mike Bayer wrote:
>
>
> On Tue, Jun 4, 2019, at 2:15 AM, Chris Withers wrote:
>> Now, what I'm trying to test is that I haven't forgotten to include
>> the "with session.transaction". The problem is that, without the
>> transaction.rollback(), the test passes regardless of whether the
>> "with session.transaction" is there, and with it there, it always fails.
>>
>> What's the best way for writing tests that have the database setup DDL
>> run in a transaction that's rolled back at the end of the session,
>> each test in a subtransaction that gets rolled back at the end of each
>> test, and also test that code under test is committing or rolling back
>> as required?
>
>
> So when you make the Session it has a .transaction that you start with,
> when you come out of your tests, that should be the same .transaction
> when you get the Session back.  If it's different, then the test did not
> leave the Session in the same state.  does that work ?

I think what I'm getting to here is that there are two Session instances
here, one that's set up by the web app under test, and one by pytest as
a fixture.

However, if I understand correctly, they're both running inside the
sub-transaction returned by engine.connect().begin_nested(), which is in
turn inside the transaction returned by engine.connect().begin().

So, how do I roll back the further subtransaction created by the web
framework instantiating Session from a sessionmaker bound to the
connection in which begin_nested() has been called, which under non-test
running would actually be a top level transaction assuming I understand
the pattern correctly, in such as way that if the code-under-test has
committed on is session, the session being used to check expectations in
the unit test will see the results, but if it that commit has been
forgotten, it will not?

cheers,

Chris

[Mike Bayer]

I'm not following all your code but if there are two sessions in play I'd probably try to avoid that, there should be only one Session you care about.  the test fixtures should be external to everything and make sure there's just the one session.   if there are two in play, I'm not sure how they both get bound to your test transaction.

cheers,

Chris

-- 

SQLAlchemy - 

The Python SQL Toolkit and Object Relational Mapper

http://www.sqlalchemy.org/

To post example code, please provide an MCVE: Minimal, Complete, and Verifiable Example.  See  http://stackoverflow.com/help/mcve for a full description.

--- 

You received this message because you are subscribed to the Google Groups "sqlalchemy" group.

To unsubscribe from this group and stop receiving emails from it, send an email to sqlalchemy+...@googlegroups.com.

To post to this group, send email to sqlal...@googlegroups.com.

Visit this group at https://groups.google.com/group/sqlalchemy.

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/471aa85f-776f-b899-1fe8-2f3cc009da38%40withers.org.

[Chris Withers]

On 04/06/2019 23:21, Mike Bayer wrote:
>
>
> I'm not following all your code but if there are two sessions in play
> I'd probably try to avoid that, there should be only one Session you
> care about.

This comes back to something I asked you about on Twitter a while ago:
the code under test gets its session by calling a sessionmaker; how can
I have that return an existing session, which appears to be what you're
suggesting, rather than a new suggestion, which appears to be all they
can do.

> the test fixtures should be external to everything

I don't understand what you mean by this.

> and make
> sure there's just the one session.   if there are two in play, I'm not
> sure how they both get bound to your test transaction.

I believe they start a nested transaction?

Chris

[Chris Withers]

On 04/06/2019 23:21, Mike Bayer wrote:

On Tue, Jun 4, 2019, at 4:33 PM, Chris Withers wrote:

So, how do I roll back the further subtransaction created by the web 

framework instantiating Session from a sessionmaker bound to the 

connection in which begin_nested() has been called, which under non-test 

running would actually be a top level transaction assuming I understand 

the pattern correctly, in such as way that if the code-under-test has 

committed on is session, the session being used to check expectations in 

the unit test will see the results, but if it that commit has been 

forgotten, it will not?

I'm not following all your code but if there are two sessions in play I'd probably try to avoid that, there should be only one Session you care about.  the test fixtures should be external to everything and make sure there's just the one session.   if there are two in play, I'm not sure how they both get bound to your test transaction.

Even this doesn't appear to be enough, here's the simplest reproducer script I can get to:

import os

from diary.model import Session, Base, Event, Types
from sqlalchemy import create_engine

engine = create_engine(os.environ['TEST_DB_URL'])
Session.configure(bind=engine)

conn = engine.connect()
transaction = conn.begin()
try:
    Base.metadata.create_all(bind=conn, checkfirst=False)
    Session.configure(bind=conn)

    sub_transaction = conn.begin_nested()
    try:

        session = Session()

        # code under test:
        event = Event(date='2019-06-02', type=Types.done, text='some stuff got done')
        session.add(event)
        session.flush()
        session.close()

        # test:
        session.rollback()
        assert session.query(Event).count() == 0

    finally:
        sub_transaction.rollback()

finally:
    transaction.rollback()

That session.close() appears to be the problem. It's a normal and required part of the code under test, but it throws away the SessionTransaction without rolling it back, so by the time the test does session.rollback(), it's doing it on a new SessionTransaction and so has no effect and the assertion fails because there event created is still around...

Is this a bug or am I just confusing myself and everyone else?

Chris

[Mike Bayer]

On Tue, Jun 4, 2019, at 6:31 PM, Chris Withers wrote:

On 04/06/2019 23:21, Mike Bayer wrote:

> 

> 

> I'm not following all your code but if there are two sessions in play 

> I'd probably try to avoid that, there should be only one Session you 

> care about. 

This comes back to something I asked you about on Twitter a while ago: 

the code under test gets its session by calling a sessionmaker; how can 

I have that return an existing session, which appears to be what you're 

suggesting, rather than a new suggestion, which appears to be all they 

can do.

yeah you have to inject and mock aggressively so that your code being tested only sees the Session objects your fixtures want them to.

> the test fixtures should be external to everything 

I don't understand what you mean by this.

the same thing as my statement above.   The code being tested shouldn't be able to make it's own Session if you want it to have one that you are controlling, which means when your test harness runs it has to swap out everything in the application that might be in the way of this.  The term "harness" implies your entire application sits "inside" the harness, that's what I mean by "external".

> and make 

> sure there's just the one session.   if there are two in play, I'm not 

> sure how they both get bound to your test transaction.

I believe they start a nested transaction?

one Session has nothing to do with another so I don't know what you mean.

Chris

-- 

SQLAlchemy - 

The Python SQL Toolkit and Object Relational Mapper

http://www.sqlalchemy.org/

To post example code, please provide an MCVE: Minimal, Complete, and Verifiable Example.  See  http://stackoverflow.com/help/mcve for a full description.

--- 

You received this message because you are subscribed to the Google Groups "sqlalchemy" group.

To unsubscribe from this group and stop receiving emails from it, send an email to sqlalchemy+...@googlegroups.com.

To post to this group, send email to sqlal...@googlegroups.com.

Visit this group at https://groups.google.com/group/sqlalchemy.

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/50bb9529-f98b-24aa-484f-1274c8fe3290%40withers.org.

[Mike Bayer]

I can run your script if you remove the "diary" thing from it

Chris

--

SQLAlchemy -

The Python SQL Toolkit and Object Relational Mapper

 

http://www.sqlalchemy.org/

 

To post example code, please provide an MCVE: Minimal, Complete, and Verifiable Example. See http://stackoverflow.com/help/mcve for a full description.

---

You received this message because you are subscribed to the Google Groups "sqlalchemy" group.

To unsubscribe from this group and stop receiving emails from it, send an email to sqlalchemy+...@googlegroups.com.

To post to this group, send email to sqlal...@googlegroups.com.

Visit this group at https://groups.google.com/group/sqlalchemy.

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/55721019-2de3-12b4-3796-90ca34b5ea4c%40withers.org.

[Chris Withers]

(sorry, meant to send this to the list)

On 05/06/2019 15:52, Mike Bayer wrote:

That session.close() appears to be the problem. It's a normal and required part of the code under test, but it throws away the SessionTransaction without rolling it back, so by the time the test does session.rollback(), it's doing it on a new SessionTransaction and so has no effect and the assertion fails because there event created is still around...

Is this a bug or am I just confusing myself and everyone else?

I can run your script if you remove the "diary" thing from it

Hmm, I wonder what's different?

I'm on Python 3.7.1, SQLAlchemy 1.3.4, Postgres 11.3, here's a totally self contained script:

import os

from sqlalchemy import Column, Integer, Text
from sqlalchemy import create_engine
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import sessionmaker

Base = declarative_base()

Session = sessionmaker()

class Event(Base):
    __tablename__ = 'entry'
    id = Column(Integer(), primary_key=True)
    text = Column(Text)


engine = create_engine(os.environ['TEST_DB_URL'])
Session.configure(bind=engine)

conn = engine.connect()
transaction = conn.begin()

try:
    Base.metadata.create_all(bind=conn, checkfirst=False)
    Session.configure(bind=conn)

    sub_transaction = conn.begin_nested()
    try:

        session = Session()

        # code under test:

        event = Event(text='some stuff got done')
        session.add(event)
        session.flush()
        session.close()

        

# test:
        session.rollback()
        assert session.query(Event).count() == 0

    finally:
        sub_transaction.rollback()

finally:
    transaction.rollback()

Which gives me:

$ python sessions_are_weird.py
Traceback (most recent call last):
  File "sessions_are_weird.py", line 40, in <module>
    assert session.query(Event).count() == 0
AssertionError

Whereas after the rollback, I'd expect that count to be zero...

Chris

[Chris Withers]

On 05/06/2019 16:41, Mike Bayer wrote:
>
>> Which gives me:
>>
>> $ python sessions_are_weird.py
>> Traceback (most recent call last):
>>   File "sessions_are_weird.py", line 40, in <module>
>>     assert session.query(Event).count() == 0
>> AssertionError
>>
>> Whereas after the rollback, I'd expect that count to be zero...
>

> this is not working because of this:
>
>         session.close()
>
>         # test:
>         session.rollback()
>
>
> the session is closed first so rollback() will do nothing.

Yeah, that was my point: That session.close() appears to be the problem.

It's a normal and required part of the code under test, but it throws
away the SessionTransaction without rolling it back, so by the time the
test does session.rollback(), it's doing it on a new SessionTransaction
and so has no effect and the assertion fails because there event created
is still around...

Put differently, the web app closes the session at the end of its
request handling, which seems legit, right?

How come close() doesn't rollback the SessionTransaction if it throws it
away?

Chris

[Mike Bayer]

that's currently what .close() does, it discards the connection.   this is safe because the connection pool ensures transactions are rolled back.   This might have to change with some of the 2.0 things I'm thinking about but it's not clear yet.

Anyway, you don't get a rollback here because the session is bound to an external connection so the connection pool is not involved.     if you want to roll back the work that the application did, that would be your sub_transaction.rollback():

    sub_transaction = conn.begin_nested()

    try:

        session = Session()

        # code under test:

        event = Event(text='some stuff got done')

        session.add(event)

        session.flush()

        session.close()

    finally:

        sub_transaction.rollback()

        assert session.query(Event).count() == 0

the test harness is giving you two choices.  you can look at the state of the DB after your program has done some things and *before* your harness has reversed its work, or you can look at the state of the DB *after* your harness has reversed its work.   I'm not sure what the combination of "session.rollback()" + "sub_transaction.rollback()" is trying to achieve or what would be happening between the two.

Chris

-- 

SQLAlchemy - 

The Python SQL Toolkit and Object Relational Mapper

http://www.sqlalchemy.org/

To post example code, please provide an MCVE: Minimal, Complete, and Verifiable Example.  See  http://stackoverflow.com/help/mcve for a full description.

--- 

You received this message because you are subscribed to the Google Groups "sqlalchemy" group.

To unsubscribe from this group and stop receiving emails from it, send an email to sqlalchemy+...@googlegroups.com.

To post to this group, send email to sqlal...@googlegroups.com.

Visit this group at https://groups.google.com/group/sqlalchemy.

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/806b6413-8537-8813-5c32-1238086300dd%40withers.org.

[Chris Withers]

On 05/06/2019 17:15, Mike Bayer wrote:
>
>> How come close() doesn't rollback the SessionTransaction if it throws it
>> away?
>
> that's currently what .close() does, it discards the connection.   this
> is safe because the connection pool ensures transactions are rolled
> back.   This might have to change with some of the 2.0 things I'm
> thinking about but it's not clear yet.
>
> Anyway, you don't get a rollback here because the session is bound to an
> external connection so the connection pool is not involved.

Ah, right, so under normal conditions, if I just close a session, any
transaction and subtransactions in flight will be rolled back when the
connection is returned to the pool?

> if you
> want to roll back the work that the application did, that would be your
> sub_transaction.rollback():
>
>     sub_transaction = conn.begin_nested()
>     try:
>
>         session = Session()
>
>         # code under test:
>         event = Event(text='some stuff got done')
>         session.add(event)
>         session.flush()
>         session.close()
>
>     finally:
>         sub_transaction.rollback()
>         assert session.query(Event).count() == 0

Sure, but the test then fails when the code is correct:

try:
Base.metadata.create_all(bind=conn, checkfirst=False)
Session.configure(bind=conn)

sub_transaction = conn.begin_nested()
try:

session = Session()

# code under test:
event = Event(text='some stuff got done')
session.add(event)
session.flush()

session.commit()
session.close()

# test:
sub_transaction.rollback()

assert session.query(Event).count() == 1

finally:
sub_transaction.rollback()

finally:
transaction.rollback()

The panacea I'm after is to be able to run the DDL in a transaction, run
each test in a subtransaction off that which is rolled back at the end
of each test, but also be able to check that the code under test is
doing session.commit() where it should. Where the pattern we're
discussing, I certainly have the first two, but as you can see from what
I've just pasted above, the third one isn't there, but is it possible?

> the test harness is giving you two choices.  you can look at the state
> of the DB after your program has done some things and *before* your
> harness has reversed its work, or you can look at the state of the DB
> *after* your harness has reversed its work.

Unless I'm missing something, neither of these let the test confirm that
the code under test has called commit() when it should.

cheers,

Chris

[Mike Bayer]

so your app code calls .flush(), .commit(), and .close() explicitly within each persistence method?  and you need to make sure the .commit() is in the middle?   I'd probably use mock.patch for that level of granularity.   Usually I'd not have specific business methods calling commit() and close() at all, at the very least not .close().

> the test harness is giving you two choices.  you can look at the state 

> of the DB after your program has done some things and *before* your 

> harness has reversed its work, or you can look at the state of the DB 

> *after* your harness has reversed its work. 

Unless I'm missing something, neither of these let the test confirm that 

the code under test has called commit() when it should.

cheers,

Chris

-- 

SQLAlchemy - 

The Python SQL Toolkit and Object Relational Mapper

http://www.sqlalchemy.org/

To post example code, please provide an MCVE: Minimal, Complete, and Verifiable Example.  See  http://stackoverflow.com/help/mcve for a full description.

--- 

You received this message because you are subscribed to the Google Groups "sqlalchemy" group.

To unsubscribe from this group and stop receiving emails from it, send an email to sqlalchemy+...@googlegroups.com.

To post to this group, send email to sqlal...@googlegroups.com.

Visit this group at https://groups.google.com/group/sqlalchemy.

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/d7f7c9ac-2d32-6e60-efac-f86b501683f7%40withers.org.

[Chris Withers]

On 05/06/2019 20:47, Mike Bayer wrote:
>
>
>> The panacea I'm after is to be able to run the DDL in a transaction, run
>> each test in a subtransaction off that which is rolled back at the end
>> of each test, but also be able to check that the code under test is
>> doing session.commit() where it should. Where the pattern we're
>> discussing, I certainly have the first two, but as you can see from what
>> I've just pasted above, the third one isn't there, but is it possible?
>
>
> so your app code calls .flush(), .commit(), and .close() explicitly
> within each persistence method?

Not really, it's more framework code, but most frameworks nowadays
provide testing methods along the lines of:

client.get('/something?whatever=1')

...and they exercise the full request handling code, that tends to
include some form of session lifecycle middleware or what have you.

> and you need to make sure the .commit()
> is in the middle?

Yep.

> I'd probably use mock.patch for that level of
> granularity.   Usually I'd not have specific business methods calling
> commit() and close() at all, at the very least not .close().

Okay, so sounds like in an ideal world, the framework should provide a
way to sub out that transaction middleware when unit testing and then
for functional testing, I just need to fall back to dropping everything
in the db, or having a fresh db created from scratch?

cheers,

Chris

[Mike Bayer]

why can't you use the rollback fixture for functional testing?  whether or not you can assert that commit() was called, you can still have an enclosing transaction that gets rolled back.

cheers,

Chris

-- 

SQLAlchemy - 

The Python SQL Toolkit and Object Relational Mapper

http://www.sqlalchemy.org/

To post example code, please provide an MCVE: Minimal, Complete, and Verifiable Example.  See  http://stackoverflow.com/help/mcve for a full description.

--- 

You received this message because you are subscribed to the Google Groups "sqlalchemy" group.

To unsubscribe from this group and stop receiving emails from it, send an email to sqlalchemy+...@googlegroups.com.

To post to this group, send email to sqlal...@googlegroups.com.

Visit this group at https://groups.google.com/group/sqlalchemy.

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/c5c0a17c-0671-b86c-842e-f03cded132ba%40withers.org.

[Chris Withers]

On 10/06/2019 15:40, Mike Bayer wrote:
>
>
>> Okay, so sounds like in an ideal world, the framework should provide a
>> way to sub out that transaction middleware when unit testing and then
>> for functional testing, I just need to fall back to dropping everything
>> in the db, or having a fresh db created from scratch?
>
>
> why can't you use the rollback fixture for functional testing?  whether
> or not you can assert that commit() was called, you can still have an
> enclosing transaction that gets rolled back.

Sorry, I missed this. This sounds right, but I seem to remember hitting
problems with this, and then I got pulled onto another project.

Chris