mirror of
https://github.com/openobserve/openobserve.git
synced 2026-11-04 00:08:55 +00:00
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>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
ce726253f8
commit
0ee8a667c2
@@ -4,10 +4,13 @@ Assertion-integrity analyzer for the E2E Council pipeline (deterministic — no
|
||||
|
||||
Two modes:
|
||||
fingerprint <spec.js> -> print a JSON fingerprint of the spec's assertions
|
||||
check <spec.js> [--baseline <fingerprint.json>] [--allow-weakening]
|
||||
check <spec.js> [--baseline <fingerprint.json>] [--allow-weakening] [--strengthen]
|
||||
-> static anti-pattern scan + (optional) diff-aware
|
||||
heal-weakening check; writes nothing, prints a JSON
|
||||
verdict, exits 2 if any CRITICAL finding.
|
||||
--strengthen (Refiner mode): permit ADDED negative
|
||||
matchers (assert-absence is valid strengthening);
|
||||
still blocks any DROP of assertions.
|
||||
|
||||
Conservative by design: only HIGH-CONFIDENCE patterns are flagged as critical, because a gate with
|
||||
false positives gets disabled by humans (the same trap that made mutation-testing a poor fit).
|
||||
@@ -166,11 +169,20 @@ def swallowed_assertion_tests(lines):
|
||||
return flagged
|
||||
|
||||
|
||||
def check(spec_path, baseline=None, allow_weakening=False):
|
||||
def check(spec_path, baseline=None, allow_weakening=False, strengthen=False):
|
||||
"""Run the static anti-pattern scan (tautologies, conditional-only assertions) and, when a
|
||||
baseline is given, the diff-aware heal-weakening guard. Returns a verdict dict; never raises on
|
||||
a malformed/partial baseline — missing baseline keys fall back to the current value (skip that
|
||||
comparison) rather than crashing the gate."""
|
||||
comparison) rather than crashing the gate.
|
||||
|
||||
`strengthen` (Refiner mode): the Refiner STRENGTHENS weak/mismatched tests, which can legitimately
|
||||
(a) ADD negative matchers (assert a column is *absent* before a re-search) and (b) park a genuinely
|
||||
unwired test as fixme. So in strengthen mode we permit a negatives rise ONLY when expect() ALSO rose
|
||||
(an addition, not an in-place flip — a flip with flat expect() is still flagged), and permit a
|
||||
skip/fixme rise (honest feature-incomplete). `heal-removed-assertions` (expect() may never drop) and
|
||||
EVERY static anti-pattern check (tautology, conditional-only, swallowed, no-runnable) STILL apply —
|
||||
so strengthen mode cannot be used to drop assertions or green-wash. (NOTE: do not also pass
|
||||
--allow-weakening in strengthen mode; that disables the removed guard and defeats the point.)"""
|
||||
src = open(spec_path, encoding='utf-8').read()
|
||||
lines = src.split('\n')
|
||||
findings = []
|
||||
@@ -207,10 +219,18 @@ def check(spec_path, baseline=None, allow_weakening=False):
|
||||
if fp['expects'] < base_expects:
|
||||
findings.append({'severity': 'critical', 'rule': 'heal-removed-assertions',
|
||||
'detail': f"expect() count dropped {base_expects} -> {fp['expects']} during heal"})
|
||||
if fp['negatives'] > base_negatives:
|
||||
# Inverted = a positive flipped to a negative IN PLACE. In --strengthen mode (Refiner) ADDING a
|
||||
# negative is legitimate, but ONLY if expect() ALSO rose — that proves the negative is a new
|
||||
# assertion, not a flip. A negative rise with no new expect() is an in-place flip → still flag,
|
||||
# even in strengthen mode. (Outside strengthen mode any negative rise is flagged, as before.)
|
||||
if fp['negatives'] > base_negatives and (not strengthen or fp['expects'] <= base_expects):
|
||||
findings.append({'severity': 'critical', 'rule': 'heal-inverted-assertions',
|
||||
'detail': f"negative matchers rose {base_negatives} -> {fp['negatives']} during heal (positive flipped to negative?)"})
|
||||
if fp['skips'] > base_skips:
|
||||
'detail': f"negative matchers rose {base_negatives} -> {fp['negatives']} (positive flipped to negative in place — expect() did not rise)"})
|
||||
# Rising skip/fixme is silent-skipping during heal — blocked. The Refiner's HONEST
|
||||
# feature-incomplete path (park a test as fixme, assertion body kept intact) is permitted ONLY in
|
||||
# --strengthen mode; expect() still cannot drop (guarded above) and an all-fixme spec is still
|
||||
# caught by no-runnable-tests, so this can't be used to green-wash.
|
||||
if fp['skips'] > base_skips and not strengthen:
|
||||
findings.append({'severity': 'critical', 'rule': 'heal-skipped-tests',
|
||||
'detail': f"skip/fixme rose {base_skips} -> {fp['skips']} during heal (use the feature-incomplete path, not silent skipping)"})
|
||||
|
||||
@@ -226,13 +246,15 @@ def main():
|
||||
if mode == 'fingerprint':
|
||||
print(json.dumps(fingerprint(open(spec, encoding='utf-8').read())))
|
||||
return
|
||||
baseline, allow = None, False
|
||||
baseline, allow, strengthen = None, False, False
|
||||
args = sys.argv[3:]
|
||||
if '--baseline' in args:
|
||||
baseline = json.load(open(args[args.index('--baseline') + 1]))
|
||||
if '--allow-weakening' in args:
|
||||
allow = True
|
||||
result = check(spec, baseline, allow)
|
||||
if '--strengthen' in args:
|
||||
strengthen = True
|
||||
result = check(spec, baseline, allow, strengthen)
|
||||
print(json.dumps(result, indent=2))
|
||||
sys.exit(2 if result['verdict'] == 'FAIL' else 0)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user