schireson / pytest-mock-resources

Pytest Fixtures that let you actually test against external resource (Postgres, Mongo, Redshift...) dependent code.
https://pytest-mock-resources.readthedocs.io/en/latest/quickstart.html
MIT License
179 stars 19 forks source link

feat: Add flag to clean up postgres databases. #204

Open DanCardin opened 6 months ago

DanCardin commented 6 months ago

Fixes https://github.com/schireson/pytest-mock-resources/issues/202.

It probably makes sense to follow this up with a pmr --cleanup or the like kind of CLI extension, that more directly just deletes all pmr_* named databases. If there's a hard test failure, you enter a debugger and exit uncleanly, you ctrl+c enough, or various other means: it's still fairly straightforward to end up with left over databases.

1 unresolved issue:

josiah-lunit commented 6 months ago

For issues regarding pytest-asyncio (which I agree is not in a good state now), I find using nest_asyncio resolved some of the issues I had with different scoped fixtures.

DanCardin commented 6 months ago

A potentially good callout, although it'd mean it would mandate use of it for downstream users, not just our tests.

The current test failures seem to not be related at least...which makes sense, since i'm bypassing the new fixture for async code.

They're also not a failure I'm experiencing locally...

coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 9764591739

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
src/pytest_mock_resources/fixture/postgresql.py 123 125 98.4%
<!-- Total: 155 157 98.73% -->
Totals Coverage Status
Change from base Build 9589737334: 1.5%
Covered Lines: 1609
Relevant Lines: 1753

πŸ’› - Coveralls
DanCardin commented 2 months ago

Sigh, the moment I got passing CI, I've now realized that there's a pytest deficiency that seems not to allow me to dynamically create new fixtures with variable dependent fixtures. at least not ergonomically. What i assume must be happening is that this is now forcing unnecessary database creations due to that.

I suppose I'll have to revise this further...

josiah-lunit commented 2 months ago

Sigh, the moment I got passing CI, I've now realized that there's a pytest deficiency that seems not to allow me to dynamically create new fixtures with variable dependent fixtures. at least not ergonomically. What i assume must be happening is that this is now forcing unnecessary database creations due to that.

I suppose I'll have to revise this further...

I appreciate the effort! For now we are using something like the following to scan for pmr resources and drop them after the testing suite is complete:

async def clean_databases(engine: AsyncEngine):
    """
    Needed because `pytest-mock-resources` doesn't clean up after itself if you connect to an existing database.
    See https://github.com/schireson/pytest-mock-resources/issues/202
    """
    async with engine.connect() as conn:
        query = "SELECT datname FROM pg_database WHERE datname LIKE 'pytest_mock_resource%';"
        databases = await conn.scalars(text(query))
        for database in databases:
            try:
                query = f"DROP DATABASE {database}"
                await conn.execute(text(query))
            except Exception as e:
                logging.error(f"Could not drop database {database}: {e}")
                raise e
        query = "SELECT datname FROM pg_database WHERE datname LIKE 'pmr_template_pg_%';"
        templates = await conn.scalars(text(query))
        for template in templates:
            try:
                query = f"DROP DATABASE {template}"
                await conn.execute(text(query))
            except Exception as e:
                logging.error(f"Could not drop template {template}: {e}")

@pytest_asyncio.fixture(scope="session")
async def alembic_engine(postgres_async_alembic: AsyncEngine, postgres_async_teardown: AsyncEngine):
    yield postgres_async_alembic
    await postgres_async_alembic.dispose()
    await clean_databases(postgres_async_teardown)
DanCardin commented 2 months ago

Well amusingly we recently ran into issues in CI related to this at $job, so it's now in our critical path πŸ˜†. I think the solution for vanilla test databases is a lot simpler, and will be my shorter term solution here. It's the template databases which are the complicated problem.