mirror of
https://github.com/openobserve/openobserve.git
synced 2026-11-04 08:09:27 +00:00
6dfcb8a44a2cc4e976b9c772ac3b9d54b846933f
10
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  Shrinath RaoandClaude Opus 4.8
|
43263ad45e |
test: fix regression Merge Reports failing with missing html-report (force reporters) (#12939)
## Did we introduce this? **No.** This is a pre-existing fragility in the `Merge Reports and Upload to TestDino` job of the **Playwright Regression Tests** workflow — the `unset CI` mechanism has been there since the workflow was created (#11669, 2026-05). It is unrelated to any test change. ## Symptom Job fails with exit code 2 **even when all e2e shards pass** (e.g. run [28410300804](https://github.com/openobserve/openobserve/actions/runs/28410300804/job/84183381446) — 11/11 shards green, only Merge Reports red): ``` Contents of playwright-results: -rw-r--r-- 1 runner runner 0 ... .gitkeep ls: cannot access 'playwright-results/html-report/': No such file or directory ##[error]Process completed with exit code 2. ``` ## Root cause `playwright.config.js` selects reporters by `process.env.CI`: ```js reporter: process.env.CI ? [['blob', { outputDir: 'blob-report' }]] : [['html', { outputFolder: 'playwright-results/html-report' }], ['json', { outputFile: 'playwright-results/report.json' }]] ``` The merge step depended on `unset CI` to take the html/json branch. The runner exports `CI=true`, and `unset CI` does not reliably reach the `merge-reports` subprocess — so the config re-selects the **`blob`** reporter, which writes a merged `blob-report/` and **nothing** into `playwright-results/`. The downstream `ls playwright-results/html-report/` then exits 2. ### Reproduced locally with this run's blob artifacts - `CI=true` + `merge-reports --config playwright.config.js` → writes `blob-report/report.zip`, no `playwright-results/html-report/` → `ls` exit 2 (matches CI exactly). - Reporters forced explicitly → always writes `html-report/index.html` + `report.json`, regardless of `CI`. ## Fix Stop depending on `unset CI`. Force the reporters and their output paths explicitly: ```bash PLAYWRIGHT_HTML_OUTPUT_DIR=playwright-results/html-report \ PLAYWRIGHT_HTML_OPEN=never \ PLAYWRIGHT_JSON_OUTPUT_NAME=playwright-results/report.json \ npx playwright merge-reports --reporter html,json ./all-blob-reports ``` Plus a guard that fails with a clear message (and lists `all-blob-reports/`) if the expected outputs are missing — instead of the cryptic `ls` exit 2. ## Verification (local, against this run's 10 blob artifacts) Simulating CI (`CI=true GITHUB_ACTIONS=true`): ``` merge exit=0 GUARD: PASS playwright-results/html-report/index.html (717 KB) playwright-results/report.json (587 KB, stats: expected=75 unexpected=0 flaky=0 skipped=9) blob-report/ → not created ``` Downstream `jq '.stats'` reads the produced `report.json` correctly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Shrinath RaoandClaude Opus 4.8
|
1155da8400 |
ci: per-test flaky/failed → ci_test_results + per-shard API counts → ci_test_counts (#12930)
## Why
Two CI-observability gaps, both about **per-test/per-shard counts** that
the run-level metrics couldn't show:
1. **Flaky vs failed was conflated.** "Failed Shards by Module" counted
shard-*job* failures — but a shard that **failed then passed on rerun is
flaky, not failed**. A bare count told you nothing and misclassified
recovered flakes as failures.
2. **API had no test counts at all.** API `ci_test_runs` rows carried
only run health (conclusion/duration) — `tests_total` was NULL — so the
Test Counts dashboard had nothing for API.
## What
### Per-test flaky **and** failed → `ci_test_results`
- The **merge job** parses the merged `report.json` for tests with
`status="flaky"` (failed→passed on retry) or `status="unexpected"`
(failed after retries) via the new commented
**`.github/scripts/extract-flaky.js`**, emitting **title + module +
status**.
- **report_to_openobserve** posts **one row per such test** to
**`ci_test_results`**.
- Row schema: `{run_id, repo, suite, workflow, ingest_source, status
("flaky"|"failed"), module, test_title, test_file}` — powers accurate
per-module **flaky** *and* **failed** dashboards, no misclassification.
### Per-shard API test counts → `ci_test_counts` (+ run-doc total)
- Each API pytest job emits `--junitxml` and uploads an
**`api-junit__<category>__<leg>`** artifact (category = the test shard:
integration / query_agent / regression / … ENT also cli / vortex_* /
oss_in_ent / ent_top / ent_rbac).
- **report_to_openobserve** runs
**`.github/scripts/parse-api-junit.py`**, which reads the category per
artifact and **dedups config-leg reruns by test identity** (query_agent
opt true|false, regression flag combos) so the same suite isn't counted
twice.
- The deduped **total** folds into the run doc (`ci_test_runs`,
suite=api) so API shows in the same KPI/pie panels as UI; **per-category
rows** go to the new **`ci_test_counts`** stream for the per-shard
breakdown (the API analog of UI's `ci_test_modules`).
## Notes
- **Forward-only** — historical runs never stored junit / per-test names
(only counts), so no backfill; both streams accumulate from merge.
- All **non-fatal**: any parse/ingest error only warns, never fails the
workflow. `_timestamp` unset → ingest time.
- Pairs with the same-named branch on the other repo (OSS ⇄ ENT).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
||
|
Shrinath RaoandClaude Opus 4.8
|
0a2cbdb387 |
ci: emit per-shard rows for UI runs (ci_test_runs_shards) — e2e per-shard dashboards (#12919)
Gives the **e2e/UI tabs** the same per-shard breakdown that Regression already has (Slowest Shards / Shard Duration over time / Per-Module Health). `o2-run-report.sh` already sees every `e2e /` shard job in `THIS_JOBS`; this also flattens them to one row per shard in **`ci_test_runs_shards`** (mirrors `ci_regression_shards`). API runs have no `e2e /` jobs → no-op. `_timestamp` unset → ingest time (aligned with the run doc). bash + jq verified locally. Pairs with the same-branch ENT PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Shrinath RaoandClaude Opus 4.8
|
48d8cf770c |
fix: broken UI/API metrics ingest (bash brace-default mangled jq --argjson) (#12918)
## Urgent — regression from #12917 `o2-run-report.sh` used `--argjson shards "${SHARDS:-{}}"`. In bash, `${SHARDS:-{}}` parses as **“SHARDS or `{`” plus a trailing literal `}`**, so when SHARDS was set jq received `{...}}` — invalid JSON → `payload build failed` → **no `ci_test_runs` doc ingested for any UI/API run since #12917 merged** (~1h of missing UI/API run metrics). Regression + inventory are unaffected (they don't use this script). ## Fix `SHARDS` and `BUILD_DUR` are already defaulted (`'{}'` / `null`) just above, so pass them directly: ``` --argjson build "$BUILD_DUR" --argjson shards "$SHARDS" ``` Verified locally: payload now builds (`build_duration_sec:360, shards_total:21, shards_failed:1`); `bash -n` clean. Please merge ASAP to restore UI/API reporting. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Shrinath RaoandClaude Opus 4.8
|
65296d88ab |
ci: capture build time + shard tallies (UI/API) + per-shard regression rows (#12917)
Closes the **UI ↔ Regression dashboard parity** gaps by capturing the few fields the dashboards were missing. One PR (pairs with the same-branch ENT PR). ## Changes - **`o2-run-report.sh`** (UI + API runs → `ci_test_runs`): adds - `build_duration_sec` (from the `build_binary` job) - `shards_total / shards_passed / shards_failed / shards_skipped` (from the `e2e /` shard jobs) - Null/0 for suites without them (API) — harmless. - **`playwright_regression.yml`**: after the run doc, emits **one row per shard** to a new **`ci_regression_shards`** stream (flattened from `payload.shards`) → enables per-shard duration / slowest-shard / per-module panels. ## Why The Regression tab had per-shard + build-time panels that UI/Combined couldn't match because the data wasn't captured. Now it is. ## Safety All additions are non-fatal (the report job is `continue-on-error`; shard emit is guarded). Verified locally: build/shard jq (build=360, shards total/passed/failed correct), per-shard row shape (module extracted), and YAML. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Shrinath RaoandClaude Opus 4.8
|
9bf7164e2b |
ci: capture UI (Playwright) per-test counts (ci_test_runs) (#12915)
## What Capture **UI (Playwright) per-test counts** in the `ci_test_runs` OpenObserve stream — `tests_total/passed/failed/flaky/skipped` per run. This was meant to land in #12914 but missed that squash-merge (pushed just after), so it's its own PR now. Companion: o2-enterprise#2046. ## How - `playwright.yml` merge job gains a `stats` output (from the merged `report.json`). - `report_to_openobserve` reads it and emits the test-count fields via a small generic **`EXTRA_JSON`** merge hook added to `o2-run-report.sh`. - Non-fatal as before; no-ops when secrets are unset. API (pytest) counts are not included (would need `--junitxml` + XML aggregation — separate change). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Shrinath RaoandClaude Opus 4.8
|
94f6aab74e |
ci: live retry-aware metrics for playwright, api-testing + regression (#12914)
## What Makes the per-run test dashboards **live**. Adds non-fatal `report_to_openobserve` jobs that emit **one retry-aware run-summary doc per run** to OpenObserve, following the metrics work merged earlier (regression/docgen/council). Companion PR in the other repo mirrors this. **Scope (this PR):** 1. **`playwright.yml` + `api-testing.yml`** → new report jobs → `ci_test_runs` (`suite=ui` / `suite=api`). 2. **`playwright_regression.yml`** → retry-aware reporting → `ci_regression` (OSS: augments the existing job; ENT: adds the job — was uninstrumented). ## How it works - **Shared `.github/scripts/o2-run-report.sh`** (used by playwright/api) derives the run's `conclusion` from job results, wall-clock from the jobs API, and **sums cost across all attempts**. Regression jobs do the same inline (they also emit shard/build/test fields). - Emits: `run_id`, `run_attempt`, `retries`, `was_retried`, `conclusion`, `suite`, `repo`, `branch`, `author`, `commit_sha`, `pr_number`, `trigger`, `final_duration_sec`, `total_duration_sec`, `retry_wasted_sec`. - **Dedup model:** re-runs append one doc per attempt; dashboards keep the **latest attempt per `run_id`** via `row_number() OVER (PARTITION BY run_id ORDER BY run_attempt DESC)`. Retries become an *attribute*, never duplicate rows; `total_duration_sec` on the kept row carries cumulative compute. - **Safe by construction:** `continue-on-error` + the never-fail poster — can't fail/slow a test run; skips when no tests ran; **no-ops until `O2_REPORTING_*` secrets are set**. ## Review AI review addressed (mktemp payload, `GH_TOKEN` guard, non-negative duration clamp). YAML + jq unit-tested + `actionlint` clean on the added jobs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Shrinath RaoandClaude Opus 4.8
|
bfade1f4f3 |
ci: report regression run metrics to OpenObserve (ci_regression) (#12912)
## What
Adds a CI→OpenObserve metrics sidecar to the Playwright Regression
workflow. A new terminal job `report_to_openobserve` emits **one summary
JSON document per run** into the `ci_regression` OpenObserve stream, so
we can build reliability/performance dashboards.
This is the OSS half of a cross-repo change — the companion
o2-enterprise PR (`ci/o2-reporting`) instruments the DocGen and E2E
Council engines into `ci_docgen` / `ci_council`.
## How it works (safe by construction)
- **Non-invasive:** derives per-job timings + per-shard pass/fail from
the GitHub jobs API and the merged Playwright stats. The only edit to an
existing job is one additive `stats` output on
`merge_and_upload_reports`.
- **Cannot break CI:** the job is `continue-on-error` and posts through
`.github/scripts/o2-report.sh`, which always `exit 0`s (a missing secret
/ rotated passcode / unreachable host only warns).
- **No-ops until secrets exist** — safe to merge first.
## Metrics captured
Reliability (conclusion, shard pass/fail, test pass/fail/flaky/skipped),
duration & cost (wall-clock, `runner_seconds`, build time, per-shard
durations), throughput (trigger/actor/branch).
## Required secrets (this repo + o2-enterprise)
- `O2_REPORTING_INGEST_BASE` — `https://<host>/api/<org>`
- `O2_REPORTING_AUTH` — `base64("email:passcode")`
Full schema, dashboard SQL, and setup notes:
`.github/scripts/O2-REPORTING.md`.
## Validation
- YAML parse + jq unit tests + `actionlint` clean on the added code.
- Live **HTTP 200** test-ingest into the target instance, both via raw
curl and through the poster script.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
||
|
Shrinath RaoandClaude Opus 4.8
|
0ee8a667c2 |
test: E2E Council Refiner — remediate Sentinel findings instead of dead-ending (#12876)
## Why
The E2E Council blocked **#12857** (no test PR opened) because the
post-heal **Sentinel re-audit found 2 critical quality issues** — even
though **9/10 tests passed**. The tests were **not worthless**; 2 just
*under-claimed* (the body asserted something weaker than the test name
promised):
1. `"should not persist FTS default across page reload"` — re-searches
after reload and asserts the column visible, which passes whether it
*persisted* or *re-resolved*. Never verifies non-persistence.
2. `"should clear FTS marker and re-resolve for new stream on stream
change"` — only checks "table rendered", never that FTS actually
re-resolved.
Blocking was correct (green ≠ meaningful), but a **dead-end block throws
away the other 90%**. The pipeline should **fix the problem and make it
right**, not give up. A no-PR outcome should be a *true last resort*.
## What — The Refiner (Phase 5.5)
A remediation loop **inside `verify_heal`**, after the Sentinel re-audit
and before the quality gate (**reuses the already-booted binary** — no
rebuild):
- **Only fires when needed.** If the re-audit is clean (PASS / 0
critical), the step reads the verdict, sets `ran=false`, and exits —
**no DeepSeek call, no test re-run**. Cheap on the happy path.
- **Strengthen-first.** Feeds the exact Sentinel findings to the
Refiner, which **adds/tightens assertions** so each test verifies what
its name promises → re-runs on the live binary (**green must hold**) →
re-checks assertion-integrity → re-audits. Capped at **2 rounds**.
- **Anti-laundering (the whole game).** Assertions may only be
**ADDED/strengthened — never dropped, inverted in place, or
renamed-to-dodge** a missing assertion. If strengthening goes red
because the **feature isn't wired**, it takes the honest `test.fixme` +
evidence exit (no green-wash).
- **`assert_integrity.py --strengthen`**: permits *added* negative
matchers (assert-absence is legit strengthening) while **still blocking
any dropped assertion** (removed/skipped guards + all static checks
stay). Unit-proven in the commit.
- **Quality gate** now judges the **post-refine** state; blocks only if
the Refiner couldn't keep the spec green honestly.
- **No-PR report** now phrases a block as a true last resort ("the
Refiner attempted to strengthen … but could not resolve every finding
honestly").
### Reuse path (clobber off)
`verify_heal` now also runs on **reuse runs**, and the "already covered"
short-circuit is suppressed on reuse — so **re-running `/e2e` on a
previously-BLOCKED spec re-validates + refines the existing tests into a
PR** instead of stopping at "already covered". No new tests are added on
reuse; the Refiner only strengthens what's there. (Cost: a reuse run
rebuilds even when tests were already perfect — acceptable since re-runs
are explicit; can add a "skip if a green test PR already exists" guard
later.)
## Validation
- `assert_integrity.py` unit-checked: added negative → FAIL without
`--strengthen`, PASS with it; dropped assertion → still FAIL with
`--strengthen`.
- Workflow YAML validated; step order confirmed **Sentinel re-audit →
Refiner → Quality gate**.
- Live end-to-end best validated by re-running `/e2e` on **#12857 with
clobber off** — the Refiner should strengthen the 2 flagged tests and
open the PR.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
||
|
Shrinath RaoandClaude Opus 4.8
|
900ebe67c6 |
ci(e2e-council): test-quality gate — stop certifying green-but-worthless specs (#12722)
## The problem (from the #12719 review) The pipeline's only hard gate was **"the Healer's authoritative run is green."** That objective *rewards* making a spec pass by gutting the assertions that test the feature. In #12719 the result was a suite that: - dropped the fix-query-card assertion, - asserted `count >= 0` (a tautology), - buried real assertions inside `if`s whose `else` just logs, and - **asserted the feature is absent** (`expectFixQueryCardNotVisible`) — certifying a half-built feature and set to **turn red when the feature is finished**. > A passing test and a valuable test are different things, and we only measured the first. This PR makes **"done" mean "the tests would fail if the feature were broken."** ## Deterministic backbone (no LLM) — `.github/scripts/assert_integrity.py` The most important guards don't depend on the model behaving: - **Diff-aware heal guard** — snapshot the spec's assertions *before* healing; **reject** any heal that **deleted**, **inverted** (`toBeVisible` → `not.toBeVisible`), or **skipped** assertions to reach green. - **Static anti-patterns** — tautologies (`count >= 0`, `expect(true)`) and **conditional-only** tests (every `expect` inside an `if` → the test can pass asserting nothing). - Conservative by design (only high-confidence patterns) — a gate with false positives gets disabled by humans. Restored from a **trusted ref** in verify_heal (never the PR tree's copy). ## Workflow - New **Assertion-integrity gate** after the authoritative run + a combined **Quality gate decision** (integrity **+** Sentinel verdict). `pr_back` now requires `quality_status == pass` → a green-but-worthless spec **never becomes a PR**. - The post-heal **Sentinel re-audit now feeds the blocking gate** (was advisory-only). LLM flakiness fails **open**; the deterministic gate is the hard teeth. - **`feature-incomplete`** honest exit: when a test can't pass because the feature isn't wired, the Healer parks it `test.fixme` + writes **evidence**; the workflow opens **no PR** and posts the gap (file:line) on the source PR — the single highest-value output. - `pr_back` **reconciles `test-summary.json` against the FINAL spec** (drops phantom rows) so the PR body can't claim tests that don't exist. ## Prompts - **Healer:** hard anti-weakening rules + the `feature-incomplete` honest exit. - **Sentinel:** tautology / conditional-only / negative-only-on-feature / name-mismatch as **CRITICAL** checks. - **Engineer:** no dead page-object scaffolding; ban `waitForTimeout` as a sync primitive; assertions must be real and match the test name. - **Architect:** one spec = one area (no Frankenstein specs); de-duplicate scenarios. ## Considered and deferred: mutation-sanity (extra feature-absent build) Theoretically the cleanest definition of a valuable test, but **dropped for now**: it overlaps the cheap static checks, has **no clean "feature-absent" baseline** under merged/multi-PR input, risks false positives that erode trust, and doubles the build. The static guards catch the same #12719 failures at near-zero cost; prevention (Healer can't invert + honest `feature-incomplete`) beats the expensive detector. ## Testing - `assert_integrity.py` unit-tested against tautology / conditional-only (incl. single-line) / heal-removed / heal-inverted fixtures, with no false positive on a legit `if` + unconditional `expect`. - Workflow YAML validated; analyzer compiles. - **First live run still needed** (per the project's "run the path before done" rule) — to see the gate block a real weak spec and the `feature-incomplete` comment fire. Stacked on #12716 (ENT sister flow); the diff collapses to just the quality program once #12716 lands. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |