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.

Sign up for GitHub

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

CI: Run gen-server tests with Postgres #1336

Merged
merged 9 commits into from
Dec 15, 2024
Merged

CI: Run gen-server tests with Postgres #1336

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
bde7c37
CI: add server tests with Postgresql
fflorent Oct 28, 2024
7575c20
Regenerate initialDocSql.ts
SleepyLeslie Dec 5, 2024
a608fc5
Turn off Postgres JIT in CI workflow
SleepyLeslie Dec 6, 2024
046dd49
Separate server and gen-server tests
SleepyLeslie Dec 6, 2024
dfac3ae
Print Postgres JIT status before running tests
SleepyLeslie Dec 6, 2024
95995fd
Use POSTGRES_INITDB_ARGS
SleepyLeslie Dec 6, 2024
8729626
Update test:gen-server target
SleepyLeslie Dec 9, 2024
8662fdf
Revert "Regenerate initialDocSql.ts"
SleepyLeslie Dec 9, 2024
144f3a1
Split gen-server tests with sqlite/postgres
SleepyLeslie Dec 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 41 additions & 2 deletions .github/workflows/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ jobs:
- ':lint:python:client:common:smoke:stubs:'
- ':server-1-of-2:'
- ':server-2-of-2:'
- ':gen-server-1-of-2:'
- ':gen-server-2-of-2:'
- ':nbrowser-^[A-D]:'
- ':nbrowser-^[E-L]:'
- ':nbrowser-^[M-N]:'
Expand Down Expand Up @@ -63,7 +65,7 @@ jobs:
run: yarn run lint:ci

- name: Make sure bucket is versioned
if: contains(matrix.tests, ':server-')
if: contains(matrix.tests, ':server-') || contains(matrix.tests, ':gen-server-')
env:
AWS_ACCESS_KEY_ID: administrator
AWS_SECRET_ACCESS_KEY: administrator
Expand Down Expand Up @@ -96,12 +98,31 @@ jobs:
if: contains(matrix.tests, ':stubs:')
run: MOCHA_WEBDRIVER_HEADLESS=1 yarn run test:stubs

- name: Run gen-server tests with minio and redis
if: contains(matrix.tests, ':gen-server-')
run: |
PGPASSWORD=db_password psql -h localhost -U db_user -w db_name -c "SHOW ALL;" | grep ' jit '
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this? Looks like a print for a test, I just want to be sure of that :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An extra bit of info to confirm JIT is turned off successfully. For me it was useful for debugging CI problems, and I just kept it.

export TEST_SPLITS=$(echo $TESTS | sed "s/.*:gen-server-\([^:]*\).*/\1/")
yarn run test:gen-server
TYPEORM_TYPE=postgres TYPEORM_HOST=localhost TYPEORM_DATABASE=db_name TYPEORM_USERNAME=db_user TYPEORM_PASSWORD=db_password yarn run test:gen-server
env:
MOCHA_WEBDRIVER_HEADLESS: 1
TESTS: ${{ matrix.tests }}
GRIST_DOCS_MINIO_ACCESS_KEY: administrator
GRIST_DOCS_MINIO_SECRET_KEY: administrator
TEST_REDIS_URL: "redis://localhost/11"
GRIST_DOCS_MINIO_USE_SSL: 0
GRIST_DOCS_MINIO_ENDPOINT: localhost
GRIST_DOCS_MINIO_PORT: 9000
GRIST_DOCS_MINIO_BUCKET: grist-docs-test

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about splitting this up into two steps (e.g. Run gen-server tests with postgres and Run gen-server tests with sqlite)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also think it logically makes more sense to split them, but I put them together to avoid excessive duplication in the env section. Do you feel strongly about it?

Copy link
Collaborator

@fflorent fflorent Dec 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would yaml's anchor / alias solve your issue? This resource help understanding the concept: https://www.educative.io/blog/advanced-yaml-syntax-cheatsheet

      - name: Run gen-server tests with minio and redis with sqlite
        if: contains(matrix.tests, ':gen-server-')
        run: |
          export TEST_SPLITS=$(echo $TESTS | sed "s/.*:gen-server-\([^:]*\).*/\1/")
          yarn run test:gen-server
        env: &gen-server-env-vars
          MOCHA_WEBDRIVER_HEADLESS: 1
          TESTS: ${{ matrix.tests }}
          GRIST_DOCS_MINIO_ACCESS_KEY: administrator
          GRIST_DOCS_MINIO_SECRET_KEY: administrator
          TEST_REDIS_URL: "redis://localhost/11"
          GRIST_DOCS_MINIO_USE_SSL: 0
          GRIST_DOCS_MINIO_ENDPOINT: localhost
          GRIST_DOCS_MINIO_PORT: 9000
          GRIST_DOCS_MINIO_BUCKET: grist-docs-test

      - name: Run gen-server tests with minio and redis with postgresql
        if: contains(matrix.tests, ':gen-server-')
        run: |
          PGPASSWORD=db_password psql -h localhost -U db_user -w db_name -c "SHOW ALL;" | grep ' jit '
          export TEST_SPLITS=$(echo $TESTS | sed "s/.*:gen-server-\([^:]*\).*/\1/")
          yarn run test:gen-server
        env: 
          <<: *gen-server-env-vars
          TYPEORM_TYPE: postgres
          TYPEORM_HOST: localhost
          TYPEORM_DATABASE: db_name
          TYPEORM_USERNAME: db_user
          TYPEORM_PASSWORD: db_password
          # Also change the TEST_REDIS_URL value?

And you might want to factorize a bit more… or not… Sometimes we have to choose a tradeoff between readability and factorization.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fflorent Sadly anchors are not supported by GH actions: actions/runner#1182

- name: Run server tests with minio and redis
if: contains(matrix.tests, ':server-')
run: |
export TEST_SPLITS=$(echo $TESTS | sed "s/.*:server-\([^:]*\).*/\1/")
MOCHA_WEBDRIVER_HEADLESS=1 yarn run test:server
yarn run test:server
env:
MOCHA_WEBDRIVER_HEADLESS: 1
TESTS: ${{ matrix.tests }}
GRIST_DOCS_MINIO_ACCESS_KEY: administrator
GRIST_DOCS_MINIO_SECRET_KEY: administrator
Expand Down Expand Up @@ -167,6 +188,24 @@ jobs:
--health-timeout 5s
--health-retries 5

postgresql:
image: postgres:latest
env:
POSTGRES_USER: db_user
POSTGRES_PASSWORD: db_password
POSTGRES_DB: db_name
# JIT is enabled by default since Postgres 17 and has a huge negative impact on performance,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# JIT is enabled by default since Postgres 17 and has a huge negative impact on performance,
# JIT is enabled by default since Postgres 17 and has a huge negative impact on tests performance,

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It affects not only test performance. See https://support.getgrist.com/self-managed/#what-is-a-home-database

# making many tests timeout.
# https://support.getgrist.com/self-managed/#what-is-a-home-database
POSTGRES_INITDB_ARGS: "-c jit=off"
ports:
- 5432:5432
options: >-
--health-cmd "pg_isready -U db_user"
--health-interval 10s
--health-timeout 5s
--health-retries 5

candidate:
needs: build_and_test
if: ${{ success() && github.event_name == 'push' }}
Expand Down
4 changes: 2 additions & 2 deletions app/server/lib/initialDocSql.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ CREATE TABLE IF NOT EXISTS "_grist_REPL_Hist" (id INTEGER PRIMARY KEY, "code" TE
CREATE TABLE IF NOT EXISTS "_grist_Attachments" (id INTEGER PRIMARY KEY, "fileIdent" TEXT DEFAULT '', "fileName" TEXT DEFAULT '', "fileType" TEXT DEFAULT '', "fileSize" INTEGER DEFAULT 0, "fileExt" TEXT DEFAULT '', "imageHeight" INTEGER DEFAULT 0, "imageWidth" INTEGER DEFAULT 0, "timeDeleted" DATETIME DEFAULT NULL, "timeUploaded" DATETIME DEFAULT NULL);
CREATE TABLE IF NOT EXISTS "_grist_Triggers" (id INTEGER PRIMARY KEY, "tableRef" INTEGER DEFAULT 0, "eventTypes" TEXT DEFAULT NULL, "isReadyColRef" INTEGER DEFAULT 0, "actions" TEXT DEFAULT '', "label" TEXT DEFAULT '', "memo" TEXT DEFAULT '', "enabled" BOOLEAN DEFAULT 0, "watchedColRefList" TEXT DEFAULT NULL, "options" TEXT DEFAULT '');
CREATE TABLE IF NOT EXISTS "_grist_ACLRules" (id INTEGER PRIMARY KEY, "resource" INTEGER DEFAULT 0, "permissions" INTEGER DEFAULT 0, "principals" TEXT DEFAULT '', "aclFormula" TEXT DEFAULT '', "aclColumn" INTEGER DEFAULT 0, "aclFormulaParsed" TEXT DEFAULT '', "permissionsText" TEXT DEFAULT '', "rulePos" NUMERIC DEFAULT 1e999, "userAttributes" TEXT DEFAULT '', "memo" TEXT DEFAULT '');
INSERT INTO _grist_ACLRules VALUES(1,1,63,'[1]','',0,'','',1e999,'','');
INSERT INTO _grist_ACLRules VALUES(1,1,63,'[1]','',0,'','',9.0e+999,'','');
SleepyLeslie marked this conversation as resolved.
Show resolved Hide resolved
CREATE TABLE IF NOT EXISTS "_grist_ACLResources" (id INTEGER PRIMARY KEY, "tableId" TEXT DEFAULT '', "colIds" TEXT DEFAULT '');
INSERT INTO _grist_ACLResources VALUES(1,'','');
CREATE TABLE IF NOT EXISTS "_grist_ACLPrincipals" (id INTEGER PRIMARY KEY, "type" TEXT DEFAULT '', "userEmail" TEXT DEFAULT '', "userName" TEXT DEFAULT '', "groupName" TEXT DEFAULT '', "instanceId" TEXT DEFAULT '');
Expand Down Expand Up @@ -82,7 +82,7 @@ CREATE TABLE IF NOT EXISTS "_grist_REPL_Hist" (id INTEGER PRIMARY KEY, "code" TE
CREATE TABLE IF NOT EXISTS "_grist_Attachments" (id INTEGER PRIMARY KEY, "fileIdent" TEXT DEFAULT '', "fileName" TEXT DEFAULT '', "fileType" TEXT DEFAULT '', "fileSize" INTEGER DEFAULT 0, "fileExt" TEXT DEFAULT '', "imageHeight" INTEGER DEFAULT 0, "imageWidth" INTEGER DEFAULT 0, "timeDeleted" DATETIME DEFAULT NULL, "timeUploaded" DATETIME DEFAULT NULL);
CREATE TABLE IF NOT EXISTS "_grist_Triggers" (id INTEGER PRIMARY KEY, "tableRef" INTEGER DEFAULT 0, "eventTypes" TEXT DEFAULT NULL, "isReadyColRef" INTEGER DEFAULT 0, "actions" TEXT DEFAULT '', "label" TEXT DEFAULT '', "memo" TEXT DEFAULT '', "enabled" BOOLEAN DEFAULT 0, "watchedColRefList" TEXT DEFAULT NULL, "options" TEXT DEFAULT '');
CREATE TABLE IF NOT EXISTS "_grist_ACLRules" (id INTEGER PRIMARY KEY, "resource" INTEGER DEFAULT 0, "permissions" INTEGER DEFAULT 0, "principals" TEXT DEFAULT '', "aclFormula" TEXT DEFAULT '', "aclColumn" INTEGER DEFAULT 0, "aclFormulaParsed" TEXT DEFAULT '', "permissionsText" TEXT DEFAULT '', "rulePos" NUMERIC DEFAULT 1e999, "userAttributes" TEXT DEFAULT '', "memo" TEXT DEFAULT '');
INSERT INTO _grist_ACLRules VALUES(1,1,63,'[1]','',0,'','',1e999,'','');
INSERT INTO _grist_ACLRules VALUES(1,1,63,'[1]','',0,'','',9.0e+999,'','');
CREATE TABLE IF NOT EXISTS "_grist_ACLResources" (id INTEGER PRIMARY KEY, "tableId" TEXT DEFAULT '', "colIds" TEXT DEFAULT '');
INSERT INTO _grist_ACLResources VALUES(1,'','');
CREATE TABLE IF NOT EXISTS "_grist_ACLPrincipals" (id INTEGER PRIMARY KEY, "type" TEXT DEFAULT '', "userEmail" TEXT DEFAULT '', "userName" TEXT DEFAULT '', "groupName" TEXT DEFAULT '', "instanceId" TEXT DEFAULT '');
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
"test:stubs": "GRIST_TEST_LOGIN=1 ./test/test_env.sh mocha ${DEBUG:+-b --no-exit} $([ -z $DEBUG ] && echo --forbid-only) -g \"${GREP_TESTS}\" --slow 8000 -R test/xunit-file '_build/test/nbrowser_with_stubs/**/*.js'",
"test:client": "./test/test_env.sh mocha ${DEBUG:+'-b'} '_build/test/client/**/*.js'",
"test:common": "./test/test_env.sh mocha ${DEBUG:+'-b'} '_build/test/common/**/*.js'",
"test:server": "TEST_SUITE=server TEST_SUITE_FOR_TIMINGS=server TIMINGS_FILE=test/timings/server.txt ./test/test_env.sh mocha ${DEBUG:+'-b'} -g \"${GREP_TESTS}\" -R test/xunit-file '_build/test/server/**/*.js' '_build/test/gen-server/**/*.js'",
"test:server": "TEST_SUITE=server TEST_SUITE_FOR_TIMINGS=server TIMINGS_FILE=test/timings/server.txt ./test/test_env.sh mocha ${DEBUG:+'-b'} -g \"${GREP_TESTS}\" -R test/xunit-file '_build/test/server/**/*.js'",
"test:gen-server": "TEST_SUITE=gen-server TEST_SUITE_FOR_TIMINGS=gen-server TIMINGS_FILE=test/timings/gen-server.txt ./test/test_env.sh mocha ${DEBUG:+'-b'} -g \"${GREP_TESTS}\" -R test/xunit-file '_build/test/gen-server/**/*.js'",
"test:smoke": "./test/test_env.sh mocha _build/test/nbrowser/Smoke.js",
"test:docker": "./test/test_under_docker.sh",
"test:python": "sandbox_venv3/bin/python sandbox/grist/runtests.py ${GREP_TESTS:+discover -p \"test*${GREP_TESTS}*.py\"}",
Expand Down