fix(ci): failOnFlakyTests ergänzen + falsche Audit-Aussage korrigieren - #980
Conversation
|
Warning Review limit reached
Next review available in: 1 minute You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughDie CI-Playwright-Konfiguration aktiviert einen Retry und ChangesCI-E2E-Zuverlässigkeit
Estimated code review effort: 2 (Einfach) | ~10 Minuten Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 158f3ba283
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - **`retries: 1` allein hätte das E2E-Gate geschwächt.** Playwright beendet einen Lauf mit flaky-Tests mit **Exit-Code 0**; der Retry-Report allein hält den Check nicht rot. Da alle sechs Smokes Required Checks sind, wäre eine intermittierende Regression damit zum grünen Merge-Gate geworden — das Gate wäre schwächer gewesen als vorher, nicht nur besser instrumentiert. `failOnFlakyTests: !!process.env.CI` ergänzt: Der Retry liefert weiterhin Trace und „flaky"-Ausweis, der Lauf bleibt aber rot. | ||
| - **Falsche Aussage in `docs/ci-e2e-audit.md` §9 korrigiert.** Der Text nannte einen „neuen `run-budget`-Job in CI" als „lokal verifiziert" — beides unzutreffend: es gibt keinen solchen Job, die Spec ist nicht verdrahtet, und lokal ist sie nicht grün, sondern deckt den offenen Defekt aus #978 auf. Der Pfad hat damit **keinerlei** CI-Abdeckung; genau das steht jetzt dort. | ||
| - **Auditdokument ist keine Planungsquelle mehr.** Die Empfehlungen E1–E9 werden ab sofort über GitHub Issues nachverfolgt (#978 für den Budget-Defekt, #979 als Sammel-Issue E1–E8), wie es AGENTS.md verlangt. `docs/ci-e2e-audit.md` bleibt Auditbefund und Belegkontext; bei Abweichungen gilt der Issue-Stand. | ||
| - **Branch-Protection erweitert (ehem. E5).** `Backend PR smoke gate` und `Frontend PR smoke gate` sind jetzt Required Checks — 17 statt 15, `strict: true` unverändert. Vorher war ein PR mit rotem Lint, Typecheck oder Unit-Test mergebar. |
There was a problem hiding this comment.
This entry says that main now has 17 required checks, while README.md:31,249, docs/STATUS.md:55,61,72,148, and ROADMAP.md:51,71 still say that main has no branch protection and treat enabling it as pending release work. These mutually exclusive states make the documented maturity and next priorities unreliable; verify the external setting and update the canonical status and roadmap in this slice.
AGENTS.md reference: AGENTS.md:L41-L45
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Erledigt in 7c71b9d — die Aussage war tatsächlich falsch.
README.md, docs/STATUS.md und ROADMAP.md behaupteten alle drei, main besitze keine Branch-Protection (Beleg: ein 404 der Protection-API). Nachgemessen:
{"anzahl_checks":17,"deletions":false,"enforce_admins":true,"force_pushes":false,
"linear_history":false,"required_reviews":0,"required_status_checks_strict":true}
Die Protection war schon vor diesem Slice aktiv — 15 Required Checks; dieser Slice hat sie auf 17 erweitert. Die Doku war also bereits zum Auditzeitpunkt nachweislich veraltet; genau das steht jetzt als Hinweis in STATUS.md, damit der 404-Beleg nicht erneut jemanden in die Irre führt.
| | E3 | **Fehlende Auth-Header** an den zwei Aufrufstellen aus P9 ergänzen. | Hängt an E2 — solange der Helper ohnehin nichts assertet, ändert der Header nur die Log-Zeile. Gemeinsam erledigen. | | ||
| | E4 | **`checkKeyboardNavigation` verschärfen**: statt „irgendein Tab setzt Fokus" die tatsächliche Tab-Reihenfolge gegen die DOM-Reihenfolge prüfen. | Die aktuelle Abschwächung war die dokumentierte Lösung für falsch-negative Läufe (#838, #921). Eine Verschärfung ohne neues Konzept holt die Flakiness zurück. Braucht einen eigenen Entwurf. | | ||
| | E5 | **PR-Gates als Required Checks aufnehmen.** `Backend PR smoke gate` und `Frontend PR smoke gate` laufen auf jedem PR, sind aber **nicht** required — ein PR mit rotem Lint, Typecheck oder Unit-Test ist derzeit mergebar. | Änderung an der Branch-Protection, nicht am Repository. Entscheidung des Betreibers. | | ||
| | ~~E5~~ **erledigt** | **PR-Gates als Required Checks aufnehmen.** `Backend PR smoke gate` und `Frontend PR smoke gate` liefen auf jedem PR, waren aber **nicht** required — ein PR mit rotem Lint, Typecheck oder Unit-Test war mergebar. | Am 2026-07-31 nach Freigabe umgesetzt: Branch-Protection von 15 auf 17 Required Checks erweitert, `strict: true` unverändert. Die „Check startet nie"-Falle ist ausgeschlossen — `ci.yml` hat keinen `paths`-Filter, und beide Jobs stehen auf `if: github.event_name == 'pull_request'`. | |
There was a problem hiding this comment.
Marking E5 as completed here keeps this audit functioning as a live fifth tracker despite the new disclaimer that GitHub Issues own follow-up state: future readers must still reconcile completion markers and priorities in this table against #978/#979. Freeze this section as a dated audit snapshot or remove live recommendation statuses so executable follow-up is maintained only in the permitted planning sources.
AGENTS.md reference: AGENTS.md:L73-L73
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Berechtigt — erledigt in 7c71b9d.
Der Disclaimer allein hat den Widerspruch nicht aufgelöst: solange E5 ein ~~erledigt~~ trug, war die Tabelle genau das, was AGENTS.md untersagt. §9 ist jetzt als datierte Momentaufnahme eingefroren:
- Überschrift: „Empfehlungen — Momentaufnahme vom 2026-07-31, wird nicht fortgeschrieben"
- kein Erledigungsstatus mehr in der Tabelle; die E5-Zeile nennt nur noch die damalige Begründung („Branch-Protection zu ändern brauchte eine ausdrückliche Freigabe, keine Audit-Entscheidung")
- auch der Live-Zeiger
**E9** → **#978**ist raus; die Issue-Links stehen nur noch einmal im Vorspann
Ausführbarer Folgestand liegt damit ausschließlich in #978 und #979.
| // Ein Retry NUR in CI — ausschliesslich fuer die Diagnose (Trace beider | ||
| // Versuche, "flaky"-Ausweis im Report). Lokal bleibt es bei 0, damit | ||
| // Flakiness beim Entwickeln sofort auffaellt. |
There was a problem hiding this comment.
When the first attempt fails and the retry succeeds, this configuration does not produce the claimed traces of both attempts: use.trace remains retain-on-failure at line 26, which Playwright documents as keeping traces only for failed runs, so the successful retry trace is discarded. failOnFlakyTests changes the suite result but not that retry's passed status; use retain-on-failure-and-retries or another retry-retaining mode if comparing both traces is the intended diagnostic benefit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Zutreffend — der Kommentar hat mehr versprochen, als die Konfiguration liefert. Erledigt in 7c71b9d.
Ich habe die Aussage korrigiert statt den Modus zu wechseln: bei einem flaky-Test ist der fehlgeschlagene Versuch der diagnostisch aussagekräftige, retain-on-failure-and-retries würde vor allem die Artefaktgröße erhöhen. Der Kommentar nennt jetzt genau das, inklusive der Begründung, warum der andere Modus bewusst nicht gesetzt ist. use.trace bleibt unverändert auf retain-on-failure — keine Verhaltensänderung.
Dieselbe Formulierung stand auch in docs/ci-e2e-audit.md §5 und §8 sowie im CHANGELOG.md; alle drei Stellen sind nachgezogen.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/ci-e2e-audit.md`:
- Line 126: Vereinheitliche die deutschen Anführungszeichen: In
docs/ci-e2e-audit.md im Bereich 126–126, 263–263 und CHANGELOG.md im Bereich
10–10 ersetze „flaky" durch „flaky“. In docs/ci-e2e-audit.md im Bereich 290–291
schließe „irgendein Tab setzt Fokus“ und „Check startet nie“ mit `“` statt dem
ASCII-Zeichen ab.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4a7cad84-1a9a-4fa8-a2ad-d860955af760
📒 Files selected for processing (3)
CHANGELOG.mddocs/ci-e2e-audit.mdfrontend/playwright.config.ts
- playwright.config.ts: Kommentar behauptete faelschlich Traces beider Versuche; retain-on-failure verwirft den Trace eines bestandenen Retrys. Aussage korrigiert, Verzicht auf retain-on-failure-and-retries begruendet. - ci-e2e-audit.md §9 als datierte Momentaufnahme eingefroren (kein Erledigt-Status mehr im Dokument, Nachverfolgung nur in #978/#979). - README/STATUS/ROADMAP: falsche Aussage "keine Branch-Protection" korrigiert; nachgemessen 17 Required Checks, strict, enforce_admins. - Deutsche Schlusszeichen in selbst verfassten Passagen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V6zsyxzDRymnyMeBCsUxDT
Codex-Findings zu PR #977. Alle drei zutreffend, zwei davon echte Fehler. P1 — retries: 1 allein haette das E2E-Gate GESCHWAECHT. Playwright beendet einen Lauf mit flaky-Tests mit Exit-Code 0; der "flaky"-Ausweis im Report haelt den Check nicht rot. Da alle sechs Smokes Required Checks sind, waere eine intermittierende Regression damit zum gruenen Merge-Gate geworden. failOnFlakyTests: !!process.env.CI ergaenzt. Empirisch verifiziert mit einem kuenstlich flakigen Test (Datei-Zaehler, damit der Zustand den Worker-Neustart ueberlebt): retries: 1 allein -> "1 flaky", Exit 0 + failOnFlakyTests -> "1 flaky", Exit 1 Der Retry liefert weiterhin Trace und flaky-Ausweis; die Messlatte sinkt nicht. P1 — docs/ci-e2e-audit.md war keine reine Auditdatei. Die Empfehlungen E1-E9 machten sie zur parallelen Planungsquelle neben README/STATUS/ROADMAP/Issues, was AGENTS.md verbietet. Nachverfolgung laeuft jetzt ueber #978 (Budget-Defekt) und #979 (Sammel-Issue E1-E8); das Dokument bleibt Auditbefund und Belegkontext. P2 — falsche Aussage in §9 korrigiert. Der Text nannte einen "neuen run-budget-Job in CI" als "lokal verifiziert". Beides unzutreffend: es gibt keinen solchen Job, die Spec ist nicht verdrahtet, und lokal ist sie nicht gruen, sondern deckt den offenen Defekt aus #978 auf. Der Pfad hat KEINERLEI CI-Abdeckung — das steht jetzt so dort. Ausserdem nachgezogen: E5 als erledigt markiert (Branch-Protection von 15 auf 17 Required Checks erweitert, PR-Gates aufgenommen). Verifikation: pre-push-gate.sh ALL GREEN. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V6zsyxzDRymnyMeBCsUxDT
- playwright.config.ts: Kommentar behauptete faelschlich Traces beider Versuche; retain-on-failure verwirft den Trace eines bestandenen Retrys. Aussage korrigiert, Verzicht auf retain-on-failure-and-retries begruendet. - ci-e2e-audit.md §9 als datierte Momentaufnahme eingefroren (kein Erledigt-Status mehr im Dokument, Nachverfolgung nur in #978/#979). - README/STATUS/ROADMAP: falsche Aussage "keine Branch-Protection" korrigiert; nachgemessen 17 Required Checks, strict, enforce_admins. - Deutsche Schlusszeichen in selbst verfassten Passagen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V6zsyxzDRymnyMeBCsUxDT
3288219 to
57eb081
Compare
1 participant
Arbeitet die drei Codex-Findings aus #977 ab. Alle drei zutreffend, zwei davon echte Fehler aus dem Audit-PR.
P1 —
retries: 1allein hätte das E2E-Gate geschwächtCodex hat den entscheidenden Punkt getroffen: Playwright beendet einen Lauf mit flaky-Tests mit Exit-Code 0. Der „flaky"-Ausweis steht zwar im Report, hält den Check aber nicht rot. Da alle sechs Smokes Required Checks sind, wäre eine intermittierende Regression damit zum grünen Merge-Gate geworden — das Gate wäre schwächer gewesen als vorher, nicht nur besser instrumentiert. Meine Begründung im Config-Kommentar war insofern falsch.
failOnFlakyTests: !!process.env.CIergänzt.Empirisch verifiziert mit einem künstlich flakigen Test (Datei-basierter Zähler, damit der Zustand den Worker-Neustart nach dem Fehlschlag überlebt):
retries: 1allein1 flakyretries: 1+failOnFlakyTests1 flakyDer Retry liefert weiterhin den flaky-Ausweis im Report und — via
trace: retain-on-failure— den Trace des fehlgeschlagenen Versuchs; die Messlatte sinkt nicht.P1 —
docs/ci-e2e-audit.mdwar eine parallele PlanungsquelleDie Empfehlungsliste E1–E9 machte das Dokument faktisch zum Tracker neben README/STATUS/ROADMAP/Issues, was AGENTS.md untersagt. Nachverfolgung läuft jetzt über Issues:
Das Dokument bleibt Auditbefund und Belegkontext; bei Abweichungen gilt der Issue-Stand. Anmerkung zur Einordnung: Auditdokumente mit Folgearbeiten haben im Repo Präzedenz (
docs/AUDIT_REPORT_MAY_2026.md,docs/audit-followup-issues.md,docs/audits/) — die Trennung „Befund im Dokument, Nachverfolgung im Issue" löst die Spannung sauber auf.P2 — falsche Aussage in §9 korrigiert
Der Text nannte einen „neuen
run-budget-Job in CI" als „lokal verifiziert". Beides unzutreffend: Es gibt keinen solchen Job, die Spec ist nicht verdrahtet, und lokal ist sie nicht grün — sie deckt den offenen Defekt aus #978 auf. Der Pfad hat damit keinerlei CI-Abdeckung. Genau das steht jetzt dort.Das war ein Rückstand aus einer früheren Fassung des Dokuments, den ich beim Umschwenken auf „nicht verdrahten" nicht nachgezogen hatte. Berechtigter Fund.
Außerdem nachgezogen
E5 als erledigt markiert: Branch-Protection von 15 auf 17 Required Checks erweitert (
Backend PR smoke gate,Frontend PR smoke gate),strict: trueunverändert.Verifikation
bash scripts/pre-push-gate.sh→ ALL GREEN🤖 Generated with Claude Code
https://claude.ai/code/session_01V6zsyxzDRymnyMeBCsUxDT
Summary by CodeRabbit
Verbesserungen
Dokumentation
Nachtrag 2026-07-31 — Review-Findings (7c71b9d)
CHANGELOG.md:20): README/STATUS/ROADMAP behauptetenmainhabe keine Branch-Protection. Nachgemessen: 17 Required Checks,strict: true,enforce_admins: true— und bereits vor diesem Slice 15. Alle drei Dateien korrigiert.docs/ci-e2e-audit.md): §9 als datierte Momentaufnahme eingefroren; kein Erledigungsstatus mehr im Auditdokument, Nachverfolgung nur in fix(budget): Hartes Run-Budget greift im Report-Pfad nicht (status=completed statt stopped) #978/ci: verbleibende Folgearbeiten aus dem CI-/E2E-Audit (E1, E4, E6, E8) #979.frontend/playwright.config.ts): Behauptung „Trace beider Versuche“ war mitretain-on-failurefalsch. Aussage korrigiert statt Modus gewechselt; Verzicht aufretain-on-failure-and-retriesist im Kommentar begründet. Keine Verhaltensänderung.Gate nach den Änderungen:
bash scripts/pre-push-gate.sh→ ALL GREEN.