-
Notifications
You must be signed in to change notification settings - Fork 374
ci: speed up integration tests #3235
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
base: main
Are you sure you want to change the base?
Changes from all commits
ea47dab
7f69548
b449821
8db6a2e
5dd3c68
a8311d9
4bb1fe9
3715e06
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| name: "Start Databases" | ||
| description: "Kicks off the PostgreSQL, ClickHouse, and Redis containers in the background. Run this as early as possible: pulls and boots then overlap with the Go builds, and the start-services action later waits for health before migrating." | ||
|
|
||
| runs: | ||
| using: "composite" | ||
| steps: | ||
| - name: Start Database Containers | ||
| env: | ||
| CLICKHOUSE_USERNAME: "e2b" | ||
| CLICKHOUSE_PASSWORD: "clickity-clicky-click" | ||
| CLICKHOUSE_PORT: "9000" | ||
| CLICKHOUSE_DATABASE: "default" | ||
| run: | | ||
| # Everything is backgrounded so the image pulls and container boots | ||
| # overlap with the (minutes-long) package builds that follow. The | ||
| # start-services action waits for container health with a bounded | ||
| # timeout, which also surfaces any startup failure from here. | ||
| docker run -d --name postgres \ | ||
| -e POSTGRES_USER=postgres \ | ||
| -e POSTGRES_PASSWORD=local \ | ||
| -e POSTGRES_DB=mydatabase \ | ||
| -p 5432:5432 \ | ||
| --health-cmd="pg_isready -U postgres" \ | ||
| --health-interval=5s \ | ||
| --health-timeout=2s \ | ||
| --health-retries=5 \ | ||
| postgres:18 & | ||
|
|
||
| docker run -d --name redis \ | ||
| -p 6379:6379 \ | ||
| --health-cmd="redis-cli ping" \ | ||
| --health-interval=5s \ | ||
| --health-timeout=2s \ | ||
| --health-retries=5 \ | ||
| redis:8 & | ||
|
|
||
| make -C packages/clickhouse run & | ||
| shell: bash | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,25 +30,42 @@ inputs: | |
| runs: | ||
| using: "composite" | ||
| steps: | ||
| - name: Run PostgreSQL Database | ||
| # The postgres/clickhouse/redis containers were already kicked off in the | ||
| # background by the start-databases action (early in the job, so their | ||
| # pulls and boots overlapped with the package builds). Here we wait for | ||
| # database health and migrate. ClickHouse migrations run goose directly | ||
| # on the host (migrate-host): building the migrator image took ~2 min per | ||
| # job even with layer caching, for a container whose only purpose is to | ||
| # run goose against migrations that get volume-mounted anyway. | ||
| - name: Wait for Databases and Migrate | ||
| env: | ||
| TESTS_E2B_API_KEY: "e2b_5ec17bd3933af21f80dc10bba686691c4fcd7057" | ||
| TESTS_E2B_ACCESS_TOKEN: "sk_e2b_17bd3933af21f80dc10bba686691c4fcd7057123" | ||
| TESTS_SANDBOX_TEAM_ID: "834777bd-9956-45ca-b088-9bac9290e2ac" | ||
| TESTS_SANDBOX_USER_ID: "2a5a9fc5-db8d-4af7-ac9e-d0f9272463bc" | ||
| CLICKHOUSE_USERNAME: "e2b" | ||
| CLICKHOUSE_PASSWORD: "clickity-clicky-click" | ||
| CLICKHOUSE_PORT: "9000" | ||
| CLICKHOUSE_DATABASE: "default" | ||
| run: | | ||
| docker run -d --name postgres \ | ||
| -e POSTGRES_USER=postgres \ | ||
| -e POSTGRES_PASSWORD=local \ | ||
| -e POSTGRES_DB=mydatabase \ | ||
| -p 5432:5432 \ | ||
| --health-cmd="pg_isready -U postgres" \ | ||
| --health-interval=5s \ | ||
| --health-timeout=2s \ | ||
| --health-retries=5 \ | ||
| postgres:latest | ||
| while [ "$(docker inspect -f '{{.State.Health.Status}}' postgres 2>/dev/null)" != "healthy" ]; do echo "Waiting for PostgreSQL to be healthy..."; sleep 2; done | ||
| echo "PostgreSQL is healthy!" | ||
| wait_healthy() { | ||
| local name="$1" timeout="${2:-120}" i | ||
| for ((i = 0; i < timeout; i += 2)); do | ||
| if [ "$(docker inspect -f '{{.State.Health.Status}}' "$name" 2>/dev/null)" = "healthy" ]; then | ||
| echo "$name is healthy!" | ||
| return 0 | ||
| fi | ||
| echo "Waiting for $name to be healthy..." | ||
| sleep 2 | ||
| done | ||
| echo "::error::$name did not become healthy within ${timeout}s" | ||
| docker logs "$name" 2>&1 | tail -50 || true | ||
| return 1 | ||
| } | ||
|
|
||
| wait_healthy postgres 120 | ||
| wait_healthy clickhouse 120 | ||
| wait_healthy redis 60 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Health wait ignores timeout failureMedium Severity The new Reviewed by Cursor Bugbot for commit 3715e06. Configure here. |
||
|
|
||
| # Install extensions | ||
| docker exec postgres psql -U postgres -d mydatabase -c "CREATE SCHEMA extensions; CREATE EXTENSION IF NOT EXISTS pgcrypto SCHEMA extensions;" | ||
|
|
@@ -58,60 +75,18 @@ runs: | |
| echo "TESTS_E2B_ACCESS_TOKEN=${TESTS_E2B_ACCESS_TOKEN}" >> .env.test | ||
| echo "TESTS_SANDBOX_TEAM_ID=${TESTS_SANDBOX_TEAM_ID}" >> .env.test | ||
| echo "TESTS_SANDBOX_USER_ID=${TESTS_SANDBOX_USER_ID}" >> .env.test | ||
| set -x | ||
| make migrate | ||
| make -C tests/integration seed | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ClickHouse killed between CI stepsHigh Severity The ClickHouse server is started with a shell background job in the Reviewed by Cursor Bugbot for commit ea47dab. Configure here. |
||
| shell: bash | ||
|
|
||
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@v4 | ||
|
|
||
| - name: Build codegen image with caching | ||
| uses: docker/build-push-action@v7 | ||
| with: | ||
| context: packages/clickhouse | ||
| file: packages/clickhouse/Dockerfile | ||
| tags: clickhouse-migrator:latest | ||
| load: true # makes the image available for `docker run` | ||
| cache-from: type=gha | ||
| cache-to: type=gha,mode=max | ||
|
|
||
| - name: Run Clickhouse | ||
| env: | ||
| CLICKHOUSE_USERNAME: "e2b" | ||
| CLICKHOUSE_PASSWORD: "clickity-clicky-click" | ||
| CLICKHOUSE_PORT: "9000" | ||
| CLICKHOUSE_DATABASE: "default" | ||
| run: | | ||
| echo "CLICKHOUSE_MIGRATOR_IMAGE=clickhouse-migrator" >> .env.test | ||
| echo "REDIS_URL=localhost:6379" >> .env.test | ||
| echo "CLICKHOUSE_USERNAME=${CLICKHOUSE_USERNAME}" >> .env.test | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟣 Pre-existing typo on line 118: Extended reasoning...What the bug is Line 118 of echo "CLICKHOUSE_CONNECTION_STRING=clickhouse://${CLICKHOUSE_USERNAME}:${CLICKHOUSE_PASSWORD}@$localhost:${CLICKHOUSE_PORT}/${CLICKHOUSE_DATABASE}" >> .env.testThe fragment Step-by-step proof
Why existing code doesn't prevent it\n\nThe surrounding |
||
| echo "CLICKHOUSE_PORT=${CLICKHOUSE_PORT}" >> .env.test | ||
| echo "CLICKHOUSE_PASSWORD=${CLICKHOUSE_PASSWORD}" >> .env.test | ||
| echo "CLICKHOUSE_DATABASE=${CLICKHOUSE_DATABASE}" >> .env.test | ||
| echo "CLICKHOUSE_CONNECTION_STRING=clickhouse://${CLICKHOUSE_USERNAME}:${CLICKHOUSE_PASSWORD}@$localhost:${CLICKHOUSE_PORT}/${CLICKHOUSE_DATABASE}" >> .env.test | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The connection string contains Please remove the echo "CLICKHOUSE_CONNECTION_STRING=clickhouse://${CLICKHOUSE_USERNAME}:${CLICKHOUSE_PASSWORD}@localhost:${CLICKHOUSE_PORT}/${CLICKHOUSE_DATABASE}" >> .env.test |
||
|
|
||
| make -C packages/clickhouse run & | ||
| while [ "$(docker inspect -f '{{.State.Health.Status}}' clickhouse 2>/dev/null)" != "healthy" ]; do echo "Waiting for Clickhouse to be healthy..."; sleep 2; done | ||
| echo "Clickhouse is healthy!" | ||
|
|
||
| # We build the image in separate step to cache it and avoid rebuilding it every time | ||
| make -C packages/clickhouse migrate-without-build | ||
| shell: bash | ||
|
|
||
| - name: Run Redis | ||
| run: | | ||
| docker run -d --name redis \ | ||
| -p 6379:6379 \ | ||
| --health-cmd="redis-cli ping" \ | ||
| --health-interval=5s \ | ||
| --health-timeout=2s \ | ||
| --health-retries=5 \ | ||
| redis:latest | ||
|
|
||
| while [ "$(docker inspect -f '{{.State.Health.Status}}' redis 2>/dev/null)" != "healthy" ]; do echo "Waiting for Redis to be healthy..."; sleep 2; done | ||
| echo "Redis is healthy!" | ||
| set -x | ||
| make migrate | ||
| make -C tests/integration seed | ||
|
|
||
| echo "REDIS_URL=localhost:6379" >> .env.test | ||
| make -C packages/clickhouse migrate-host | ||
| shell: bash | ||
|
|
||
| - name: Start Services | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the integration job running on the self-hosted
infra-testsrunner, thisdocker runis now backgrounded and its exit status is never observed. If a cancelled or failed prior job leaves a healthy container namedpostgres(similarlyredisbelow), the new launch fails with a name conflict but the step still succeeds; the laterwait_healthy postgresonly checks by name and can accept the stale database, so migrations/seeds/tests run against leftover state instead of a fresh container. Before this change the foregrounddocker runfailure stopped the job immediately.Useful? React with 👍 / 👎.