Compare commits
9 Commits
948a68c280
...
76db26d68b
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
76db26d68b | ||
|
|
3de0d2f0fe | ||
|
|
0abbc147e2 | ||
|
|
6210480952 | ||
|
|
e17f4110f1 | ||
|
|
fa46492759 | ||
|
|
3965541879 | ||
|
|
582191d014 | ||
|
|
118100e58d |
@@ -56,6 +56,26 @@ jobs:
|
||||
exit 1
|
||||
fi
|
||||
|
||||
- name: Assert no (upload|download)-artifact past v3
|
||||
shell: bash
|
||||
run: |
|
||||
# Self-test: verify the regex catches v4+ and does not catch v3.
|
||||
tmp=$(mktemp)
|
||||
printf ' uses: actions/upload-artifact@v5\n' > "$tmp"
|
||||
grep -qP '^\s+uses:\s+actions/(upload|download)-artifact@v[4-9]' "$tmp" \
|
||||
|| { echo "FAIL: guard self-test — regex missed upload-artifact@v5"; rm "$tmp"; exit 1; }
|
||||
printf ' uses: actions/upload-artifact@v3\n' > "$tmp"
|
||||
grep -qvP '^\s+uses:\s+actions/(upload|download)-artifact@v[4-9]' "$tmp" \
|
||||
|| { echo "FAIL: guard self-test — regex incorrectly flagged upload-artifact@v3"; rm "$tmp"; exit 1; }
|
||||
rm "$tmp"
|
||||
# Guard: Gitea Actions (act_runner) does not implement the v4 artifact protocol.
|
||||
# Both upload-artifact and download-artifact share the same incompatibility.
|
||||
# Pin to @v3. See ADR-014 / #557.
|
||||
if grep -RPn '^\s+uses:\s+actions/(upload|download)-artifact@v[4-9]' .gitea/workflows/; then
|
||||
echo "::error::actions/(upload|download)-artifact@v4+ is unsupported on Gitea Actions (act_runner). Pin to @v3. See ADR-014 / #557."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
- name: Run unit and component tests with coverage
|
||||
shell: bash
|
||||
run: |
|
||||
@@ -77,9 +97,10 @@ jobs:
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Gitea Actions (act_runner) does not implement upload-artifact v4 protocol — pinned per ADR-014. Do NOT upgrade. See #557.
|
||||
- name: Upload coverage reports
|
||||
if: always()
|
||||
uses: actions/upload-artifact@v4
|
||||
uses: actions/upload-artifact@v3
|
||||
with:
|
||||
name: coverage-reports
|
||||
path: |
|
||||
@@ -113,9 +134,10 @@ jobs:
|
||||
|| { echo "FAIL: /hilfe/transkription.html missing from prerender output"; exit 1; }
|
||||
echo "PASS: only /hilfe/transkription.html prerendered."
|
||||
|
||||
# Gitea Actions (act_runner) does not implement upload-artifact v4 protocol — pinned per ADR-014. Do NOT upgrade. See #557.
|
||||
- name: Upload screenshots
|
||||
if: always()
|
||||
uses: actions/upload-artifact@v4
|
||||
uses: actions/upload-artifact@v3
|
||||
with:
|
||||
name: unit-test-screenshots
|
||||
path: frontend/test-results/screenshots/
|
||||
@@ -269,6 +291,7 @@ jobs:
|
||||
MAIL_HOST=mailpit
|
||||
MAIL_PORT=1025
|
||||
APP_MAIL_FROM=noreply@local
|
||||
IMPORT_HOST_DIR=/tmp/dummy-import
|
||||
EOF
|
||||
|
||||
- name: Bring up minio
|
||||
|
||||
@@ -56,9 +56,10 @@ jobs:
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Gitea Actions (act_runner) does not implement upload-artifact v4 protocol — pinned per ADR-014. Do NOT upgrade. See #557.
|
||||
- name: Upload coverage log on failure
|
||||
if: failure()
|
||||
uses: actions/upload-artifact@v4
|
||||
uses: actions/upload-artifact@v3
|
||||
with:
|
||||
name: coverage-log-run-${{ matrix.run }}
|
||||
path: /tmp/coverage-test-${{ github.run_id }}-${{ matrix.run }}.log
|
||||
|
||||
92
docs/adr/013-client-branches-coverage-threshold.md
Normal file
92
docs/adr/013-client-branches-coverage-threshold.md
Normal file
@@ -0,0 +1,92 @@
|
||||
# ADR 013 — Client-Project Branch Coverage Threshold
|
||||
|
||||
**Status:** Accepted
|
||||
**Date:** 2026-05-14
|
||||
**Issues:** [#556 — threshold drop](https://git.raddatz.cloud/marcel/familienarchiv/issues/556) · [#496 — long-tail-grind tracking](https://git.raddatz.cloud/marcel/familienarchiv/issues/496)
|
||||
|
||||
---
|
||||
|
||||
## Context
|
||||
|
||||
The browser-mode component test suite (`vitest.client-coverage.config.ts`) enforces Istanbul coverage thresholds across `lines`, `functions`, `branches`, and `statements`. The `branches` metric was set to 80%, but the codebase sits at **75%** — below the gate — causing every CI run of `unit-tests` and `coverage-flake-probe` to fail on this check alone, even when all tests are green.
|
||||
|
||||
**Measured baseline (2026-05-14, branch `feat/issue-553-birpc-async-mock-factory`, head `2e6cc346`):**
|
||||
|
||||
```
|
||||
branches: 75% (below the 80% gate — reason for this ADR)
|
||||
lines: ≥ 80%
|
||||
functions: ≥ 80%
|
||||
statements: ≥ 80%
|
||||
```
|
||||
|
||||
Reproducer:
|
||||
|
||||
```bash
|
||||
cd frontend && npm ci && npx vitest run -c vitest.client-coverage.config.ts --coverage
|
||||
```
|
||||
|
||||
### The long-tail-grind problem
|
||||
|
||||
In Istanbul's branch accounting, when a child component gains test coverage its branches are added to the parent's denominator. A child moving from 40% → 80% coverage can drag a parent from 78% → 72% because more branches in the call graph become reachable and must be covered. This is not a bug — it is how branch accounting works — but it means that on a large SvelteKit application the denominator grows with every coverage improvement, making an arbitrary 80% ceiling a constant grind. Per #496, the expected cost to reach 80% branches from 75% is 30–100+ commits with no guarantee of stability.
|
||||
|
||||
### Why this layer is different
|
||||
|
||||
The 80% branch floor used for backend unit/integration tests is appropriate for Java service code and permission logic. Browser-mode component coverage measures Svelte template branches: conditional class bindings, `{#if}` blocks, empty/loaded/error state guards. These branches have a fundamentally different accounting model and a higher inherent denominator. This ADR **only** lowers the browser-mode component gate; the backend test coverage gates are unaffected.
|
||||
|
||||
### Security-relevant uncovered components
|
||||
|
||||
The following auth/permission-boundary components currently have low or zero branch coverage. When ratchet-up work begins (see below), these are the highest-priority targets:
|
||||
|
||||
- `src/routes/login/+page.svelte`
|
||||
- `src/routes/forgot-password/+page.svelte`
|
||||
- `src/routes/reset-password/+page.svelte`
|
||||
- `src/routes/register/+page.svelte`
|
||||
|
||||
Note: the 75% figure already reflects the absence of coverage on these files. Lowering the gate does not create this gap — it makes the existing state legible.
|
||||
|
||||
---
|
||||
|
||||
## Decision
|
||||
|
||||
Drop the `branches` threshold from `80` → `75` in `frontend/vitest.client-coverage.config.ts`. Leave `lines`, `functions`, and `statements` at `80`.
|
||||
|
||||
The 75% figure matches the measured current state, allowing CI to pass while deliberate coverage improvement work (tracked in #496) continues without blocking other PRs. The asymmetry in the thresholds block is intentional and documented with an inline comment pointing here.
|
||||
|
||||
---
|
||||
|
||||
## Ratchet Rule
|
||||
|
||||
The branches threshold ratchets **up by 3 percentage points** when the rolling 3-PR-average client-project branches figure on `main` stays at or above `threshold + 3pp` for ≥ 30 consecutive days. Direction is **up-only** — never lower the floor below 75 without a new ADR superseding this one. Manual today (verify before any `vitest.client-coverage.config.ts` edit); a future automation issue may codify the check.
|
||||
|
||||
Concretely:
|
||||
- When `main` sustains ≥ 78% branches across 3 consecutive PRs for 30 days → raise gate to 78%
|
||||
- When `main` sustains ≥ 81% branches across 3 consecutive PRs for 30 days → raise gate back to 80%
|
||||
|
||||
---
|
||||
|
||||
## Non-goals
|
||||
|
||||
- **Not** raising actual branch coverage — that is #496's job, tracked separately.
|
||||
- **Not** touching the server-project coverage configuration (`vitest.config.ts`) — only the client project hits the long-tail-grind pattern.
|
||||
- **Not** removing or relaxing any existing test files, `skipIf` guards, or axe-playwright accessibility runs.
|
||||
|
||||
---
|
||||
|
||||
## Consequences
|
||||
|
||||
**Easier:**
|
||||
- CI unblocked — `unit-tests` and `coverage-flake-probe` jobs pass when all tests are green
|
||||
- The ratchet rule creates a concrete, observable path back to 80%
|
||||
|
||||
**Harder:**
|
||||
- The gate now has near-zero headroom — any branch regression that drops below 75% will fail CI immediately
|
||||
- The 75% floor must not be treated as a permanent ceiling; the ratchet discipline requires active attention
|
||||
|
||||
---
|
||||
|
||||
## References
|
||||
|
||||
- [#496 — Branch coverage long-tail grind](https://git.raddatz.cloud/marcel/familienarchiv/issues/496)
|
||||
- [#556 — This threshold drop](https://git.raddatz.cloud/marcel/familienarchiv/issues/556)
|
||||
- [ADR 012 — Browser-Mode Test Mocking Strategy](./012-browser-test-mocking-strategy.md)
|
||||
- `frontend/vitest.client-coverage.config.ts` — thresholds block (lines 44–51)
|
||||
122
docs/adr/014-upload-artifact-v3-pin.md
Normal file
122
docs/adr/014-upload-artifact-v3-pin.md
Normal file
@@ -0,0 +1,122 @@
|
||||
# ADR 014 — Pin actions/upload-artifact to v3 (Gitea act_runner v4 protocol incompatibility)
|
||||
|
||||
**Status:** Accepted
|
||||
**Date:** 2026-05-14
|
||||
**Issues:** [#557 — re-regression](https://git.raddatz.cloud/marcel/familienarchiv/issues/557) · [#14 — original incident](https://git.raddatz.cloud/marcel/familienarchiv/issues/14)
|
||||
|
||||
---
|
||||
|
||||
## Context
|
||||
|
||||
`actions/upload-artifact` is available in two incompatible major versions. The v4 client
|
||||
uploads via a GitHub-specific artifact API that is **not implemented** in Gitea's
|
||||
`act_runner` (the self-hosted CI substrate established by ADR-011). When a workflow step
|
||||
uses `actions/upload-artifact@v4` on this runner, `act_runner` returns a non-zero exit
|
||||
code from the v4 client even when all tests pass, producing:
|
||||
|
||||
> green test suite — red job status — no artifact uploaded
|
||||
|
||||
The failure lands in the upload step, _after_ the test output, making it hard to diagnose
|
||||
from the build log.
|
||||
|
||||
### Incident history
|
||||
|
||||
| Date | Commit | Event |
|
||||
|---|---|---|
|
||||
| 2026-03-19 | `9f3f022e` | Original downgrade: `upload-artifact@v4 → v3` |
|
||||
| 2026-03-19 | `4142c7cd` | Rationale committed; closes #14 |
|
||||
| 2026-05-05 | `410b91e2` | Re-regression: upgraded back to v4 without referencing #14 |
|
||||
| 2026-05-14 | this PR | Second downgrade + ADR + grep guard |
|
||||
|
||||
The root cause of the re-regression was institutional-memory failure: the original
|
||||
rationale was captured only in a commit body, invisible at the point of change (the
|
||||
`uses:` line). This ADR, the inline comments, and the grep guard are the three
|
||||
defence layers that replace that missing breadcrumb.
|
||||
|
||||
---
|
||||
|
||||
## Decision
|
||||
|
||||
**Pin all `actions/upload-artifact` and `actions/download-artifact` call sites to `@v3`.**
|
||||
|
||||
Both action families share the same v4 protocol incompatibility with `act_runner`.
|
||||
Pinning to the major tag (`@v3`) keeps us on the latest v3 patch without Renovate noise.
|
||||
|
||||
Three call sites are pinned:
|
||||
- `.gitea/workflows/ci.yml` — "Upload coverage reports" step
|
||||
- `.gitea/workflows/ci.yml` — "Upload screenshots" step
|
||||
- `.gitea/workflows/coverage-flake-probe.yml` — "Upload coverage log on failure" step
|
||||
|
||||
Each pinned `uses:` line carries a load-bearing inline comment:
|
||||
|
||||
```yaml
|
||||
# Gitea Actions (act_runner) does not implement upload-artifact v4 protocol — pinned per ADR-014. Do NOT upgrade. See #557.
|
||||
- uses: actions/upload-artifact@v3
|
||||
```
|
||||
|
||||
A CI grep guard enforces the constraint automatically (see below).
|
||||
|
||||
---
|
||||
|
||||
## Consequences
|
||||
|
||||
### Enforcement layers (defence in depth)
|
||||
|
||||
1. **Inline comments** on every `uses:` line — visible at the point of change.
|
||||
2. **CI grep guard** in `.gitea/workflows/ci.yml` ("Assert no (upload|download)-artifact
|
||||
past v3") — fails the build if a future commit re-introduces `@v4` or higher on any
|
||||
workflow file. Anchored to YAML `uses:` lines to avoid false positives on embedded
|
||||
shell strings. Includes a self-test that proves the regex catches v4+ before scanning
|
||||
the repo.
|
||||
3. **This ADR** — canonical rationale; cross-referenced by comments and guard message.
|
||||
|
||||
### How to spot the symptom
|
||||
|
||||
- Test suite output shows green (vitest, surefire, pytest all exit 0)
|
||||
- CI job status shows red
|
||||
- Artifacts section of the run is empty
|
||||
- Build log shows a non-zero exit from the `Upload …` step immediately after green tests
|
||||
|
||||
### `@v3` maintenance-mode status
|
||||
|
||||
GitHub placed `actions/upload-artifact@v3` in maintenance mode (no new features) but it
|
||||
has not been removed and carries no known unpatched CVE as of this writing. If GitHub
|
||||
publishes a v3-specific security advisory, that is an additional trigger to re-evaluate
|
||||
(see upgrade conditions below).
|
||||
|
||||
### When to remove this pin
|
||||
|
||||
Re-evaluate pinning **when either condition is met:**
|
||||
|
||||
1. `gitea/act_runner` ships a release with v4 artifact protocol support. Track upstream:
|
||||
<https://gitea.com/gitea/act_runner>
|
||||
2. `actions/upload-artifact@v3` acquires an unpatched CVE that cannot be mitigated
|
||||
at the runner level.
|
||||
|
||||
When upgrading: remove the grep guard step, update all three `uses:` lines, remove the
|
||||
inline comments, and update this ADR's status to Superseded.
|
||||
|
||||
---
|
||||
|
||||
## Alternatives
|
||||
|
||||
### SHA pinning (`uses: actions/upload-artifact@<sha>`)
|
||||
|
||||
More secure against action repository compromise, but adds Renovate update friction
|
||||
and is disproportionate for a self-hosted, single-tenant Gitea instance with one
|
||||
trusted contributor (ADR-011). Rejected.
|
||||
|
||||
### Minor/patch pinning (`@v3.4.0`)
|
||||
|
||||
Avoids Renovate PRs but freezes us on a specific patch. The v3 major track is in
|
||||
maintenance mode — minor pinning has no benefit and would require manual updates
|
||||
for any v3 security patches. Rejected.
|
||||
|
||||
### Renovate `packageRules` bypass
|
||||
|
||||
Would prevent automated PRs from proposing v4. Not needed while Renovate is not
|
||||
configured for this repository. Revisit if Renovate is introduced.
|
||||
|
||||
### Migrating the runner to a v4-compatible Gitea release
|
||||
|
||||
Out of scope for this issue. A separate decision; tracked in #557's non-goals.
|
||||
@@ -200,7 +200,7 @@ jobs:
|
||||
working-directory: frontend
|
||||
- name: Upload screenshots
|
||||
if: always()
|
||||
uses: actions/upload-artifact@v4 # ← upgraded from v3
|
||||
uses: actions/upload-artifact@v3 # pinned per ADR-014 — Gitea Actions does not implement v4 protocol. Do NOT upgrade.
|
||||
with:
|
||||
name: unit-test-screenshots
|
||||
path: frontend/test-results/screenshots/
|
||||
@@ -227,7 +227,7 @@ jobs:
|
||||
working-directory: backend
|
||||
- name: Upload test results
|
||||
if: always()
|
||||
uses: actions/upload-artifact@v4 # ← upgraded from v3
|
||||
uses: actions/upload-artifact@v3 # pinned per ADR-014 — Gitea Actions does not implement v4 protocol. Do NOT upgrade.
|
||||
with:
|
||||
name: backend-test-results
|
||||
path: backend/target/surefire-reports/
|
||||
@@ -329,7 +329,7 @@ jobs:
|
||||
E2E_BACKEND_URL: http://localhost:8080
|
||||
- name: Upload E2E results
|
||||
if: always()
|
||||
uses: actions/upload-artifact@v4 # ← upgraded from v3
|
||||
uses: actions/upload-artifact@v3 # pinned per ADR-014 — Gitea Actions does not implement v4 protocol. Do NOT upgrade.
|
||||
with:
|
||||
name: e2e-results
|
||||
path: frontend/test-results/e2e/
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { cleanup, render } from 'vitest-browser-svelte';
|
||||
import { page, userEvent } from 'vitest/browser';
|
||||
import { page } from 'vitest/browser';
|
||||
import { createRawSnippet } from 'svelte';
|
||||
|
||||
vi.mock('$env/static/public', () => ({ PUBLIC_NOTIFICATION_POLL_MS: '60000' }));
|
||||
@@ -96,13 +96,13 @@ describe('Layout – user dropdown', () => {
|
||||
|
||||
it('opens dropdown on button click', async () => {
|
||||
render(Layout, { data: makeData(), children: emptySnippet });
|
||||
await page.getByRole('button', { name: /MM/ }).click();
|
||||
((await page.getByRole('button', { name: /MM/ }).element()) as HTMLElement).click();
|
||||
await expect.element(page.getByRole('link', { name: /Profil/i })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('profile link points to /profile', async () => {
|
||||
render(Layout, { data: makeData(), children: emptySnippet });
|
||||
await page.getByRole('button', { name: /MM/ }).click();
|
||||
((await page.getByRole('button', { name: /MM/ }).element()) as HTMLElement).click();
|
||||
await expect
|
||||
.element(page.getByRole('link', { name: /Profil/i }))
|
||||
.toHaveAttribute('href', '/profile');
|
||||
@@ -110,16 +110,16 @@ describe('Layout – user dropdown', () => {
|
||||
|
||||
it('logout button is in the dropdown', async () => {
|
||||
render(Layout, { data: makeData(), children: emptySnippet });
|
||||
await page.getByRole('button', { name: /MM/ }).click();
|
||||
((await page.getByRole('button', { name: /MM/ }).element()) as HTMLElement).click();
|
||||
await expect.element(page.getByRole('button', { name: /Abmelden/i })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('closes dropdown when Escape is pressed', async () => {
|
||||
render(Layout, { data: makeData(), children: emptySnippet });
|
||||
const btn = page.getByRole('button', { name: /MM/ });
|
||||
await btn.click();
|
||||
const btnEl = (await page.getByRole('button', { name: /MM/ }).element()) as HTMLElement;
|
||||
btnEl.click();
|
||||
await expect.element(page.getByRole('link', { name: /Profil/i })).toBeInTheDocument();
|
||||
await userEvent.keyboard('{Escape}');
|
||||
btnEl.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }));
|
||||
await tick();
|
||||
await expect.element(page.getByRole('link', { name: /Profil/i })).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
@@ -44,7 +44,8 @@ export default defineConfig({
|
||||
thresholds: {
|
||||
lines: 80,
|
||||
functions: 80,
|
||||
branches: 80,
|
||||
// branches at 75 — long-tail parent/child accounting grind; see docs/adr/013-client-branches-coverage-threshold.md and #496
|
||||
branches: 75,
|
||||
statements: 80
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user