Use postgres for CI run_unit_tests#286
Conversation
knikolla
left a comment
There was a problem hiding this comment.
Does this still allow to optionally run the tests with SQLite? Running tests with Postgres should be an additional thing rather than completely removing SQLite.
521061e to
e510a71
Compare
|
@knikolla I've made the one test case in |
knikolla
left a comment
There was a problem hiding this comment.
Before the introduction of this change, you could run setup.sh followed by run_unit_tests.sh in your local machine without having to setup anything externally and they would work on both Mac and Linux. These changes break that.
Please preserve original behavior in the original functioning of the scripts.
For example, maybe add the postgresql user and db configuration and DB_URL variable export within the GitHub action workflow, rather than the script file itself. This way the CI in GitHub will use postgres whereas when a user is testing in their local machine and running the file they will still work as before.
e510a71 to
424d74e
Compare
The production Coldfront uses Postgres. The tests will now more accurately reflect the production environment. Local testing can still be done with SQLite by not setting the `DB_URL` environment variable. A Postgres-specific bug was found regarding creating Field Of Science entries. More details in `tests/unit/test_migrate_field_of_science.py`.
424d74e to
af7c67a
Compare
There was a problem hiding this comment.
Pull request overview
This pull request migrates the CI testing infrastructure from MySQL to PostgreSQL to better match the production environment. The change addresses a specific bug where Postgres enforces character limits on database fields that MySQL doesn't, which was discovered when trying to add new allocation attributes.
Changes:
- Adds Postgres support to CI workflows by setting the DB_URL environment variable
- Installs and configures PostgreSQL service in the CI setup script
- Fixes a Postgres-specific sequence synchronization issue for FieldOfScience model tests
- Consolidates CI setup across workflows to use a common setup.sh script
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test-requirements.txt | Adds psycopg2 package for PostgreSQL database connectivity |
| src/coldfront_plugin_cloud/tests/unit/test_migrate_field_of_science.py | Adds setUp method to reset Postgres sequence for FieldOfScience IDs to prevent insertion conflicts |
| ci/setup.sh | Starts PostgreSQL service and sets password when running in CI environment |
| .github/workflows/test-unit.yaml | Adds DB_URL environment variable pointing to Postgres, updates step name |
| .github/workflows/test-functional-microstack.yaml | Adds DB_URL environment variable, consolidates setup to use setup.sh script, updates step name |
| .github/workflows/test-functional-microshift.yaml | Adds DB_URL environment variable for consistency with other workflows |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Closes #254.
A Postgres-specific bug was found regarding creating Field Of Science entries. More details in
tests/unit/test_migrate_field_of_science.py.