Skip to content

fix(e2e): traces really are always captured now (completes DEC-073) - #51

Merged
omar-nahhas merged 1 commit into
mainfrom
fix/dec-073-traces-always-on
Aug 4, 2026
Merged

fix(e2e): traces really are always captured now (completes DEC-073)#51
omar-nahhas merged 1 commit into
mainfrom
fix/dec-073-traces-always-on

Conversation

@omar-nahhas

Copy link
Copy Markdown
Contributor

v2.1.4 flipped the trace default to on in playwright-base.config.ts and I reported DEC-073 as delivered. It was not. Two other settings still forced retain-on-failure, and both took precedence over the one I changed.

  1. The reusable workflow hardcoded
    BDD_TRACE: ${{ inputs.capture-all-traces && 'on' || 'retain-on-failure' }}
    — so every consumer run explicitly passed retain-on-failure into the container.
  2. The BDD suite has its own config, e2e/bdd/playwright.config.ts — the suite consumers actually run via run-bdd.sh — whose default was also retain-on-failure. Same duplicated-setting pattern as the collector: I fixed one copy and missed the others.

Evidence this is real

All four consumers, now on v2.1.4, fully green:

repo collected
chat-comments 0 trace(s), 12 screenshot(s), 12 video(s)
diff-visualizer 0 trace(s), 11 screenshot(s), 11 video(s)
mermaid-diagrams 0 trace(s), 9 screenshot(s), 9 video(s)
share-chat 0 trace(s), 4 screenshot(s), 4 video(s)

Screenshots and videos flow (the v2.1.4 collector fix landed), traces are still zero on green runs — exactly the defect DEC-073 was written to eliminate.

Change

Replaces the boolean flip with a trace-mode string input (default on, accepts any Playwright mode), so there is a real escape hatch for disk-pressure cases without silently reverting the decision. capture-all-traces is kept so existing callers keep working, but it can no longer suppress traces.

Merging this needs a v2.1.5 tag to reach consumers — the workflow change flows immediately via @main, but the BDD config lives in the pinned submodule.

v2.1.4 flipped the trace default to "on" in playwright-base.config.ts and I
reported DEC-073 as delivered. It was not: two other settings still forced
retain-on-failure and both took precedence over the one I changed.

1. The reusable workflow hardcoded
     BDD_TRACE: ${{ inputs.capture-all-traces && "on" || "retain-on-failure" }}
   so every consumer run explicitly passed retain-on-failure into the container.
2. The BDD suite has its OWN config (e2e/bdd/playwright.config.ts) — the suite
   consumers actually run via run-bdd.sh — whose default was also
   retain-on-failure. Same duplicated-setting pattern as the collector: I fixed
   one copy and missed the others.

Proven by share-chart sync PR #19 on v2.1.4: "copied 0 trace(s), 4 screenshot(s),
4 video(s)" — screenshots and videos flowed (the collector fix landed) while
traces did not, on a fully green run.

Replaces the boolean capture-all-traces flip with a trace-mode string input
(default "on", accepts any Playwright mode) so there is a real escape hatch for
disk-pressure cases without silently reverting the decision.
capture-all-traces is kept so existing callers keep working, but it can no
longer suppress traces.
@omar-nahhas
omar-nahhas merged commit cce0578 into main Aug 4, 2026
1 check passed
@omar-nahhas
omar-nahhas deleted the fix/dec-073-traces-always-on branch August 4, 2026 01:13
@omar-nahhas omar-nahhas mentioned this pull request Aug 4, 2026
omar-nahhas added a commit that referenced this pull request Aug 4, 2026
Cuts the release carrying the DEC-073 completion (#51). Consumers pin the devkit
submodule to a release tag, and the BDD suite config — where the surviving
retain-on-failure default lived — ships inside that submodule, so the trace fix
only reaches the fleet once this tag exists.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant