#3 tests: simplify db fixtures a bit
Closed 3 years ago by dcallagh. Opened 3 years ago by dcallagh.
dcallagh/waiverdb tests-dont-wipe-db  into  master

tests: simplify db fixtures a bit
Dan Callaghan • 3 years ago  
file modified
+3 -33
@@ -14,12 +14,15 @@ 

  import pytest

  

  from waiverdb.app import create_app, init_db

+ from waiverdb.models import db

  

  @pytest.fixture(scope='session')

  def app(tmpdir_factory, request):

      app = create_app('waiverdb.config.TestingConfig')

      db_file = tmpdir_factory.mktemp('waiverdb').join('db.sqlite')

      app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite:///%s' % db_file

+     db.init_app(app)
mjia commented 3 years ago

No need to do this. The db is already initialized when the app is created.

If you take this out it fails. That's because the db instance (which is a single shared global) is not hooked up to the app instance, which we only just created in this function.

mjia commented 3 years ago

It works fine in my evn. The global is hooked already in the create_app function. See line 47 at waiverdb/app.py

+     init_db(app)

      # Establish an application context before running the tests.

      ctx = app.app_context()

      ctx.push()
@@ -30,39 +33,6 @@ 

      request.addfinalizer(teardown)

      return app

  

- @pytest.fixture(scope='session')
mjia commented 3 years ago

My idea of using session is that each test will be running in their own fresh db. In this way, I do not need to worry about what other tests have put into the db.

Ahh but I don't think that actually works, does it? It is init'ing and dropping the db here in this fixture but it is session scoped. So you still get the same db for the entire test run.

mjia commented 3 years ago

Each test is wrapped in a transaction so it will not insert any data into the db since the transaction is rolled back on test teardown.

How does that work when the request handler does db.session.commit() though?

Also will this break down if a particular test doesn't explicitly request the session fixture?

mjia commented 3 years ago

If the session fixture is not used, it may break down as it could insert some bad data. We could set autouse=True or just need to keep in mind for using it, :-)

- def db(app, request):

-     """Session-wide test database."""

-     db = init_db(app)

-     def teardown():

-         db.drop_all()
mjia commented 3 years ago

This is just for keeping the sqlite db file as small as possible. If we have set up a Jenkins job to run the tests, it will generate a lot of temporary files on the server. It might cause problems. For debugging purpose, people can just hack the code to do it.

The pytest tempdir fixture already takes care of rotating the directories so they don't grow infinitely, while still keeping recent ones to be inspected by hand if necessary. That was why I suggested to use it :-)

-     request.addfinalizer(teardown)

-     return db

- 

- @pytest.fixture(scope='function')

- def session(db, request):

-     """Creates a new database session for a test."""

-     connection = db.engine.connect()

-     transaction = connection.begin()

- 

-     # https://github.com/mitsuhiko/flask-sqlalchemy/issues/345

-     class _dict(dict):

-         def __nonzero__(self):

-             return True

- 

-     options = dict(bind=connection, binds=_dict())

-     session = db.create_scoped_session(options=options)

- 

-     db.session = session

- 

-     def teardown():

-         transaction.rollback()

-         connection.close()

-         session.remove()

- 

-     request.addfinalizer(teardown)

-     return session

- 

  @pytest.yield_fixture

  def client(app):

      """A Flask test client. An instance of :class:`flask.testing.TestClient`

file modified
+1 -1
@@ -12,7 +12,7 @@ 

  import pytest

  import json

  

- def test_create_waiver(client, session):

+ def test_create_waiver(client):

      data = {

          'result_id': 123,

          'product_version': 'fool-1',

We don't need to manage the session stuff manually, Flask-SQLAlchemy
already handles that if we hook it up correctly:

See: http://flask-sqlalchemy.pocoo.org/2.1/contexts/

We also don't need to drop all tables (it's already in a temp directory)
and leaving the data behind can help when investigating failing tests.

My idea of using session is that each test will be running in their own fresh db. In this way, I do not need to worry about what other tests have put into the db.

This is just for keeping the sqlite db file as small as possible. If we have set up a Jenkins job to run the tests, it will generate a lot of temporary files on the server. It might cause problems. For debugging purpose, people can just hack the code to do it.

No need to do this. The db is already initialized when the app is created.

The pytest tempdir fixture already takes care of rotating the directories so they don't grow infinitely, while still keeping recent ones to be inspected by hand if necessary. That was why I suggested to use it :-)

Ahh but I don't think that actually works, does it? It is init'ing and dropping the db here in this fixture but it is session scoped. So you still get the same db for the entire test run.

If you take this out it fails. That's because the db instance (which is a single shared global) is not hooked up to the app instance, which we only just created in this function.

It works fine in my evn. The global is hooked already in the create_app function. See line 47 at waiverdb/app.py

Each test is wrapped in a transaction so it will not insert any data into the db since the transaction is rolled back on test teardown.

How does that work when the request handler does db.session.commit() though?

Also will this break down if a particular test doesn't explicitly request the session fixture?

If the session fixture is not used, it may break down as it could insert some bad data. We could set autouse=True or just need to keep in mind for using it, :-)

Okay I'm on board. So if we want to inspect the state of the db after a test we have to hack it a bit (comment out rollback and drop_all) but the upside is each test starts from a clean db and doesn't have to worry about being influenced by leftover rows.

Will abandon this.

Pull-Request has been closed by dcallagh

3 years ago