From c68bda8c30238d9fd3d0e7b728c838d4b62a2355 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Sat, 22 Aug 2026 11:02:12 -0500 Subject: [PATCH] ci: fold the Tests workflow into SonarQube Analysis and delete it (#123) tests.yml and sonarqube.yml had identical triggers and ran the same tests. Where tests.yml ran test:unit:json and playwright test, sonarqube.yml ran test:unit:cov and test:e2e:cov: the same two suites, differing only in reporter and instrumentation. Every pull request therefore installed dependencies twice, migrated twice, built and started the backend twice, installed a Playwright browser twice and ran the whole end-to-end suite twice. With one runner the second copy did not run in parallel, it queued. Two things in tests.yml were not duplicates and are folded in rather than dropped. The summarize steps, which render pass and fail counts and name the failing tests, where sonarqube.yml offered only raw jest and Playwright output. And the continue-on-error plus fail-at-the-end pattern that lets those summaries render on a failing run at all: sonarqube.yml used to abort at the first failing step, so a broken unit test meant no end-to-end results, no coverage and no scan, which is one fact per run when a run costs many minutes. The trade-off is fast feedback. A broken unit test used to fail tests.yml in a couple of minutes; now it is reported when the whole pipeline finishes. That is worth it with one runner, where the fast job was queued behind the slow one anyway, and where a single run reporting unit results, end-to-end results, coverage and the scan together beats two runs each reporting a fragment. Worth revisiting if the runner count changes. Two details that would each have silently broken something: The coverage scripts do not emit the JSON the summarizers read - test:unit:cov has no --json, and test:e2e:cov does not set the JSON reporter. Both are appended at the step rather than baked into package.json, since coverage and a results file are only wanted together in this one place. The Playwright step uses --reporter=list,json so the log still names the failing test rather than only writing a file. The "Backend log" step was keyed on failure(). With continue-on-error the job is not in a failed state when it runs, so failure() would never fire and the log explaining an end-to-end failure would have gone unprinted exactly when it was wanted. It is now keyed on the step outcome. The header comment also records something the old comments obscured: the integration suite runs here on every push and pull request, despite backend-integration.yml describing it as manual. That quarantine only ever applied to tests.yml. Verification, run rather than assumed: both suites executed locally with the exact commands the workflow now uses. The unit run wrote unit-results.json alongside its coverage and summarize-jest.js rendered 199 passed from it. The end-to-end run wrote playwright-results.json and summarize-playwright.js rendered 100 passed, 3 failed, 2 skipped and named all three failures - a failing run, which is the case if: always() exists for. The three failures are pre-existing and fail identically on main. Parsing the remaining workflows confirms sonarqube.yml declares one job carrying both new step ids, lint.yml and backend-integration.yml are untouched, and tests.yml is gone. The two results files are added to .gitignore. CI never committed them, but the commands are now documented and runnable locally, which makes an accidental commit a matter of time. Closes #123 Co-Authored-By: Claude Opus 5 --- .gitea/workflows/sonarqube.yml | 52 ++++++++++++- .gitea/workflows/tests.yml | 137 --------------------------------- .gitignore | 6 ++ 3 files changed, 55 insertions(+), 140 deletions(-) delete mode 100644 .gitea/workflows/tests.yml diff --git a/.gitea/workflows/sonarqube.yml b/.gitea/workflows/sonarqube.yml index 2462253..cc750a8 100755 --- a/.gitea/workflows/sonarqube.yml +++ b/.gitea/workflows/sonarqube.yml @@ -1,5 +1,17 @@ name: SonarQube Analysis +# The only workflow that runs the test suites. tests.yml used to run the unit +# and end-to-end suites as well, on identical triggers, so every pull request +# installed, migrated, built, started the backend and ran the whole end-to-end +# suite twice. With one runner the second copy did not run in parallel, it +# queued. It was deleted and its two summarize steps folded in here. See #123. +# +# Note that the integration suite DOES run here, on every push and pull request, +# despite backend-integration.yml describing it as manual. That quarantine only +# ever applied to tests.yml. It is survivable here because test:integration:cov +# passes --forceExit, which papers over the post-run hang, and because of the +# timeout below. + on: push: branches: [main] @@ -82,8 +94,13 @@ jobs: run: node migrate.js up working-directory: backend + # --json/--outputFile appended rather than baked into the script: the + # coverage run and the results file are wanted together here, and nowhere + # else. summarize-jest.js below reads that file. - name: Backend unit tests with coverage - run: npm run test:unit:cov + id: unit + continue-on-error: true + run: npm run test:unit:cov -- --json --outputFile=unit-results.json working-directory: backend # Covers everything in src/routes, which the unit suite does not touch — @@ -116,11 +133,21 @@ jobs: # Runs against an istanbul-instrumented dev server, which is what produces # window.__coverage__ for the fixture to collect. - name: Frontend end-to-end tests with coverage - run: npm run test:e2e:cov + id: e2e + continue-on-error: true + env: + PLAYWRIGHT_JSON_OUTPUT_NAME: playwright-results.json + # list as well as json, so the log still shows which test failed rather + # than only a file nobody reads until the summary step. + run: npm run test:e2e:cov -- --reporter=list,json working-directory: frontend + # Keyed off the step outcome rather than failure(). continue-on-error above + # means the job is not in a failed state at this point, so failure() would + # never fire and the log that explains an end-to-end failure would go + # unprinted precisely when it is wanted. - name: Backend log - if: failure() + if: steps.e2e.outcome == 'failure' run: cat /tmp/backend.log # Fails when nothing was collected rather than writing an empty report. An @@ -135,3 +162,22 @@ jobs: env: SONAR_HOST_URL: ${{ secrets.SONAR_HOST_URL }} SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} + + # Readable pass and fail counts, which the raw jest and Playwright output + # does not give at a glance. Both run with if: always() so a failing suite + # is still summarised — which is the case they exist for. + - name: Summarize unit tests + if: always() + run: node scripts/summarize-jest.js backend/unit-results.json "Backend Unit Test" + + - name: Summarize end-to-end tests + if: always() + run: node scripts/summarize-playwright.js frontend/playwright-results.json + + # Last, so a failing suite still produces coverage, a scan and both + # summaries first. Without this step continue-on-error above would turn a + # failing suite into a passing job, which is the one way this change could + # do real damage. + - name: Fail if either suite failed + if: steps.unit.outcome == 'failure' || steps.e2e.outcome == 'failure' + run: exit 1 diff --git a/.gitea/workflows/tests.yml b/.gitea/workflows/tests.yml deleted file mode 100644 index 67cc1db..0000000 --- a/.gitea/workflows/tests.yml +++ /dev/null @@ -1,137 +0,0 @@ -name: Tests - -# backend-integration lives in its own manual workflow -# (.gitea/workflows/backend-integration.yml) rather than running here. It held -# the runner for 3h12m on 2026-08-18 — 87 seconds of tests followed by a hang -# after the run completed — and blocked frontend-e2e behind it for the same -# three hours. Run it from Actions before merging anything that touches the API -# or the database. - -on: - push: - branches: [main] - pull_request: - types: [opened, synchronize, reopened] - workflow_dispatch: - -jobs: - backend-unit: - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Setup Node - uses: actions/setup-node@v4 - with: - node-version: '20' - - - name: Install deps - run: npm install - working-directory: backend - - - name: Run unit tests - id: unit - continue-on-error: true - run: npm run test:unit:json - working-directory: backend - - - name: Summarize - if: always() - run: node scripts/summarize-jest.js backend/unit-results.json "Backend Unit Test" - - - name: Fail job if tests failed - if: steps.unit.outcome == 'failure' - run: exit 1 - - frontend-e2e: - runs-on: ubuntu-latest - services: - postgres: - image: postgres:16 - env: - POSTGRES_USER: redefined_test - POSTGRES_PASSWORD: redefined_test - POSTGRES_DB: redefined_test - options: >- - --health-cmd "pg_isready -U redefined_test" - --health-interval 5s - --health-timeout 5s - --health-retries 10 - env: - PGHOST: postgres - PGPORT: 5432 - PGUSER: redefined_test - PGPASSWORD: redefined_test - PGDATABASE: redefined_test - PORT: 3000 - DEMO_MODE: 'true' - UPLOADS_DIR: /tmp/redefined-uploads - # NODE_ENV is deliberately unset: `npm install` omits devDependencies when - # NODE_ENV=production, which strips tsc/vite/@playwright/test and breaks the - # build. It would also flip the session cookie to Secure, which the e2e run - # serves over plain http. - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Setup Node - uses: actions/setup-node@v4 - with: - node-version: '20' - - - name: Install backend deps - run: npm install - working-directory: backend - - - name: Run migrations - run: node migrate.js up - working-directory: backend - - - name: Build backend - run: npm run build - working-directory: backend - - - name: Start backend - run: | - mkdir -p /tmp/redefined-uploads - node dist/server.js > /tmp/backend.log 2>&1 & - for i in $(seq 1 30); do - if node -e "require('http').get('http://localhost:3000/api/config', r => process.exit(r.statusCode === 200 ? 0 : 1)).on('error', () => process.exit(1))"; then - echo "Backend ready after ${i}s" - exit 0 - fi - sleep 1 - done - echo "Backend did not become ready within 30s:" - cat /tmp/backend.log - exit 1 - working-directory: backend - - - name: Install frontend deps - run: npm install - working-directory: frontend - - - name: Install Playwright browsers - run: npx playwright install --with-deps chromium - working-directory: frontend - - - name: Run Playwright tests - id: e2e - continue-on-error: true - env: - PLAYWRIGHT_JSON_OUTPUT_NAME: playwright-results.json - run: npx playwright test --reporter=json - working-directory: frontend - - - name: Backend log - if: steps.e2e.outcome == 'failure' - run: cat /tmp/backend.log - - - name: Summarize - if: always() - run: node scripts/summarize-playwright.js frontend/playwright-results.json - - - name: Fail job if tests failed - if: steps.e2e.outcome == 'failure' - run: exit 1 \ No newline at end of file diff --git a/.gitignore b/.gitignore index a7f8048..dc20b0f 100755 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,9 @@ test-results/ .superpowers/ .scannerwork/ .nyc_output/ + +# Test result JSON, written by the CI test steps for the summarize scripts and +# by anyone running the same commands locally. Regenerated every run. +backend/unit-results.json +backend/integration-results.json +frontend/playwright-results.json