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 CI tests against postgresql and mysql #602

Closed
zorun opened this issue Apr 30, 2020 · 8 comments
Closed

Add CI tests against postgresql and mysql #602

zorun opened this issue Apr 30, 2020 · 8 comments

Comments

@zorun
Copy link
Collaborator

zorun commented Apr 30, 2020

Two recent examples (#601 and #577) show that sqlite behaves very differently from postgresql and mysql, and that they both get much less exposure than sqlite. This results in bugs or regressions that are discovered late.

We cannot reasonably expect contributors to test their code with sqlite, postgresql, mysql. Before upgrading to a major ihatemoney release in production, I manually check that it works with postgresql, but it's tedious.

This sounds like a job for the CI! It should run all tests against sqlite, postgresql and mysql.

I have no idea about how the CI works and if this is easy to do or not: any idea?

@indatwood
Copy link
Contributor

Hey, that's a great idea! IHM uses TravisCI, and it has support for PostgreSQL and MySQL (and actually quite a few others).

Then I'm not sure about the best way to do it, but here are a few ideas :

  1. Create a different test configuration file and a specific test case for it, and use mixins?
  2. Specify this directly via an environment variable that's passed to the CI (and locally) by tox
  3. Do it only on the CI

@zorun
Copy link
Collaborator Author

zorun commented May 24, 2020

@indatwood thanks for the pointers!

I skimmed the doc a bit, and it seems that using env variables is the way to go.

I'm testing this now on #631

@zorun
Copy link
Collaborator Author

zorun commented May 24, 2020

Fun realisation: we may need a test to check that CI tests are actually run against postgresql ^^

@zorun
Copy link
Collaborator Author

zorun commented May 24, 2020

Ok, this is the way I got it to work in #631 :

  • add an environment variable in travis config to create additional builds that use different databases
  • tell tox to pass down this environment variable to the python code
  • in the tests, if this environment variable is defined, use it to set SQLALCHEMY_DATABASE_URI

I think we don't need more tox test environments, because postgresql and mysql require configuration so they can't be run by default. It seems simpler to keep the current tox test environments, and simply allow to choose between sqlite/postgresql/mysql with an environment variable. This is open to discussion though (related to the "dependencies" point below).

Database-specific dependencies

With the current setup, we need to install psycopg2 in the dev environment. That means all tox/travis virtualenvs, whatever the database that ends up actually being used, and also the dev setup on developer machines.

This is not such a big issue because we can use psycopg2-binary which is rather lightweight and does not require any C compilation or *-dev package dependencies.

Same for mysql: despite what IHM's doc says, pymysql does not seem to require any dependencies and is fast to install, so it should be fine to add it to the dev environment.

I'm still interested to hear if it's OK to install both in the dev environment.

Checking that tests are actually run with postgresql/mysql

This part is difficult.

Given the complex chain of information from travis down to the actual tests, it's very likely that somebody will silently break the postgresql/mysql testing setup one day. Tests will appear to pass on Travis, but they would actually run with the default sqlite instead of postgresql or mysql.

For now I don't have a good idea on how to handle this.

@Glandos
Copy link
Member

Glandos commented May 24, 2020

I'm still interested to hear if it's OK to install both in the dev environment.

I am wondering if it's possible to make them optional, but requires them to run tox tests. Or make tox issue a warning when such a requirement is missing.

@zorun
Copy link
Collaborator Author

zorun commented May 24, 2020

I thought about it, but I didn't find anything in tox to do something like that (conditionally install a package depending on the value of an environment variable)

It would be easy to install dependencies in new tox test environments like py34-postgresql, but as I said I don't think adding new test environments is a good idea.

@indatwood
Copy link
Contributor

You probably can test if the Travis env variable is set (or another one like RUN_POSTGRES_TESTS) in the tests and run it only if it is the case.

And then set the variable in the Travis config :-)

@zorun
Copy link
Collaborator Author

zorun commented Jul 12, 2021

This was finally done in #631

@zorun zorun closed this as completed Jul 12, 2021
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

No branches or pull requests

3 participants