Skip to content

Use a temporary docker postgres container for postgres DB sync tests#1243

Open
austinweisgrau wants to merge 8 commits intomove-coop:mainfrom
austinweisgrau:postgres
Open

Use a temporary docker postgres container for postgres DB sync tests#1243
austinweisgrau wants to merge 8 commits intomove-coop:mainfrom
austinweisgrau:postgres

Conversation

@austinweisgrau
Copy link
Copy Markdown
Collaborator

@austinweisgrau austinweisgrau commented Jan 19, 2025

Should follow #1242

Uses the testcontainers package to easily generate and use a temporary postgres docker container for tests. Allows for running tests on postgres without running a persistent server separately, which should enable us to run tests on postgres in the CICD environment.

@austinweisgrau austinweisgrau force-pushed the postgres branch 2 times, most recently from 7197a5c to db794e8 Compare January 19, 2025 23:33
@bmos
Copy link
Copy Markdown
Collaborator

bmos commented Feb 20, 2026

this is such a great idea!!
we might also be able to use this approach for other database packages...

@bmos
Copy link
Copy Markdown
Collaborator

bmos commented Feb 20, 2026

If you could run ruff format and ruff check --fix and push the changes, it'll re-run the tests. I'd be happy to help get it working.

Previously, the env var PGPORT would be overridden by the default
value 5432. This still works as long as PGPORT isn't set.
prior implementation would not work if a postgres table was defined
with a schema, as the quoting of `"schema.table"` is not valid and it
needs to be `schema."table"`
this runs the test in a throwaway postgres docker container rather
than needing to run the tests on an actual persistent postgres server
Comment thread parsons/databases/postgres/postgres.py Fixed
Comment thread parsons/databases/postgres/postgres.py Fixed
Comment thread parsons/databases/postgres/postgres.py Fixed
Comment thread parsons/databases/postgres/postgres.py Fixed
Comment thread parsons/databases/postgres/postgres.py Fixed
Comment thread test/test_databases/test_dbsync.py Fixed
@austinweisgrau
Copy link
Copy Markdown
Collaborator Author

The MacOS test runner does not allow for running docker containers - I made a change in 9d64d41 to skip the tests involving a docker container if docker is not available in the environment

    try:
        import docker

        docker.from_env().ping()
    except Exception:
        pytest.skip("Docker not available")

@austinweisgrau
Copy link
Copy Markdown
Collaborator Author

Turns out you can't run docker in a Windows github actions environment either, so I took a slightly more robust skipping approach in 651cb39

    container = PostgresContainer("postgres:9.5")
    try:
        container.start()
    except Exception as e:
        pytest.skip(f"Could not start Postgres container: {e}")

@github-actions
Copy link
Copy Markdown

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  parsons/databases
  alchemy.py
  table.py
  parsons/databases/postgres
  postgres.py 104
  postgres_core.py
  postgres_create_statement.py
Project Total  

This report was generated by python-coverage-comment-action

Comment thread requirements-dev.txt
ruff==0.13.0
testfixtures==8.3.0;python_version<'3.11'
testfixtures==9.1.0;python_version>='3.11'
testcontainers==4.9.0
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
testcontainers==4.9.0
testcontainers==4.14.1

might as well use the latest version here

self.host = host or os.environ.get("PGHOST")
self.db = db or os.environ.get("PGDATABASE")
self.port = port or os.environ.get("PGPORT")
self.port = port or os.environ.get("PGPORT") or 5432
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should 5432 be a string like in test_postgres.py?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants