Issue #5: CI test redesign — nested lifecycle, dynamic IDs, log collector, LLM judge

.github/workflows/docker-publish.yml

@@ -5,9 +5,13 @@ name: Docker
 # separate terms of service, privacy policy, and support
 # documentation.
 
+# Manual-only: this workflow publishes to GHCR and requires registry write
+# auth that's not configured on every fork. Trigger from the Actions tab
+# when you actually want to release. Restricted to the main branch via the
+# job's `if:` guard below so accidental dispatches from feature branches
+# can't publish.
 on:
-  push:
-    branches: [ "*" ]
+  workflow_dispatch:
 
 env:
   # Use docker.io for Docker Hub if empty
@@ -18,7 +22,8 @@ env:
 
 jobs:
   build:
-
+    # Guard: only ever publish from the main branch, even when dispatched.
+    if: github.ref == 'refs/heads/testlink_1_9_20_fixed'
     runs-on: ubuntu-latest
     permissions:
       contents: read

.github/workflows/test-pipeline.yml

@@ -1,10 +1,9 @@
 name: CI Pipeline
 
 on:
-  push:
-    branches: ["testlink_1_9_20_fixed"]
-  pull_request:
-    branches: ["testlink_1_9_20_fixed"]
+  # Manual-only: trigger from the Actions tab. No automatic firing on push
+  # or PR — keeps CI noise off feature branches and lets you run the suite
+  # against any ref on demand.
   workflow_dispatch:
     inputs:
       judge_mode:
@@ -24,18 +23,34 @@ jobs:
       suite: build
       judge_mode: ${{ inputs.judge_mode || 'simple' }}
 
-  integration:
-    name: Integration Tests
+  smoke:
+    name: Smoke Tests
     needs: build
     uses: ./.github/workflows/test-suite.yml
     with:
-      suite: integration
+      suite: smoke
       judge_mode: ${{ inputs.judge_mode || 'simple' }}
 
-  e2e:
-    name: E2E Tests
-    needs: integration
+  auth:
+    name: Auth Tests
+    needs: smoke
     uses: ./.github/workflows/test-suite.yml
     with:
-      suite: e2e
+      suite: auth
+      judge_mode: ${{ inputs.judge_mode || 'simple' }}
+
+  crud:
+    name: CRUD Tests
+    needs: auth
+    uses: ./.github/workflows/test-suite.yml
+    with:
+      suite: crud
+      judge_mode: ${{ inputs.judge_mode || 'simple' }}
+
+  workflow:
+    name: Workflow Tests
+    needs: crud
+    uses: ./.github/workflows/test-suite.yml
+    with:
+      suite: workflow
       judge_mode: ${{ inputs.judge_mode || 'simple' }}

.github/workflows/test-suite.yml

@@ -9,8 +9,12 @@ on:
         type: choice
         options:
           - "build"
-          - "integration"
-          - "e2e"
+          - "smoke"
+          - "auth"
+          - "crud"
+          - "workflow"
+          - "negative"
+          - "regression"
           - "all"
       judge_mode:
         description: "Test judge mode"
@@ -68,8 +72,6 @@ jobs:
       - name: Run tests
         id: run-tests
         run: |
-          cd cicd/tests
-
           # Build judge flags based on input
           JUDGE_FLAGS=""
           if [ "${{ inputs.judge_mode }}" = "simple" ] || [ -z "${{ inputs.judge_mode }}" ]; then
@@ -88,8 +90,9 @@ jobs:
           echo "Judge mode: ${{ inputs.judge_mode || 'simple' }}"
           echo "Judge flags: $JUDGE_FLAGS"
 
-          # Run tests with JSON output
-          npx tsx src/cli.ts run $SUITE_FLAG $JUDGE_FLAGS --format json > /tmp/test-results.json || true
+          # Run through the wrapper: handles ci-up before and ci-down after
+          # (trap EXIT) so teardown is guaranteed even on test failure.
+          bash cicd/scripts/run-tests.sh $SUITE_FLAG $JUDGE_FLAGS --format json > /tmp/test-results.json || true
 
           echo "--- JSON Results ---"
           cat /tmp/test-results.json

cicd/TESTING_GUIDELINES.md

@@ -0,0 +1,248 @@
+# CI Testing Guidelines
+
+Design rules for the TestLink CI test suite under `cicd/`. Framework-agnostic — applies whether the runner is YAML-based, Vitest, or plain shell.
+
+---
+
+## 1. Purpose
+
+TestLink is a test management system. Its value is the correctness of its API and data operations. These guidelines define **how we test it** so that:
+
+- CI runs are reproducible and deterministic
+- Test failures point to the real cause, not infrastructure drift
+- Tests can be run in isolation, reordered, or rerun without manual cleanup
+- Teardown is guaranteed, even on failure
+
+---
+
+## 2. Design Principles
+
+1. **Tests talk through the public API.** XML-RPC and REST are the contracts under test. Go through them like a real client would.
+2. **Direct DB access is reserved for three jobs only:** seeding the admin API key, asserting side effects that are not exposed via API, and emergency cleanup. Never use SQL to shortcut a test setup that could go through the API.
+3. **Every test owns its data.** A test creates what it needs, asserts against it, and deletes it. Tests do not rely on residue from previous tests.
+4. **IDs flow through capture, never hardcoded.** The ID of a created entity comes from the creation response, not from assuming "it'll be id=1".
+5. **Teardown is guaranteed.** Every scope's teardown runs in a `finally` / `trap EXIT` so that a failure does not leak state into the next run.
+6. **Idempotency.** Running the same suite twice in a row against a fresh environment produces identical results.
+
+---
+
+## 3. Nested Lifecycle Scopes
+
+Four scopes nest like Russian dolls. Each has its own `setup → run children → teardown`, with teardown always guaranteed.
+
+```
+SESSION (once per CI run)
+  └── SUITE (once per suite: smoke, auth, crud, workflow, …)
+        └── TEST CASE (once per test)
+              └── STEP (one action)
+```
+
+### 3.1 Session scope
+
+Runs once per CI invocation. Owns the infrastructure.
+
+**Setup:**
+- `docker compose -f cicd/docker-compose.ci.yml up -d`
+- Wait for healthchecks
+- Load schema: `testlink_create_tables.sql` + UDFs
+- Load default data: `testlink_create_default_data.sql`
+- Seed the admin API key (`UPDATE users SET script_key='…' WHERE id=1`)
+
+**Teardown (always):**
+- `docker compose -f cicd/docker-compose.ci.yml down -v --remove-orphans`
+
+**Rule:** session setup owns infrastructure and baseline (admin + API key) **only**. No test-specific data.
+
+### 3.2 Suite scope
+
+One per feature area (auth, crud, workflow, …). Owns shared fixtures for that area.
+
+**Setup:**
+- Create a dedicated test project with a unique name (`suite-<name>-<timestamp>`)
+- Capture the project ID for all tests in the suite
+- Create any users, roles, or custom fields this suite needs
+
+**Teardown (always):**
+- Delete the suite's test project (cascades to everything under it)
+
+**Rule:** no suite may depend on another suite's residue. Any suite must be runnable alone and in any order.
+
+### 3.3 Test case scope
+
+One per test. Owns the minimum entities the test needs.
+
+**Setup:**
+- Create the entities the test acts on (test suite, test case, test plan, build)
+- Use unique names tied to the test ID: `<testId>-<random>`
+- Capture every created ID
+
+**Teardown (always):**
+- Delete the entities this test created, in reverse order
+
+**Rule:** a test must be runnable twice in a row against the same suite fixtures without manual cleanup between runs.
+
+### 3.4 Step scope
+
+One API call with one assertion. No hidden side effects. If a step creates state that outlives itself, the creation must be explicit and captured.
+
+---
+
+## 4. Test Flow
+
+Test categories layer on top of each other. A failure at a lower layer should short-circuit the higher layers — no point testing CRUD if the server isn't responding.
+
+| Layer | Tests | Purpose |
+|---|---|---|
+| **Smoke** | `tl.ping`, login page loads | Is the system alive? |
+| **Auth** | valid key, invalid key, missing key | Can we talk to it? |
+| **CRUD (leaf-up)** | create/read/update/delete for each entity in dependency order: **project → testsuite → testcase → testplan → build → execution** | Do basic operations work? |
+| **Workflow** | assign case to plan → create build → record execution → query results | Do operations compose end-to-end? |
+| **Negative** | bad input, missing required fields, permission denied, FK violations | Does it fail correctly? |
+| **Regression** | specific bug repros (one test per closed bug) | Does a past bug stay fixed? |
+
+**Short-circuit rule:** if Smoke fails, skip everything below. Implementation: make Auth depend on Smoke, CRUD on Auth, and so on. The runner skips dependents when a dep fails.
+
+---
+
+## 5. Dynamic ID Capture
+
+IDs are never hardcoded. They flow from creation responses into subsequent steps via captured variables.
+
+### 5.1 Rule
+
+> **No numeric ID literal (`1`, `2`, `42`) may appear in a test step except in assertions about the response shape itself.**
+
+### 5.2 Pattern
+
+```yaml
+- name: Create test project
+  command: |
+    curl -sf -X POST "$TL_URL/lib/api/xmlrpc/v1/xmlrpc.php" \
+      -H "Content-Type: text/xml" \
+      -d '<?xml … tl.createTestProject … testprojectname=proj-{{runId}} …>'
+  capture:
+    projectId: "$[name=id].value"    # resolve from response
+
+- name: Create test suite under it
+  command: |
+    curl -sf … testprojectid={{projectId}} testsuitename=suite-{{runId}} …
+  capture:
+    suiteId: "$[name=id].value"
+
+- name: Delete test suite
+  command: curl -sf … testsuiteid={{suiteId}} …
+
+- name: Delete test project
+  command: curl -sf … prjid={{projectId}} …
+```
+
+### 5.3 Rationale
+
+- Runs are reorderable: no dependency on seed counters.
+- Runs are re-runnable: no "id=1 already exists" collisions.
+- Parallel safety: different runs pick different IDs without coordination.
+- Failures are traceable: logs show which concrete IDs were in play.
+
+---
+
+## 6. Test Data Rules
+
+1. **Unique names.** Every created entity gets a name that includes the run ID or a timestamp: `proj-20260419-143022`. Never `proj`, never `CI Test Project`.
+2. **Prefix by test ID** where practical (`TC-CRUD-003-project-…`) so orphaned data is traceable.
+3. **Capture every ID** from creation responses. Never assume the next auto-increment value.
+4. **Delete in reverse creation order.** Children before parents.
+5. **Idempotency.** A second run against the same fixtures must behave identically. Verify by running the suite twice locally before committing.
+6. **No implicit ordering between test cases** beyond declared `dependencies`. If test B silently needs test A's data, declare the dependency or duplicate the setup.
+
+---
+
+## 7. Teardown Guarantees
+
+Each scope's teardown must run **even when a test or step fails**. Implementation depends on scope:
+
+| Scope | Guarantee mechanism |
+|---|---|
+| Session | Wrapper script with `trap 'ci-down.sh' EXIT` |
+| Suite | Runner executes suite-teardown in a `finally` block after all tests, regardless of results |
+| Test case | Runner executes test-teardown in a `finally` block after all steps, regardless of step outcomes |
+| Step | Step is atomic; no sub-teardown |
+
+A teardown failure must surface loudly but must **not** prevent outer teardowns from running.
+
+---
+
+## 8. Why `docker-compose.ci.yml` Is Separate from `docker-compose.yml`
+
+The repo has two compose files on purpose. They serve different audiences and have incompatible lifecycle assumptions. This section is grounded in the actual current files (`docker-compose.yml` at repo root and `cicd/docker-compose.ci.yml`).
+
+| Aspect | `docker-compose.yml` (dev) | `cicd/docker-compose.ci.yml` (CI) |
+|---|---|---|
+| Audience | Developers working locally | Automated CI and test runs |
+| Lifecycle | `up` once, leave running for days | `up → test → down -v` every run |
+| DB state | Named volume `postgres` — persistent across restarts | No named DB volume — anonymous, destroyed by `down -v` |
+| Services | `db` + `maildev` + `app` + `restore` (profile=tools, opt-in) | `db` + `app` only |
+| DB credentials | `teste` / `teste` | `testlink` / `testlink` |
+| Config injection | None — relies on defaults baked into the image | Mounts `ci-config_db.inc.php` (CI DB creds) + `ci-custom_config.inc.php` (API on, SMTP off) |
+| Seed data | Optional: `restore` service loads `docs/db_sample` sample data | `init-db.sh` loads schema + default data + hardcoded admin API key |
+| Image build | `build: .` from repo root Dockerfile | `build: ..` from the same Dockerfile |
+| Image pinning | `postgres:9.6` pinned; `maildev:latest` floats | Fully pinned (`postgres:9.6`) |
+| Host port | `0.0.0.0:8090:80` | `${TL_PORT:-8091}:80` (default 8091) |
+
+### Host port: 8090 (dev) vs 8091 (CI)
+
+The two stacks bind different host ports so they can run simultaneously on a developer machine. CI uses `8091` by default, sourced from `TL_PORT` in `cicd/tests/.env`. All CI scripts and YAML test steps reference `$TL_URL` (default `http://localhost:8091`) rather than hardcoding the port; change the port in one place and everything follows.
+
+### Why the separation is mandatory
+
+1. **Teardown safety.** CI teardown runs `down -v`, which destroys volumes. The dev compose's `postgres` named volume holds a developer's in-progress database. Because the two compose files live in different directories (`./` and `cicd/`), Docker Compose gives them different project namespaces (`testlink-code` vs `cicd`) — so their volumes, containers, and networks are isolated. `down -v` run against the CI compose cannot touch the dev compose's `postgres` volume. Merging them would break this guarantee.
+2. **Reproducibility of seed and config.** CI must start from a known state: fresh schema, default data, known admin API key `a1b2…`, API enabled, SMTP off. The dev compose intentionally does not inject any of that — it relies on whatever the developer has set up. Merging the two would either corrupt dev state on every CI run or require runtime conditionals that obscure both flows.
+3. **Credentials and data isolation.** Different DB users (`teste` vs `testlink`), different injected configs, different (or no) seed data. A single file trying to serve both audiences would need env-var gymnastics that make each flow harder to reason about.
+4. **Service minimalism.** CI starts only `db` + `app`. Dev adds `maildev` for catching outbound mail and `restore` (profile-gated) for loading sample data. CI doesn't need either — faster startup, narrower failure surface, fewer moving parts when diagnosing a test failure.
+
+### Naming convention
+
+- Dev compose: **`docker-compose.yml`** at the repo root. The file `docker compose up` picks up by default — optimize for developer ergonomics.
+- CI compose: **`cicd/docker-compose.ci.yml`**, explicit path, never the default. Living under `cicd/` also changes Docker Compose's default project name to `cicd`, which is what keeps volumes and containers isolated from the dev stack.
+
+**Rule:** any new compose file for a specific purpose (e2e-only, perf testing, …) gets its own name under a dedicated directory (not the repo root). Never overload `docker-compose.yml`.
+
+---
+
+## 9. Directory Conventions
+
+```
+cicd/
+├── docker-compose.ci.yml      # CI environment definition
+├── scripts/                   # Session-scope lifecycle
+│   ├── ci-up.sh               # Session setup (build + up + seed)
+│   ├── ci-down.sh             # Session teardown (down -v)
+│   ├── init-db.sh             # Schema + default data + admin API key
+│   ├── ci-config_db.inc.php   # DB config injected into app
+│   └── ci-custom_config.inc.php  # App config (API on, SMTP off)
+├── tests/
+│   ├── src/                   # Test runner (TypeScript)
+│   └── testcases/             # Test definitions
+│       ├── smoke/
+│       ├── auth/
+│       ├── crud/
+│       ├── workflow/
+│       ├── negative/
+│       └── regression/
+└── results/                   # Per-run output (gitignored)
+```
+
+**Rule:** infrastructure code (scripts, compose) lives in `cicd/`. Test definitions live in `cicd/tests/testcases/`. Nothing test-related lives outside `cicd/`.
+
+---
+
+## 10. Checklist for Writing a New Test
+
+- [ ] Suite category is correct (smoke / auth / crud / workflow / negative / regression)
+- [ ] `dependencies` is declared if this test needs another test's entities to already pass
+- [ ] All created entities use unique names that include the run ID or timestamp
+- [ ] All IDs used in later steps are `capture:`d from earlier responses — no hardcoded integers
+- [ ] Test creates exactly what it needs and deletes it in reverse order in teardown
+- [ ] Test passes when run alone: `npm run test -- --id TC-XXX-NNN`
+- [ ] Test passes when run twice in a row against the same session
+- [ ] Assertions check response shape and content, not just exit code
+- [ ] Teardown runs even when a prior step fails (verified by injecting a failure locally)

cicd/docker-compose.ci.yml

@@ -32,7 +32,7 @@ services:
       db:
         condition: service_healthy
     ports:
-      - "8090:80"
+      - "${TL_PORT:-8091}:80"
     volumes:
       - ../logs:/var/testlink/logs:Z
       - ../upload_area:/var/testlink/upload_area:Z