gristlabs / grist-core

Grist is the evolution of spreadsheets.
https://www.getgrist.com
Apache License 2.0
7.27k stars 322 forks source link

CI: add server tests with Postgresql #1278

Open fflorent opened 2 weeks ago

fflorent commented 2 weeks ago

Context

See this comment: https://github.com/gristlabs/grist-core/pull/1205#discussion_r1815383501

Some server tests succeeded when using sqlite but failed with postgresql. This commit runs the same tests using a postgresql database to ensure both database types are supported for these tests.

Proposed solution

Make the CI run server tests with Postgresql

Related issues

Has this been tested?

fflorent commented 1 week ago

When Paul has noticed this regression specific to Postgresql, we thought we may also run the tests with this database in github CI. Though it does not seem to be as easy as I first thought. We have understood from @berhalak comment that you already run server tests against this database in your internal CI. Is it possible you either open-source the tests, or share with us how you run them so we can add the same logic to the Github CI, if that makes sense to you?

Or do you think this work is not worth the effort?

paulfitz commented 1 week ago

Is it possible you either open-source the tests, or share with us how you run them so we can add the same logic to the Github CI, if that makes sense to you?

This is what we have:

RUN_gen_server_pg() {
  # Repeat some tests against postgres
  run with_postgres env TEST_CLEAN_DATABASE=true TEST_REDIS_URL=redis://localhost/{redis} TESTDIR=$OUTDIR/testdir \
    $BIN/mocha '_build/test/gen-server/**/*.js' '_build/core/test/gen-server/**/*.js' $MOCHA_FLAGS
}

So we repeat the gen-server tests against postgres. I don't see the server tests running against postgres. They may need modification to do so if they assume the database is a file.

These tests weren't enough to catch the IN () problem, which only showed up in a completely different battery of tests we apply to test the quality of an existing deployment.