Skip to content

Replace docker compose script in yarn #8541

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

Merged
merged 10 commits into from
Apr 30, 2025
8 changes: 6 additions & 2 deletions .github/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ services:
- POSTGRES_USER=${POSTGRES_USER}
- POSTGRES_PASSWORD=${POSTGRES_PASSWORD}
ports:
- "5432:5432"
- "5434:5432"
volumes:
- "../pg/db:/var/lib/postgresql/data"
- postgres_data:/var/lib/postgresql/data
healthcheck:
test: ["CMD-SHELL", "pg_isready -d webknossos -U ${POSTGRES_USER} -h 127.0.0.1 -p 5432"]
interval: 2s
Expand All @@ -111,3 +111,7 @@ services:
timeout: 1s
interval: 5s
retries: 10

volumes:
postgres_data:
external: false
2 changes: 1 addition & 1 deletion .github/workflows/build_test_deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ on:
env:
USER_UID: 1001
USER_GID: 1001
POSTGRES_URL: "jdbc:postgresql://localhost:5432/webknossos"
POSTGRES_URL: "jdbc:postgresql://localhost:5434/webknossos"
POSTGRES_USER: "webknossos_user"
POSTGRES_PASSWORD: "secret_password"
CERTIFICATE: ${{ secrets.CERTIFICATE_FOR_CI_ONLY }}
Expand Down
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,13 @@
"test-e2e": "vitest run --fileParallelism=false frontend/javascripts/test/**/*.e2e.ts",
"test-screenshot": "vitest run --testTimeout=300000 --hookTimeout=20000 frontend/javascripts/test/puppeteer/**/*.screenshot.ts",
"test-wkorg-screenshot": "vitest run --testTimeout=300000 --hookTimeout=20000 frontend/javascripts/test/puppeteer/**/*.wkorg_screenshot.ts",
"refresh-screenshots": "rm -rf frontend/javascripts/test/screenshots/** && docker compose up screenshot-tests",
"refresh-screenshots": "tools/refresh-screenshots.sh",
"test-help": "echo For development it is recommended to run yarn test-prepare-watch in one terminal and yarn test-watch in another. This is the fastest way to perform incremental testing. If you are only interested in running test once, use yarn test.",
"remove-e2e-snapshots": "rm -rf frontend/javascripts/test/snapshots/public-test/test-bundle/test/backend-snapshot-tests/ && rm -rf frontend/javascripts/test/snapshots/public-test/test-bundle/test/enzyme/",
"remove-e2e-snapshots": "bash -c 'rm -rf frontend/javascripts/test/backend-snapshot-tests/__snapshots__/'",
"remove-all-snapshots": "rm -rf frontend/javascripts/test/snapshots/",
"stash-cmake-cache": "[ -f webknossos-jni/target/native/x86_64-linux/build/CMakeCache.txt ] && mv webknossos-jni/target/native/x86_64-linux/build/CMakeCache.txt webknossos-jni/target/native/x86_64-linux/build/CMakeCache.txt.bak || true",
"stash-pop-cmake-cache": "[ -f webknossos-jni/target/native/x86_64-linux/build/CMakeCache.txt.bak ] && mv webknossos-jni/target/native/x86_64-linux/build/CMakeCache.txt.bak webknossos-jni/target/native/x86_64-linux/build/CMakeCache.txt || true",
"refresh-e2e-snapshots": "yarn remove-e2e-snapshots && mkdir -p frontend/javascripts/test/snapshots/type-check && yarn stash-cmake-cache && docker compose down && docker compose up e2e-tests && yarn stash-pop-cmake-cache",
"refresh-e2e-snapshots": "yarn remove-e2e-snapshots && mkdir -p frontend/javascripts/test/snapshots/type-check && yarn stash-cmake-cache && ./tools/docker-env.sh sbt -v 'testOnly e2e.* -- -Ddatastore.watchFileSystem.enabled=false' && yarn stash-pop-cmake-cache",
"refresh-all-snapshots": "yarn remove-all-snapshots && yarn test && yarn refresh-e2e-snapshots",
"check-frontend": "yarn run biome check frontend package.json && tools/assert-no-test-only.sh",
"fix-frontend": "yarn run biome check frontend package.json --write && echo Please proofread the applied suggestions, as they may not be safe.",
Expand Down
20 changes: 20 additions & 0 deletions tools/docker-env.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/usr/bin/env bash

# Start Docker
export POSTGRES_URL=jdbc:postgresql://localhost:5434/webknossos
export POSTGRES_USER="webknossos"
export POSTGRES_PASSWORD="secret"

cd .github || exit
docker compose down -v
docker compose up -d postgres fossildb
cd .. || exit

"$@"
RESULT=$?

cd .github || exit
docker compose down -v
cd .. || exit

exit "$RESULT"
16 changes: 16 additions & 0 deletions tools/refresh-screenshots.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#!/usr/bin/env bash

set -xe
Comment on lines +1 to +3
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Enhance shell strictness for safer execution
Currently the script uses set -xe, but adding stricter options can catch more classes of failures and undefined variables:

-set -xe
+set -euxo pipefail
  • -u: error on unset variables
  • -o pipefail: propagate failures in pipelines
  • Keeps -x (trace) and -e (exit on error)

This hardens the script and surfaces issues early.


🏁 Script executed:

#!/bin/bash
sed -n '1,200p' tools/refresh-screenshots.sh

Length of output: 398


Enhance shell strictness for safer execution in tools/refresh-screenshots.sh
In the shebang section, replace:

- set -xe
+ set -euxo pipefail
  • -u: error on unset variables (e.g. $URL, $WK_AUTH_TOKEN)
  • -o pipefail: catch failures in any pipeline
  • retains -x (command tracing) and -e (exit on error)

This hardens the script and surfaces issues early.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#!/usr/bin/env bash
set -xe
#!/usr/bin/env bash
set -euxo pipefail


rm -rf frontend/javascripts/test/screenshots/**
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fix globbing to reliably clear screenshots
The pattern ** requires shopt -s globstar (not enabled by default), so it may not remove nested files. A simpler, reliable approach is:

-rm -rf frontend/javascripts/test/screenshots/**
+rm -rf frontend/javascripts/test/screenshots/*

Or explicitly enable recursive globbing at the top:

shopt -s globstar

Either ensures the directory is properly emptied.


docker run --rm \
--pull "always" \
--env "URL=$URL" \
--env "WK_AUTH_TOKEN=$WK_AUTH_TOKEN" \
--env "BROWSERSTACK_USERNAME=$BROWSERSTACK_USERNAME" \
--env "BROWSERSTACK_ACCESS_KEY=$BROWSERSTACK_ACCESS_KEY" \
--workdir "/home/pptruser/webknossos" \
--volume ".:/home/pptruser/webknossos" \
--user "$(id -u):$(id -g)" \
scalableminds/puppeteer:master bash -c 'for i in {1..3}; do yarn test-screenshot && break; done'