Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

storage: expose test postgres client and add migrations test #411

Merged
merged 1 commit into from May 10, 2023

Conversation

ptrus
Copy link
Member

@ptrus ptrus commented May 9, 2023

This introduces a postgres testutils package, which allows connecting to the test database from other tests in the codebase. An analyzer test that executes the migrations is also included as an example of usage (the test ensures migrations are applied without errors).

This was needed/useful in #389 and was extracted into a separate PR.

@ptrus ptrus changed the title storage: expose test postgres client and add migration tests storage: expose test postgres client and add migrations test May 9, 2023
@ptrus ptrus force-pushed the ptrus/feature/test-postgres-client branch from 0f3a601 to 4621f9f Compare May 9, 2023 19:39
Copy link
Collaborator

@mitjat mitjat left a comment

Choose a reason for hiding this comment

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

Nice cleanup, thank you!

@@ -34,7 +34,7 @@ jobs:
ports:
- 5432:5432
env:
CI_TEST_CONN_STRING: "postgresql://postgres:postgres@127.0.0.1:5432/postgres"
CI_TEST_CONN_STRING: "postgresql://postgres:postgres@127.0.0.1:5432/postgres?sslmode=disable"
Copy link
Collaborator

Choose a reason for hiding this comment

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

What changed so that this switch is needed?

Copy link
Member Author

@ptrus ptrus May 9, 2023

Choose a reason for hiding this comment

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

The golang-migrate (library used for migrations) requires it for local db (it tries to use a ssl connection by default).

Before, migrations were never run in unit tests. Other parts of the code (e.g. e2e-tests) already have this configured: https://github.com/oasisprotocol/oasis-indexer/blob/4d3a6adc8765e2414fe82fa2e403771e96535118/tests/e2e_regression/e2e_config.yml#L26

@mitjat mitjat force-pushed the ptrus/feature/test-postgres-client branch from 4621f9f to 959e05e Compare May 9, 2023 20:07
@ptrus ptrus force-pushed the ptrus/feature/test-postgres-client branch from 959e05e to c904a0e Compare May 10, 2023 10:26
@ptrus ptrus force-pushed the ptrus/feature/test-postgres-client branch from c904a0e to 3509fa9 Compare May 10, 2023 10:31
@ptrus ptrus merged commit 63023bb into main May 10, 2023
5 checks passed
@ptrus ptrus deleted the ptrus/feature/test-postgres-client branch May 10, 2023 10:38
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.

None yet

2 participants