Chiel dat 3282 set up ci multiple db #26
Conversation
DAT-3282 Set up CI
Why and what Supporting material (links, screenshots, documents, logs, stacktraces) . |
| logging.basicConfig(level=logging.WARN) | ||
|
|
||
| TEST_MYSQL_CONN_STRING = "mysql://mysql:Password1@localhost/mysql" | ||
| TEST_CONN_STRING = os.environ.get("TEST_CONN_STRING", "mysql://mysql:Password1@localhost/mysql") |
There was a problem hiding this comment.
It's probably better to remove the default and throw an error message Because the error "bad username" or "no such database" can be confusing, and no one will know to look here for it.
There was a problem hiding this comment.
Sounds reasonable. Will change that.
There was a problem hiding this comment.
My idea initially to use this as a default is that nothing would change. FYI ;)
| db-conn: [ | ||
| {db: mysql, conn: "mysql://mysql:Password1@localhost/mysql"}, | ||
| {db: postgres, conn: "postgres://postgres:Password1@localhost/postgres"} | ||
| ] |
There was a problem hiding this comment.
I like how it's progressing!
|
Hmmm... it's interesting to write the suite generically, and then have CI run against all of them. What I don't love about it though is that it doesn't run them all locally, but that also doesn't seem completely realistic... hmmm |
|
If you're referring to the cloud databases, maybe we can randomize the dataset name, so there won't be any collisions? (unless that's not the issue) |
|
@sirupsen If I understand you correctly, the thing that you don't love is that you need to fire multiple commands with multiple environment variables to run them all locally? I think we could use a make file, such that it is still only one command. Like |
|
@erezsh The tests are not referring to cloud databases. They could be empty databases for all that matter. I don't think we want to make our unit tests dependent on external databases. If we would write other types of tests, like integration tests or performance tests, then using external cloud databases would be reasonable. |
|
@sirupsen It makes sense to me to have "short tests" that run for each commit, and "full tests" that run before a release, which can include the cloud support. |
|
@erezsh Can you be more specific what you want to test with "full tests"? Do you mean, performance, integration (with other services), more edge cases, just more test coverage, etc? |
|
It's always good to have more of everything, but I was talking about basic coverage - to see the tool runs on every database with every feature enabled. |
…umns postgres override select_table_unique_columns
Now added testing for also Postgres with the unit test as is. Postgres unit tests are mostly failing for now.