devops: add Playwright E2E job to CI for stammbaum spec #363

Open
opened 2026-04-28 12:57:45 +02:00 by marcel · 8 comments
Owner

Context

frontend/e2e/stammbaum.spec.ts was committed in PR #360 but cannot run in CI because the Gitea Actions runner does not have Playwright Chromium binaries installed and the workflow does not start the full Docker Compose stack.

The spec is currently marked .skip() with a reference to this issue. Once this issue is resolved the skip annotations must be removed.

What needs to happen

  1. Add a Playwright job to .gitea/workflows/ci.yml that:
    • Starts the full Docker Compose stack (docker-compose up -d)
    • Waits for the backend to be healthy (curl actuator/health)
    • Runs npx playwright install chromium --with-deps
    • Runs npm run test:e2e (or npx playwright test e2e/stammbaum.spec.ts scoped to the new spec)
  2. Remove the test.skip() annotations from frontend/e2e/stammbaum.spec.ts

Reference

frontend/e2e/CLAUDE.md already documents the intent:

E2E tests are not currently run in CI (the pipeline stops at unit/component tests). To add them, extend infra/gitea/workflows/ci.yml with a Playwright job that starts the full Docker Compose stack first.

## Context `frontend/e2e/stammbaum.spec.ts` was committed in PR #360 but cannot run in CI because the Gitea Actions runner does not have Playwright Chromium binaries installed and the workflow does not start the full Docker Compose stack. The spec is currently marked `.skip()` with a reference to this issue. Once this issue is resolved the skip annotations must be removed. ## What needs to happen 1. Add a Playwright job to `.gitea/workflows/ci.yml` that: - Starts the full Docker Compose stack (`docker-compose up -d`) - Waits for the backend to be healthy (`curl actuator/health`) - Runs `npx playwright install chromium --with-deps` - Runs `npm run test:e2e` (or `npx playwright test e2e/stammbaum.spec.ts` scoped to the new spec) 2. Remove the `test.skip()` annotations from `frontend/e2e/stammbaum.spec.ts` ## Reference `frontend/e2e/CLAUDE.md` already documents the intent: > E2E tests are **not** currently run in CI (the pipeline stops at unit/component tests). To add them, extend `infra/gitea/workflows/ci.yml` with a Playwright job that starts the full Docker Compose stack first.
marcel added the P2-mediumdevopstest labels 2026-04-28 12:57:49 +02:00
Author
Owner

🏗️ Markus Keller — Application Architect

Observations

  • A docs/infrastructure/ci-gitea.md already exists with a complete, well-structured E2E job YAML. The issue is essentially asking to copy that spec into the live ci.yml. The design work is done — this is an execution task.
  • The docker-compose.ci.yml overlay already exists and is referenced correctly in the doc (docker compose -f docker-compose.yml -f docker-compose.ci.yml up -d db minio create-buckets). The overlay correctly swaps bind mounts for named volumes (ci_postgres_data, ci_minio_data). This is the right pattern.
  • The issue body says "Starts the full Docker Compose stack (docker-compose up -d)" but the documented approach in ci-gitea.md correctly starts only db minio create-buckets, then boots the backend as a JAR directly. This is architecturally cleaner: the backend JAR is already compiled by the preceding Maven step, and running it directly avoids Docker image build overhead in CI.
  • The OCR service is not listed in depends_on in the CI doc's approach, and the stammbaum.spec.ts tests do not call any OCR endpoints. Omitting OCR is correct — it requires 12 GB RAM, a 120-second start period, and model files. Including it in CI would be prohibitive on the NAS runner.
  • The backend service's depends_on in docker-compose.yml includes ocr-service: condition: service_started. This means a naive docker compose up -d would attempt to start the OCR service too. The selective up -d db minio create-buckets approach in the doc avoids this correctly.
  • minio/minio:latest and minio/mc are unpinned in docker-compose.yml. This is a pre-existing concern that doesn't block this issue but is the kind of drift that causes non-reproducible CI failures over time. Worth a separate ticket.
  • The doc references --APP_ADMIN_USERNAME=admin and --APP_ADMIN_PASSWORD=${{ secrets.E2E_ADMIN_PASSWORD }} on the JVM command line. Spring Boot 4 maps --KEY=VALUE CLI args correctly to environment properties, so this is valid.

Recommendations

  • Follow the ci-gitea.md design exactly. Do not start the full Compose stack — start only db minio create-buckets, then run the backend JAR directly with environment overrides. This keeps CI under 10 minutes on NAS hardware.
  • Create a Gitea secret E2E_ADMIN_PASSWORD before the CI job runs. The UserDataInitializer seeds APP_ADMIN_PASSWORD from the environment — the secret just needs to match what auth.setup.ts uses at test time.
  • Scope the E2E job's needs: to both unit-tests and backend-unit-tests so it only runs after those pass. This keeps the pipeline fail-fast without redundant work.
  • Open a separate issue for image pinning (minio/minio:latest → pinned tag). Do not bundle that fix into this PR — it's unrelated and risks blocking the E2E work.

Open Decisions

  • Should the E2E job run on every push or only on PRs? The current on: push / pull_request means it runs on every branch push. Given CI takes longer with E2E, consider restricting to pull_request or adding a path filter to skip it when only docs change. This is a product workflow preference, not a technical constraint.
## 🏗️ Markus Keller — Application Architect ### Observations - A `docs/infrastructure/ci-gitea.md` already exists with a complete, well-structured E2E job YAML. The issue is essentially asking to copy that spec into the live `ci.yml`. The design work is done — this is an execution task. - The `docker-compose.ci.yml` overlay already exists and is referenced correctly in the doc (`docker compose -f docker-compose.yml -f docker-compose.ci.yml up -d db minio create-buckets`). The overlay correctly swaps bind mounts for named volumes (`ci_postgres_data`, `ci_minio_data`). This is the right pattern. - The issue body says "Starts the full Docker Compose stack (`docker-compose up -d`)" but the documented approach in `ci-gitea.md` correctly starts only `db minio create-buckets`, then boots the backend as a JAR directly. This is architecturally cleaner: the backend JAR is already compiled by the preceding Maven step, and running it directly avoids Docker image build overhead in CI. - The OCR service is **not listed in `depends_on`** in the CI doc's approach, and the `stammbaum.spec.ts` tests do not call any OCR endpoints. Omitting OCR is correct — it requires 12 GB RAM, a 120-second start period, and model files. Including it in CI would be prohibitive on the NAS runner. - The `backend` service's `depends_on` in `docker-compose.yml` includes `ocr-service: condition: service_started`. This means a naive `docker compose up -d` would attempt to start the OCR service too. The selective `up -d db minio create-buckets` approach in the doc avoids this correctly. - `minio/minio:latest` and `minio/mc` are unpinned in `docker-compose.yml`. This is a pre-existing concern that doesn't block this issue but is the kind of drift that causes non-reproducible CI failures over time. Worth a separate ticket. - The doc references `--APP_ADMIN_USERNAME=admin` and `--APP_ADMIN_PASSWORD=${{ secrets.E2E_ADMIN_PASSWORD }}` on the JVM command line. Spring Boot 4 maps `--KEY=VALUE` CLI args correctly to environment properties, so this is valid. ### Recommendations - **Follow the `ci-gitea.md` design exactly.** Do not start the full Compose stack — start only `db minio create-buckets`, then run the backend JAR directly with environment overrides. This keeps CI under 10 minutes on NAS hardware. - **Create a Gitea secret `E2E_ADMIN_PASSWORD`** before the CI job runs. The `UserDataInitializer` seeds `APP_ADMIN_PASSWORD` from the environment — the secret just needs to match what `auth.setup.ts` uses at test time. - **Scope the E2E job's `needs:`** to both `unit-tests` and `backend-unit-tests` so it only runs after those pass. This keeps the pipeline fail-fast without redundant work. - **Open a separate issue for image pinning** (`minio/minio:latest` → pinned tag). Do not bundle that fix into this PR — it's unrelated and risks blocking the E2E work. ### Open Decisions - **Should the E2E job run on every push or only on PRs?** The current `on: push / pull_request` means it runs on every branch push. Given CI takes longer with E2E, consider restricting to `pull_request` or adding a path filter to skip it when only docs change. This is a product workflow preference, not a technical constraint.
Author
Owner

👨‍💻 Felix Brandt — Fullstack Developer

Observations

  • The stammbaum.spec.ts file uses test.describe('...', () => { test.skip(); ... }) at the describe-block level. This skips all tests in the block unconditionally. Once CI is working, removing test.skip() will immediately enable all four tests — no other code changes are needed in the spec file.
  • The four tests are well-structured and cover the right behaviors: route preservation (/briefwechsel), page heading render, empty-vs-populated state, and the year-range validation error. The soft assertion pattern in the third test (expect(emptyVisible || nodeCount > 0).toBe(true)) is intentional and acceptable — it handles two coherent states without coupling the test to data.
  • The auth.setup.ts file reads credentials from E2E_USERNAME and E2E_PASSWORD env vars with fallback defaults (admin@familyarchive.local / admin123). In CI the password default is admin123 — the same value that UserDataInitializer seeds when APP_ADMIN_PASSWORD is not set. This means the E2E job works even without a Gitea secret configured, though using a hardcoded default password in CI is a smell (see Nora's comment).
  • The playwright.config.ts webServer block starts npm run dev -- --port 3000 and uses reuseExistingServer: true. In CI there will be no pre-running server, so Playwright will start the SvelteKit dev server itself. This requires node_modules to be installed before the test step — the CI doc correctly handles this with a prior npm ci step and cache.
  • The test:e2e script is playwright test — it runs all specs in ./e2e, not just stammbaum.spec.ts. Once the skip is removed, all E2E specs run. This is fine if all other specs pass; if they are flaky on the NAS runner, it may make sense to run stammbaum.spec.ts in isolation first to validate the job works before enabling the full suite.

Recommendations

  • Remove test.skip() in the same PR that adds the CI job. The two are tightly coupled — the skip exists only because CI wasn't ready. Leaving the skip in a follow-up creates a separate tracking obligation with no benefit.
  • Run npx playwright test e2e/stammbaum.spec.ts first during development of the CI job, then switch to npm run test:e2e once the job is confirmed stable. This shortens the feedback loop significantly on the NAS runner.
  • Add --project=chromium to the npm run test:e2e call in the CI YAML to be explicit. The config only defines Chromium, so this is defensive but makes intent obvious and guards against someone adding a Firefox project later without CI budget for it.
  • Verify the webServer auto-start works with API_INTERNAL_URL and API_PROXY_TARGET set. In Compose, these are set via Docker env. When Playwright starts the dev server via the webServer block, those env vars need to be available in the CI job environment, or the Vite proxy won't know where to forward /api calls. Set API_INTERNAL_URL=http://localhost:8080 and API_PROXY_TARGET=http://localhost:8080 in the E2E job's env: block.

Open Decisions

  • Full E2E suite or just stammbaum.spec.ts in this issue? The issue title says "for stammbaum spec" but the CI job will run all E2E tests. If other specs are fragile on the NAS runner, this PR will be blocked until they are fixed. Scoping to just the stammbaum spec initially is safer and keeps this issue atomic.
## 👨‍💻 Felix Brandt — Fullstack Developer ### Observations - The `stammbaum.spec.ts` file uses `test.describe('...', () => { test.skip(); ... })` at the describe-block level. This skips all tests in the block unconditionally. Once CI is working, removing `test.skip()` will immediately enable all four tests — no other code changes are needed in the spec file. - The four tests are well-structured and cover the right behaviors: route preservation (`/briefwechsel`), page heading render, empty-vs-populated state, and the year-range validation error. The soft assertion pattern in the third test (`expect(emptyVisible || nodeCount > 0).toBe(true)`) is intentional and acceptable — it handles two coherent states without coupling the test to data. - The `auth.setup.ts` file reads credentials from `E2E_USERNAME` and `E2E_PASSWORD` env vars with fallback defaults (`admin@familyarchive.local` / `admin123`). In CI the password default is `admin123` — the same value that `UserDataInitializer` seeds when `APP_ADMIN_PASSWORD` is not set. This means the E2E job works even without a Gitea secret configured, though using a hardcoded default password in CI is a smell (see Nora's comment). - The `playwright.config.ts` `webServer` block starts `npm run dev -- --port 3000` and uses `reuseExistingServer: true`. In CI there will be no pre-running server, so Playwright will start the SvelteKit dev server itself. This requires `node_modules` to be installed before the test step — the CI doc correctly handles this with a prior `npm ci` step and cache. - The `test:e2e` script is `playwright test` — it runs all specs in `./e2e`, not just `stammbaum.spec.ts`. Once the skip is removed, all E2E specs run. This is fine if all other specs pass; if they are flaky on the NAS runner, it may make sense to run `stammbaum.spec.ts` in isolation first to validate the job works before enabling the full suite. ### Recommendations - **Remove `test.skip()` in the same PR that adds the CI job.** The two are tightly coupled — the skip exists only because CI wasn't ready. Leaving the skip in a follow-up creates a separate tracking obligation with no benefit. - **Run `npx playwright test e2e/stammbaum.spec.ts` first during development of the CI job**, then switch to `npm run test:e2e` once the job is confirmed stable. This shortens the feedback loop significantly on the NAS runner. - **Add `--project=chromium` to the `npm run test:e2e` call in the CI YAML** to be explicit. The config only defines Chromium, so this is defensive but makes intent obvious and guards against someone adding a Firefox project later without CI budget for it. - **Verify the `webServer` auto-start works with `API_INTERNAL_URL` and `API_PROXY_TARGET` set.** In Compose, these are set via Docker env. When Playwright starts the dev server via the `webServer` block, those env vars need to be available in the CI job environment, or the Vite proxy won't know where to forward `/api` calls. Set `API_INTERNAL_URL=http://localhost:8080` and `API_PROXY_TARGET=http://localhost:8080` in the E2E job's `env:` block. ### Open Decisions - **Full E2E suite or just `stammbaum.spec.ts` in this issue?** The issue title says "for stammbaum spec" but the CI job will run all E2E tests. If other specs are fragile on the NAS runner, this PR will be blocked until they are fixed. Scoping to just the stammbaum spec initially is safer and keeps this issue atomic.
Author
Owner

🔧 Tobias Wendt — DevOps & Platform Engineer

Observations

  • The docs/infrastructure/ci-gitea.md already contains a complete, tested E2E job YAML. This is good — the infra design is done. The issue is purely about copy-paste + wiring.
  • The docker-compose.ci.yml overlay correctly replaces the bind-mount volumes (./data/postgres, ./data/minio) from the base Compose file with named volumes (ci_postgres_data, ci_minio_data). Named volumes in CI are ephemeral per-run if the runner cleans up between jobs. The CI doc includes a docker compose down --volumes --remove-orphans || true cleanup step at the start, which handles leftover state from crashed previous runs correctly.
  • The doc's E2E job does not start the OCR service — it starts only db minio create-buckets. This is the right call for the NAS runner. The OCR service needs 12 GB RAM, a 120-second health start period, and model files. Skipping it keeps the E2E job feasible on the hardware.
  • minio/minio:latest is unpinned in docker-compose.yml. This is a pre-existing issue but directly affects CI reproducibility. A MinIO breaking change could silently break CI at any push without explanation.
  • axllent/mailpit:latest is also unpinned. Same concern, lower risk since E2E tests don't appear to exercise email flows heavily.
  • The CI doc's Start backend step connects the job container to the Compose network with docker network connect familienarchiv_archive-net $(cat /etc/hostname). This is the correct technique for a non-containerized job step to reach services started via Compose. The network name familienarchiv_archive-net is derived from the project directory name — this is fragile if the repo is checked out under a different directory name on the runner.
  • The Playwright browser cache step in the doc uses:
    key: playwright-chromium-${{ hashFiles('frontend/package-lock.json') }}
    
    with a conditional install-deps chromium if cache hit. This is the correct pattern — full install on miss, system deps only on hit. Saves 60–90 seconds per run.
  • The backend-unit-tests job already sets DOCKER_API_VERSION: "1.43" for the NAS runner. The E2E job should set the same env var since it also runs Docker commands.
  • The issue references curl actuator/health for the health wait. The doc uses wget -qO- which is better for Alpine-based containers. The actual YAML uses curl -sf http://localhost:8080/actuator/health | grep -q "UP" which is fine for a non-containerized step.

Recommendations

  • Copy the E2E job YAML from ci-gitea.md into ci.yml verbatim. Do not rewrite it — the doc was written with the NAS runner's constraints in mind. The $(cat /etc/hostname) network connect trick and the selective db minio create-buckets start are both load-bearing.
  • Add DOCKER_API_VERSION: "1.43" to the E2E job's env: block. This is already in backend-unit-tests and is required for the NAS runner's Docker 24.x host.
  • Pin minio/minio and minio/mc to specific digest or tag. Open a separate issue immediately after this one lands. RELEASE.2025-01-20T14-49-07Z or equivalent — check the MinIO releases page for the current stable tag.
  • Hardcode the Compose project name with -p familienarchiv on every docker compose call in the E2E job, or set COMPOSE_PROJECT_NAME=familienarchiv in the job's env. This makes the network name deterministic regardless of checkout path.
  • Create E2E_ADMIN_PASSWORD as a Gitea Actions secret before merging. The UserDataInitializer seeds admin with APP_ADMIN_PASSWORD from the environment — it works with admin123 as default, but using a secret is the correct posture and costs nothing.
  • Set needs: [unit-tests, backend-unit-tests] on the E2E job. This prevents wasted Docker Compose start time when earlier jobs fail.

Open Decisions

  • Should E2E artifacts (screenshots, traces, videos) be uploaded on success or only on failure? Currently the doc says if: always(), which uploads on every run. On a passing suite this generates storage churn with no diagnostic value. if: failure() is cheaper but means you can't see screenshots from green runs. This is a workflow preference call.
## 🔧 Tobias Wendt — DevOps & Platform Engineer ### Observations - The `docs/infrastructure/ci-gitea.md` already contains a complete, tested E2E job YAML. This is good — the infra design is done. The issue is purely about copy-paste + wiring. - The `docker-compose.ci.yml` overlay correctly replaces the bind-mount volumes (`./data/postgres`, `./data/minio`) from the base Compose file with named volumes (`ci_postgres_data`, `ci_minio_data`). Named volumes in CI are ephemeral per-run if the runner cleans up between jobs. The CI doc includes a `docker compose down --volumes --remove-orphans || true` cleanup step at the start, which handles leftover state from crashed previous runs correctly. - The doc's E2E job **does not start the OCR service** — it starts only `db minio create-buckets`. This is the right call for the NAS runner. The OCR service needs 12 GB RAM, a 120-second health start period, and model files. Skipping it keeps the E2E job feasible on the hardware. - **`minio/minio:latest` is unpinned** in `docker-compose.yml`. This is a pre-existing issue but directly affects CI reproducibility. A MinIO breaking change could silently break CI at any push without explanation. - **`axllent/mailpit:latest` is also unpinned.** Same concern, lower risk since E2E tests don't appear to exercise email flows heavily. - The CI doc's `Start backend` step connects the job container to the Compose network with `docker network connect familienarchiv_archive-net $(cat /etc/hostname)`. This is the correct technique for a non-containerized job step to reach services started via Compose. The network name `familienarchiv_archive-net` is derived from the project directory name — this is fragile if the repo is checked out under a different directory name on the runner. - The Playwright browser cache step in the doc uses: ```yaml key: playwright-chromium-${{ hashFiles('frontend/package-lock.json') }} ``` with a conditional `install-deps chromium` if cache hit. This is the correct pattern — full install on miss, system deps only on hit. Saves 60–90 seconds per run. - The `backend-unit-tests` job already sets `DOCKER_API_VERSION: "1.43"` for the NAS runner. The E2E job should set the same env var since it also runs Docker commands. - The issue references `curl actuator/health` for the health wait. The doc uses `wget -qO-` which is better for Alpine-based containers. The actual YAML uses `curl -sf http://localhost:8080/actuator/health | grep -q "UP"` which is fine for a non-containerized step. ### Recommendations - **Copy the E2E job YAML from `ci-gitea.md` into `ci.yml` verbatim.** Do not rewrite it — the doc was written with the NAS runner's constraints in mind. The `$(cat /etc/hostname)` network connect trick and the selective `db minio create-buckets` start are both load-bearing. - **Add `DOCKER_API_VERSION: "1.43"` to the E2E job's `env:` block.** This is already in `backend-unit-tests` and is required for the NAS runner's Docker 24.x host. - **Pin `minio/minio` and `minio/mc` to specific digest or tag.** Open a separate issue immediately after this one lands. `RELEASE.2025-01-20T14-49-07Z` or equivalent — check the MinIO releases page for the current stable tag. - **Hardcode the Compose project name** with `-p familienarchiv` on every `docker compose` call in the E2E job, or set `COMPOSE_PROJECT_NAME=familienarchiv` in the job's env. This makes the network name deterministic regardless of checkout path. - **Create `E2E_ADMIN_PASSWORD` as a Gitea Actions secret** before merging. The `UserDataInitializer` seeds admin with `APP_ADMIN_PASSWORD` from the environment — it works with `admin123` as default, but using a secret is the correct posture and costs nothing. - **Set `needs: [unit-tests, backend-unit-tests]`** on the E2E job. This prevents wasted Docker Compose start time when earlier jobs fail. ### Open Decisions - **Should E2E artifacts (screenshots, traces, videos) be uploaded on success or only on failure?** Currently the doc says `if: always()`, which uploads on every run. On a passing suite this generates storage churn with no diagnostic value. `if: failure()` is cheaper but means you can't see screenshots from green runs. This is a workflow preference call.
Author
Owner

📋 Elicit — Requirements Engineer

Observations

  • The issue is well-scoped and has a clear "done" condition: CI job added, test.skip() removed, tests green. This passes the INVEST test for "Small" and "Testable."
  • The issue body says the reference for how to wire up the E2E job is in frontend/e2e/CLAUDE.md, but the actual implementation blueprint is in docs/infrastructure/ci-gitea.md. The CLAUDE.md reference is directional ("extend infra/gitea/workflows/ci.yml"), not prescriptive. The issue could have been clearer about where the authoritative design lives.
  • Missing acceptance criterion: which tests must pass green? The issue says "remove the test.skip() annotations" but does not specify the expected outcome. A definition of done that includes "all four tests in stammbaum.spec.ts pass green in CI on at least one successful run" would make closure unambiguous.
  • Missing prerequisite: Gitea secret. The CI job requires E2E_ADMIN_PASSWORD to be set as a Gitea Actions secret. If not created before the first CI run, the secret resolves to empty string, auth.setup.ts falls back to the hardcoded default admin123, and the tests may still pass — but this is accidental success, not correct configuration. The issue should list "Create E2E_ADMIN_PASSWORD secret in Gitea" as an explicit task.
  • Missing task: API_INTERNAL_URL / API_PROXY_TARGET env vars for the Playwright webServer. The SvelteKit dev server needs to know where the backend is. In Docker Compose these come from the container environment. When Playwright starts the dev server directly (which the webServer block does), those vars must be passed explicitly in the CI job's env: block. This is an omission in the issue spec.
  • The label set (P2-medium, devops, test) is appropriate. P2 is correct given the tests are already skipped with a ticket reference — the app works, there is just no CI coverage for this feature.

Recommendations

  • Add a "Definition of Done" section to the issue with three explicit criteria: (1) ci.yml has a passing e2e-tests job, (2) stammbaum.spec.ts has no test.skip(), (3) the job passes green on the branch CI run before merge.
  • Add an explicit task for the Gitea secret. Even one sentence: "Before merging: create E2E_ADMIN_PASSWORD in Gitea → Settings → Actions → Secrets."
  • Add an explicit task for API_INTERNAL_URL=http://localhost:8080 in the E2E job env. Without this, the SvelteKit dev server started by Playwright's webServer block will have no target for its /api proxy and all API calls will 502.
  • Close the ambiguity in "full stack vs. selective." The issue says "starts the full Docker Compose stack" but the correct approach (from the infra doc) is selective. Update the issue body or add a comment to prevent the implementer from running docker compose up -d naively, which would attempt to start the OCR service and likely exhaust RAM on the NAS runner.
## 📋 Elicit — Requirements Engineer ### Observations - The issue is well-scoped and has a clear "done" condition: CI job added, `test.skip()` removed, tests green. This passes the INVEST test for "Small" and "Testable." - The issue body says the reference for how to wire up the E2E job is in `frontend/e2e/CLAUDE.md`, but the actual implementation blueprint is in `docs/infrastructure/ci-gitea.md`. The CLAUDE.md reference is directional ("extend `infra/gitea/workflows/ci.yml`"), not prescriptive. The issue could have been clearer about where the authoritative design lives. - **Missing acceptance criterion: which tests must pass green?** The issue says "remove the `test.skip()` annotations" but does not specify the expected outcome. A definition of done that includes "all four tests in `stammbaum.spec.ts` pass green in CI on at least one successful run" would make closure unambiguous. - **Missing prerequisite: Gitea secret.** The CI job requires `E2E_ADMIN_PASSWORD` to be set as a Gitea Actions secret. If not created before the first CI run, the secret resolves to empty string, `auth.setup.ts` falls back to the hardcoded default `admin123`, and the tests may still pass — but this is accidental success, not correct configuration. The issue should list "Create `E2E_ADMIN_PASSWORD` secret in Gitea" as an explicit task. - **Missing task: `API_INTERNAL_URL` / `API_PROXY_TARGET` env vars for the Playwright webServer.** The SvelteKit dev server needs to know where the backend is. In Docker Compose these come from the container environment. When Playwright starts the dev server directly (which the `webServer` block does), those vars must be passed explicitly in the CI job's `env:` block. This is an omission in the issue spec. - The label set (P2-medium, devops, test) is appropriate. P2 is correct given the tests are already skipped with a ticket reference — the app works, there is just no CI coverage for this feature. ### Recommendations - **Add a "Definition of Done" section to the issue** with three explicit criteria: (1) `ci.yml` has a passing `e2e-tests` job, (2) `stammbaum.spec.ts` has no `test.skip()`, (3) the job passes green on the branch CI run before merge. - **Add an explicit task for the Gitea secret.** Even one sentence: "Before merging: create `E2E_ADMIN_PASSWORD` in Gitea → Settings → Actions → Secrets." - **Add an explicit task for `API_INTERNAL_URL=http://localhost:8080`** in the E2E job env. Without this, the SvelteKit dev server started by Playwright's `webServer` block will have no target for its `/api` proxy and all API calls will 502. - **Close the ambiguity in "full stack vs. selective."** The issue says "starts the full Docker Compose stack" but the correct approach (from the infra doc) is selective. Update the issue body or add a comment to prevent the implementer from running `docker compose up -d` naively, which would attempt to start the OCR service and likely exhaust RAM on the NAS runner.
Author
Owner

🔒 Nora "NullX" Steiner — Security Engineer

Observations

  • Hardcoded fallback credentials in auth.setup.ts: admin@familyarchive.local / admin123 are hardcoded as defaults in process.env.E2E_USERNAME ?? 'admin@familyarchive.local'. If the E2E_PASSWORD Gitea secret is not set, auth.setup.ts silently uses admin123 — the same value the UserDataInitializer seeds by default. This means tests pass without any secret configured, which creates false confidence that the secret discipline is in place.
  • admin123 also appears as the default APP_ADMIN_PASSWORD in application.yaml: password: ${APP_ADMIN_PASSWORD:admin123}. If the backend is launched in CI without --APP_ADMIN_PASSWORD=<secret>, the admin account has a known, trivially guessable password. In CI this is an ephemeral environment, so the practical risk is low — but a CI runner accessible from the network during a job run would expose the backend with a known credential.
  • The CI doc's YAML passes --APP_ADMIN_USERNAME=admin on the JVM command line. CLI arguments in process lists (ps aux, container inspect) are visible to any process running as the same user. On a shared NAS runner, this could expose the admin username. The password is passed via Gitea secret, which is correct.
  • The AuthE2EController (@Profile("e2e")) exposes test-only endpoints that bypass normal auth flows. The @Profile("e2e") guard is the correct mechanism to gate these, and SecurityConfig must allow them. As long as SPRING_PROFILES_ACTIVE=e2e is never set in production, this is safe. Confirm that the production Compose file (if it exists) does not include the e2e profile — the current docker-compose.yml shows SPRING_PROFILES_ACTIVE: dev,e2e, which is fine for local dev but should not appear in any production overlay.
  • No COMPOSE_PROJECT_NAME isolation in the CI job. If two CI runs overlap on the same NAS runner (which can happen on parallel branch pushes), the docker compose down --volumes cleanup at the start of the second run will kill the first run's containers. This is not a security issue per se, but it means concurrent CI runs will corrupt each other — worth flagging to Tobias.

Recommendations

  • Create the E2E_ADMIN_PASSWORD Gitea secret before the first run and set it to something that is not admin123. The E2E profile is ephemeral, but the discipline of never accepting a known default password in any CI pipeline is worth maintaining. Any 16-char random string suffices.
  • Pass APP_ADMIN_PASSWORD via environment variable, not CLI flag. Replace --APP_ADMIN_PASSWORD=${{ secrets.E2E_ADMIN_PASSWORD }} in the java -jar call with an explicit env: block entry: APP_ADMIN_PASSWORD: ${{ secrets.E2E_ADMIN_PASSWORD }}. Environment variables are less visible than CLI args in process listings.
  • Verify SPRING_PROFILES_ACTIVE does not include e2e in any production configuration. Check docker-compose.prod.yml (if it exists) and the production .env. The e2e profile enables AuthE2EController which exposes password reset tokens in plaintext — this must never reach a real deployment.
  • Remove the admin123 default from application.yaml. Replace password: ${APP_ADMIN_PASSWORD:admin123} with password: ${APP_ADMIN_PASSWORD} (no default). This forces the env var to be explicitly set in every environment, making a misconfiguration loud rather than silently falling back to a known password.
## 🔒 Nora "NullX" Steiner — Security Engineer ### Observations - **Hardcoded fallback credentials in `auth.setup.ts`:** `admin@familyarchive.local` / `admin123` are hardcoded as defaults in `process.env.E2E_USERNAME ?? 'admin@familyarchive.local'`. If the `E2E_PASSWORD` Gitea secret is not set, `auth.setup.ts` silently uses `admin123` — the same value the `UserDataInitializer` seeds by default. This means tests pass without any secret configured, which creates false confidence that the secret discipline is in place. - **`admin123` also appears as the default `APP_ADMIN_PASSWORD` in `application.yaml`:** `password: ${APP_ADMIN_PASSWORD:admin123}`. If the backend is launched in CI without `--APP_ADMIN_PASSWORD=<secret>`, the admin account has a known, trivially guessable password. In CI this is an ephemeral environment, so the practical risk is low — but a CI runner accessible from the network during a job run would expose the backend with a known credential. - **The CI doc's YAML passes `--APP_ADMIN_USERNAME=admin` on the JVM command line.** CLI arguments in process lists (`ps aux`, container inspect) are visible to any process running as the same user. On a shared NAS runner, this could expose the admin username. The password is passed via Gitea secret, which is correct. - **The `AuthE2EController` (`@Profile("e2e")`) exposes test-only endpoints that bypass normal auth flows.** The `@Profile("e2e")` guard is the correct mechanism to gate these, and `SecurityConfig` must allow them. As long as `SPRING_PROFILES_ACTIVE=e2e` is never set in production, this is safe. Confirm that the production Compose file (if it exists) does not include the `e2e` profile — the current `docker-compose.yml` shows `SPRING_PROFILES_ACTIVE: dev,e2e`, which is fine for local dev but should not appear in any production overlay. - **No `COMPOSE_PROJECT_NAME` isolation in the CI job.** If two CI runs overlap on the same NAS runner (which can happen on parallel branch pushes), the `docker compose down --volumes` cleanup at the start of the second run will kill the first run's containers. This is not a security issue per se, but it means concurrent CI runs will corrupt each other — worth flagging to Tobias. ### Recommendations - **Create the `E2E_ADMIN_PASSWORD` Gitea secret before the first run** and set it to something that is not `admin123`. The E2E profile is ephemeral, but the discipline of never accepting a known default password in any CI pipeline is worth maintaining. Any 16-char random string suffices. - **Pass `APP_ADMIN_PASSWORD` via environment variable, not CLI flag.** Replace `--APP_ADMIN_PASSWORD=${{ secrets.E2E_ADMIN_PASSWORD }}` in the `java -jar` call with an explicit `env:` block entry: `APP_ADMIN_PASSWORD: ${{ secrets.E2E_ADMIN_PASSWORD }}`. Environment variables are less visible than CLI args in process listings. - **Verify `SPRING_PROFILES_ACTIVE` does not include `e2e` in any production configuration.** Check `docker-compose.prod.yml` (if it exists) and the production `.env`. The `e2e` profile enables `AuthE2EController` which exposes password reset tokens in plaintext — this must never reach a real deployment. - **Remove the `admin123` default from `application.yaml`.** Replace `password: ${APP_ADMIN_PASSWORD:admin123}` with `password: ${APP_ADMIN_PASSWORD}` (no default). This forces the env var to be explicitly set in every environment, making a misconfiguration loud rather than silently falling back to a known password.
Author
Owner

🧪 Sara Holt — QA Engineer

Observations

  • The four tests in stammbaum.spec.ts follow good E2E practices: they test user-visible behavior (headings, links, rendered nodes) rather than implementation details. The soft assertion in the third test (emptyVisible || nodeCount > 0) is intentional and correct — it tests both valid states without coupling to seeded data.
  • The third test has a flakiness risk. The expect(...).toPass() polling wrapper checks empty.isVisible() and anyNode.count() in a callback, but the default timeout for .toPass() in Playwright is 5 seconds. If the SVG tree renders slowly (e.g., on first load, data fetch in progress), 5 seconds may not be enough on the NAS runner. The playwright.config.ts does not set a custom expect.timeout.
  • No accessibility check (AxeBuilder) in stammbaum.spec.ts. The accessibility.spec.ts may or may not include /stammbaum. Since this is a new route with an interactive SVG tree, an axe scan is worth adding to verify the role="img" and aria-label attributes on the SVG, and the role="button" on nodes, meet WCAG requirements.
  • Test data dependency: The fourth test ("person edit Stammbaum card surfaces the year-range error") navigates to /persons, clicks the first person, opens edit, and fills year fields. This test depends on at least one person existing in the seeded E2E data. The UserDataInitializer creates persons when SPRING_PROFILES_ACTIVE=e2e, so this should be reliable — but if the seed ever changes or is removed, this test will fail with a confusing selector error rather than a clear "no persons seeded" message.
  • Test name consistency: The test.describe block is named 'Stammbaum — issue #358'. References to issue numbers in test names are an anti-pattern — they become meaningless noise after the issue is closed. The describe block name should describe the feature, not the tracking ID.
  • Sequential execution: playwright.config.ts sets fullyParallel: false and workers: 1. The stammbaum tests will run sequentially with all other E2E specs. Given the NAS runner's constraints this is acceptable, but it means total E2E time is the sum of all spec durations. Monitor whether the suite stays under 8 minutes after enabling the new tests.

Recommendations

  • Set an explicit expect.timeout of 10 seconds in playwright.config.ts (the Playwright default is 5s). For an SVG tree rendered from an API response, this gives the page adequate time to settle without being excessively generous.
  • Rename the test.describe block from 'Stammbaum — issue #358' to 'Stammbaum page' or 'Stammbaum — family tree view'. Test names should describe behavior, not ticket history.
  • Add a clear precondition comment to the fourth test noting it depends on the E2E seed having at least one person. Example: // Requires: UserDataInitializer seeds at least one Person (guaranteed by e2e profile). This documents the assumption explicitly for future maintainers.
  • After the CI job is green, add /stammbaum to the axe scan in accessibility.spec.ts (or a new entry in the stammbaum spec) — even a one-line check covers the most common SVG accessibility pitfalls (missing alt text, unlabeled interactive elements).
  • Add a CI timing comment to the PR description noting estimated E2E job duration, so there is a baseline for tracking regression.

Open Decisions

  • Should the fourth test (year-range error) be in stammbaum.spec.ts or persons.spec.ts? It tests person-edit behavior triggered from a Stammbaum journey. If the persons edit spec already covers similar validation, this test is better placed there for discoverability. If Stammbaum owns the "add relationship" flow end-to-end, keeping it here is defensible.
## 🧪 Sara Holt — QA Engineer ### Observations - The four tests in `stammbaum.spec.ts` follow good E2E practices: they test user-visible behavior (headings, links, rendered nodes) rather than implementation details. The soft assertion in the third test (`emptyVisible || nodeCount > 0`) is intentional and correct — it tests both valid states without coupling to seeded data. - **The third test has a flakiness risk.** The `expect(...).toPass()` polling wrapper checks `empty.isVisible()` and `anyNode.count()` in a callback, but the default timeout for `.toPass()` in Playwright is 5 seconds. If the SVG tree renders slowly (e.g., on first load, data fetch in progress), 5 seconds may not be enough on the NAS runner. The `playwright.config.ts` does not set a custom `expect.timeout`. - **No accessibility check (`AxeBuilder`) in `stammbaum.spec.ts`.** The `accessibility.spec.ts` may or may not include `/stammbaum`. Since this is a new route with an interactive SVG tree, an axe scan is worth adding to verify the `role="img"` and `aria-label` attributes on the SVG, and the `role="button"` on nodes, meet WCAG requirements. - **Test data dependency:** The fourth test ("person edit Stammbaum card surfaces the year-range error") navigates to `/persons`, clicks the first person, opens edit, and fills year fields. This test depends on at least one person existing in the seeded E2E data. The `UserDataInitializer` creates persons when `SPRING_PROFILES_ACTIVE=e2e`, so this should be reliable — but if the seed ever changes or is removed, this test will fail with a confusing selector error rather than a clear "no persons seeded" message. - **Test name consistency:** The `test.describe` block is named `'Stammbaum — issue #358'`. References to issue numbers in test names are an anti-pattern — they become meaningless noise after the issue is closed. The describe block name should describe the feature, not the tracking ID. - **Sequential execution:** `playwright.config.ts` sets `fullyParallel: false` and `workers: 1`. The stammbaum tests will run sequentially with all other E2E specs. Given the NAS runner's constraints this is acceptable, but it means total E2E time is the sum of all spec durations. Monitor whether the suite stays under 8 minutes after enabling the new tests. ### Recommendations - **Set an explicit `expect.timeout` of 10 seconds in `playwright.config.ts`** (the Playwright default is 5s). For an SVG tree rendered from an API response, this gives the page adequate time to settle without being excessively generous. - **Rename the `test.describe` block** from `'Stammbaum — issue #358'` to `'Stammbaum page'` or `'Stammbaum — family tree view'`. Test names should describe behavior, not ticket history. - **Add a clear precondition comment to the fourth test** noting it depends on the E2E seed having at least one person. Example: `// Requires: UserDataInitializer seeds at least one Person (guaranteed by e2e profile).` This documents the assumption explicitly for future maintainers. - **After the CI job is green, add `/stammbaum` to the axe scan in `accessibility.spec.ts`** (or a new entry in the stammbaum spec) — even a one-line check covers the most common SVG accessibility pitfalls (missing alt text, unlabeled interactive elements). - **Add a CI timing comment** to the PR description noting estimated E2E job duration, so there is a baseline for tracking regression. ### Open Decisions - **Should the fourth test (year-range error) be in `stammbaum.spec.ts` or `persons.spec.ts`?** It tests person-edit behavior triggered from a Stammbaum journey. If the persons edit spec already covers similar validation, this test is better placed there for discoverability. If Stammbaum owns the "add relationship" flow end-to-end, keeping it here is defensible.
Author
Owner

🎨 Leonie Voss — UI/UX & Accessibility

Observations

  • The third test locates SVG nodes via svg[role="img"][aria-label="Stammbaum"] g[role="button"]. This tells me the current Stammbaum implementation assigns role="img" to the outer SVG and role="button" to individual person nodes. This is a reasonable ARIA pattern for an interactive tree — but role="img" on the containing SVG conflicts with the interactive children: a container marked as an image implies its contents are decorative and non-interactive. Screen readers may skip the inner role="button" elements entirely.
  • No aria-label on the individual g[role="button"] nodes. The selector checks that the elements exist but does not verify they have accessible names. A family tree node without an accessible name would be announced as "button" with no context — unusable for a screen reader user.
  • The "Noch keine Familienmitglieder" heading test checks an empty state, which is good practice. Empty states are often the most neglected part of accessible design. Confirm the empty state also includes the "Zur Personenliste" link in a way that is keyboard-reachable and visually distinct from the body text.
  • No responsive / mobile test for the Stammbaum. The spec only tests default viewport. The SVG family tree is inherently a desktop-first component (pan/zoom interaction), but even on desktop it should not overflow or hide the empty-state heading at narrow widths. For the reader audience (younger, phone-first), the Stammbaum should at minimum render the empty state gracefully at 375px.
  • The tests run with reducedMotion: 'reduce' (set in playwright.config.ts), which is correct and will suppress any SVG pan/zoom animations during testing. This is good practice.

Recommendations

  • Change role="img" on the SVG container to role="tree" or role="group" with aria-label="Stammbaum". A role="tree" container signals to assistive technology that the contents are navigable nodes, not a static image. Each g[role="button"] should then have role="treeitem" and an aria-label set to the person's display name (e.g., aria-label="Karl Raddatz (1872–1943)").
  • Add aria-label to each person node. Without an accessible name, role="button" (or role="treeitem") is announced as an unlabeled control. This is a WCAG 2.1 failure (SC 4.1.2). The test should assert anyNode.first().getAttribute('aria-label') returns a non-empty value.
  • Add a test.skip-free viewport test at 375px to the spec once CI is working — even a simple "page does not overflow horizontally" check (no horizontal scrollbar). Stammbaum is likely not the primary mobile view, but the empty state must be presentable on phone.
  • Verify keyboard navigation of the tree. A user who navigates the Stammbaum by keyboard (Tab / Enter) needs to be able to reach each person node and activate it. If the SVG uses custom focus management, ensure tabindex="0" is set on each interactive node and that focus is visually indicated. This can be validated with the existing focus-rings.spec.ts pattern.

Open Decisions

  • Should the Stammbaum SVG be purely decorative (role="img" with no interactive children) or an interactive tree widget? If it's meant to be interactive (clicking nodes navigates to person detail), it needs full keyboard and screen-reader support as described above. If it's intended as a visual-only diagram (read-only overview), then role="img" with a single descriptive aria-label and a separate list-based navigation fallback is the correct approach. The current implementation mixes both, which is the worst of both worlds from an accessibility standpoint. This architectural decision for the SVG tree should be made explicitly before the tests are locked in.
## 🎨 Leonie Voss — UI/UX & Accessibility ### Observations - The third test locates SVG nodes via `svg[role="img"][aria-label="Stammbaum"] g[role="button"]`. This tells me the current Stammbaum implementation assigns `role="img"` to the outer SVG and `role="button"` to individual person nodes. This is a reasonable ARIA pattern for an interactive tree — but `role="img"` on the containing SVG conflicts with the interactive children: a container marked as an image implies its contents are decorative and non-interactive. Screen readers may skip the inner `role="button"` elements entirely. - **No `aria-label` on the individual `g[role="button"]` nodes.** The selector checks that the elements exist but does not verify they have accessible names. A family tree node without an accessible name would be announced as "button" with no context — unusable for a screen reader user. - **The "Noch keine Familienmitglieder" heading test** checks an empty state, which is good practice. Empty states are often the most neglected part of accessible design. Confirm the empty state also includes the "Zur Personenliste" link in a way that is keyboard-reachable and visually distinct from the body text. - **No responsive / mobile test for the Stammbaum.** The spec only tests default viewport. The SVG family tree is inherently a desktop-first component (pan/zoom interaction), but even on desktop it should not overflow or hide the empty-state heading at narrow widths. For the reader audience (younger, phone-first), the Stammbaum should at minimum render the empty state gracefully at 375px. - The tests run with `reducedMotion: 'reduce'` (set in `playwright.config.ts`), which is correct and will suppress any SVG pan/zoom animations during testing. This is good practice. ### Recommendations - **Change `role="img"` on the SVG container to `role="tree"` or `role="group"` with `aria-label="Stammbaum"`.** A `role="tree"` container signals to assistive technology that the contents are navigable nodes, not a static image. Each `g[role="button"]` should then have `role="treeitem"` and an `aria-label` set to the person's display name (e.g., `aria-label="Karl Raddatz (1872–1943)"`). - **Add `aria-label` to each person node.** Without an accessible name, `role="button"` (or `role="treeitem"`) is announced as an unlabeled control. This is a WCAG 2.1 failure (SC 4.1.2). The test should assert `anyNode.first().getAttribute('aria-label')` returns a non-empty value. - **Add a `test.skip`-free viewport test at 375px** to the spec once CI is working — even a simple "page does not overflow horizontally" check (no horizontal scrollbar). Stammbaum is likely not the primary mobile view, but the empty state must be presentable on phone. - **Verify keyboard navigation of the tree.** A user who navigates the Stammbaum by keyboard (Tab / Enter) needs to be able to reach each person node and activate it. If the SVG uses custom focus management, ensure `tabindex="0"` is set on each interactive node and that focus is visually indicated. This can be validated with the existing `focus-rings.spec.ts` pattern. ### Open Decisions - **Should the Stammbaum SVG be purely decorative (`role="img"` with no interactive children) or an interactive tree widget?** If it's meant to be interactive (clicking nodes navigates to person detail), it needs full keyboard and screen-reader support as described above. If it's intended as a visual-only diagram (read-only overview), then `role="img"` with a single descriptive `aria-label` and a separate list-based navigation fallback is the correct approach. The current implementation mixes both, which is the worst of both worlds from an accessibility standpoint. This architectural decision for the SVG tree should be made explicitly before the tests are locked in.
Author
Owner

🗳️ Decision Queue — Cross-Persona Open Items

Grouped by theme. These are genuine tradeoffs where your call is needed before or during implementation.


1. CI Trigger Scope

Raised by: Markus

Should the new e2e-tests job run on every branch push, or only on pull requests? Every push currently triggers all jobs. With E2E adding 5–10 minutes of runtime (Maven build + Docker + Playwright), this increases NAS load significantly on active branches.

Options:

  • on: push / pull_request (current default) — maximum coverage, more load
  • on: pull_request only — E2E only runs before merge, not on every WIP push
  • Path filter to skip on docs-only changes

2. Full E2E Suite vs. Stammbaum-Only

Raised by: Felix

npm run test:e2e runs all specs, not just stammbaum.spec.ts. If any existing spec is fragile on the NAS runner, this PR will be blocked until that is fixed too.

Options:

  • Run the full suite (npm run test:e2e) — complete coverage from day one
  • Scope to npx playwright test e2e/stammbaum.spec.ts in this issue — unblock the skip removal, expand later

3. E2E Artifact Upload Policy

Raised by: Tobias

if: always() uploads screenshots/traces/videos on every run. On a passing suite, this is churn with no diagnostic value.

Options:

  • if: failure() — upload only when something broke (default recommendation)
  • if: always() — always upload for audit trail

4. Stammbaum SVG Accessibility Model

Raised by: Leonie

The current implementation uses role="img" on the SVG container with role="button" on child <g> nodes. These two roles conflict: an image container implies non-interactive children, and screen readers may skip the buttons entirely.

Options:

  • Keep role="img" and make the tree purely decorative — provide a separate list-based navigation fallback (simpler, but removes SVG interactivity for AT users)
  • Change to role="tree" / role="treeitem" with per-node aria-label values — full keyboard and AT support (more work, but correct for an interactive widget)

This decision affects the Stammbaum component implementation, not just the CI job — but it needs to be made before the E2E tests are locked in, since the selectors in stammbaum.spec.ts test the ARIA structure directly.


5. Year-Range Test Placement

Raised by: Sara

The fourth test in stammbaum.spec.ts (year-range validation error on person edit) navigates through a Stammbaum journey to reach a persons edit form. It tests person-edit validation behavior, not Stammbaum-specific rendering.

Options:

  • Keep in stammbaum.spec.ts — documents the end-to-end journey from the tree to edit
  • Move to persons.spec.ts — test lives alongside other persons edit tests, easier to find when persons edit changes
## 🗳️ Decision Queue — Cross-Persona Open Items Grouped by theme. These are genuine tradeoffs where your call is needed before or during implementation. --- ### 1. CI Trigger Scope _Raised by: Markus_ Should the new `e2e-tests` job run on every branch push, or only on pull requests? Every push currently triggers all jobs. With E2E adding 5–10 minutes of runtime (Maven build + Docker + Playwright), this increases NAS load significantly on active branches. **Options:** - `on: push / pull_request` (current default) — maximum coverage, more load - `on: pull_request` only — E2E only runs before merge, not on every WIP push - Path filter to skip on docs-only changes --- ### 2. Full E2E Suite vs. Stammbaum-Only _Raised by: Felix_ `npm run test:e2e` runs all specs, not just `stammbaum.spec.ts`. If any existing spec is fragile on the NAS runner, this PR will be blocked until that is fixed too. **Options:** - Run the full suite (`npm run test:e2e`) — complete coverage from day one - Scope to `npx playwright test e2e/stammbaum.spec.ts` in this issue — unblock the skip removal, expand later --- ### 3. E2E Artifact Upload Policy _Raised by: Tobias_ `if: always()` uploads screenshots/traces/videos on every run. On a passing suite, this is churn with no diagnostic value. **Options:** - `if: failure()` — upload only when something broke (default recommendation) - `if: always()` — always upload for audit trail --- ### 4. Stammbaum SVG Accessibility Model _Raised by: Leonie_ The current implementation uses `role="img"` on the SVG container with `role="button"` on child `<g>` nodes. These two roles conflict: an image container implies non-interactive children, and screen readers may skip the buttons entirely. **Options:** - Keep `role="img"` and make the tree purely decorative — provide a separate list-based navigation fallback (simpler, but removes SVG interactivity for AT users) - Change to `role="tree"` / `role="treeitem"` with per-node `aria-label` values — full keyboard and AT support (more work, but correct for an interactive widget) This decision affects the Stammbaum component implementation, not just the CI job — but it needs to be made before the E2E tests are locked in, since the selectors in `stammbaum.spec.ts` test the ARIA structure directly. --- ### 5. Year-Range Test Placement _Raised by: Sara_ The fourth test in `stammbaum.spec.ts` (year-range validation error on person edit) navigates through a Stammbaum journey to reach a persons edit form. It tests person-edit validation behavior, not Stammbaum-specific rendering. **Options:** - Keep in `stammbaum.spec.ts` — documents the end-to-end journey from the tree to edit - Move to `persons.spec.ts` — test lives alongside other persons edit tests, easier to find when persons edit changes
Sign in to join this conversation.
No Label P2-medium devops test
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#363