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

Add testing support for postgres #31

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add testing support for postgres #31

wants to merge 5 commits into from

Conversation

dwhswenson
Copy link
Member

This works locally, although it may take some work to get it running in the CI workflow.

  • Opted for direct install of postgres, since that seemed easier than Docker.
  • Switch which backend you're using by setting the env var EXORCIST_TEST_DB; sqlite (default) or postgres
    • All tests will run; only the ones that build off the fresh_db (which is most of them) will switch database backend. Obviously, tests of from_filename are still using SQLite even if you request postgres.
  • multiple devtools scripts created, partly for future flexibility, mainly for convenience when testing locally
    • separate install and create stages because create is more likely to be reused again locally than install (e.g., delete the directory that contains the DB)
    • extra postgres 1-liners for restart and stop mainly so I don't have to look up the pg_ctl documentation (since I probably won't even remember that the command is called pg_ctl).

@codecov
Copy link

codecov bot commented Aug 29, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.20% 🎉

Comparison is base (c0722a5) 91.44% compared to head (2efdc46) 91.65%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #31      +/-   ##
==========================================
+ Coverage   91.44%   91.65%   +0.20%     
==========================================
  Files           7        7              
  Lines         526      539      +13     
==========================================
+ Hits          481      494      +13     
  Misses         45       45              
Files Changed Coverage Δ
exorcist/tests/test_taskstatusdb.py 99.34% <100.00%> (+0.02%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@richardjgowers
Copy link
Contributor

@mikemhenry is probably going to be the CI wizard here getting postgres hooked up

@dwhswenson rather than toggling between sqlite/other backends, could instead the fixture just look for a $POSTGRES_BACKEND env variable and skip testing postgres if not found? This will be frustrating to test multiple backends at once otherwise

@mikemhenry
Copy link

If this is working then I am not sure if you need me to do anything, but this is how I did this in the past:

https://github.com/samplchallenges/SAMPL-league/blob/main/.github/workflows/CI.yml#L23-L38

Then just used the standard stuff with pytest/django testing but I think what David has setup makes it easier to run tests locally if you don't want to use your system's postgress or run a container locally

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