mirror of
https://github.com/openobserve/openobserve.git
synced 2026-11-05 16:10:43 +00:00
Shrinath Rao
496c0d5842
## What **Phase 3** of the Playwright→pytest API-test migration — the anti-drift guardrail. (Phase 1 = Correlation #13788 / o2-ent#2419; Phase 2 = Alerts #13791.) > 📚 **Stacked on #13791.** Base is the Phase 2 branch so the diff shows only these two files; GitHub auto-retargets this PR to `main` once #13791 merges. ## Changes - **`tests/ui-testing/scripts/classify-specs.js`** — codifies the migration classifier. Labels each `*.spec.js` as `UI_ONLY` / `UI+SETUP` / `PURE_API` / `REVIEW` from UI markers (`page.goto`, `.click`, `locator`, `getByRole`, `toBeVisible`, page-object imports, nav helpers…) vs API markers (`page.request`, `authedRequest`, `getAuthHeaders`, `/_json`, api-helper imports). Modes: - `--all` — regenerate the inventory (summary + PURE_API/REVIEW lists) - `--check <files>` — exit 1 if any file is pure-API (the guard) - `--json` - Conservative by design: a spec is `PURE_API` only when **not one** UI marker appears, so genuine UI tests are never nagged. - **`.github/workflows/pure-api-spec-guard.yml`** — runs `--check` on the **newly-added** `*.spec.js` files in a PR and **fails** if any is pure-API (backend/API assertions with no browser). Pushes new API tests into pytest (`tests/api-testing/`) from the start. Only added files are checked, so editing an existing UI spec never trips it. ## Why The migration removed 9 pure-API specs from the browser harness (Correlation ×5, Alerts ×4). Without a guard, new API tests get authored as Playwright specs again and the drift returns. This makes the rule enforceable in CI instead of relying on reviewer memory. ## Verification - Re-ran the classifier over all **257** specs → **`PURE_API: 0`** (both phases fully removed their targets; zero drift). Sole `REVIEW` = `cleanup.spec.js`, a teardown utility (not a test). - Self-tested the guard: ✅ flags a synthetic pure-API spec (exit 1 + actionable message), ✅ passes a UI spec (exit 0). - Resolved the 6 previously-uncertain "Category C" specs → all UI or teardown; **0 additional migrations**.
56 lines
2.3 KiB
YAML
56 lines
2.3 KiB
YAML
# Anti-drift guard for the Playwright→pytest API-test migration.
|
|
#
|
|
# Fails a PR that ADDS (or renames in) a pure-API Playwright spec — backend/API
|
|
# assertions with no browser interaction. Those belong in the pytest suite
|
|
# (tests/api-testing/), not the browser harness. Only added/renamed *.spec.js
|
|
# files are checked, so edits to existing UI specs never trip it.
|
|
name: Pure-API Spec Guard
|
|
|
|
on:
|
|
pull_request:
|
|
paths:
|
|
- "tests/ui-testing/playwright-tests/**/*.spec.js"
|
|
- "tests/ui-testing/scripts/classify-specs.js"
|
|
|
|
permissions:
|
|
contents: read
|
|
|
|
jobs:
|
|
guard:
|
|
name: Flag new pure-API Playwright specs
|
|
runs-on: ubuntu-latest
|
|
steps:
|
|
- name: Checkout
|
|
uses: actions/checkout@v5
|
|
with:
|
|
fetch-depth: 0
|
|
|
|
# Everything runs in one step: the untrusted filenames from `git diff`
|
|
# stay inside a bash array and are passed to node as argv — they are NEVER
|
|
# interpolated into the script via ${{ }}, so a malicious filename like
|
|
# `x$(...).spec.js` cannot execute in the runner. BASE is the PR target
|
|
# branch name (repo-controlled), passed via env, not inlined.
|
|
- name: Check added/renamed specs for pure-API drift
|
|
env:
|
|
BASE: ${{ github.base_ref }}
|
|
run: |
|
|
git fetch --no-tags origin "$BASE"
|
|
# Added (A) or renamed (R) files under playwright-tests; --name-only
|
|
# yields the destination path for renames. Run the diff as its OWN
|
|
# command (not via process substitution) so a failure to resolve
|
|
# origin/$BASE...HEAD is fatal — otherwise the guard would fail-open
|
|
# (empty list → "nothing to check" → exit 0). classify-specs.js filters
|
|
# to *.spec.js and skips anything that no longer exists.
|
|
if ! git diff --name-only --diff-filter=AR "origin/$BASE...HEAD" \
|
|
-- tests/ui-testing/playwright-tests/ > added.txt; then
|
|
echo "::error::git diff could not resolve origin/$BASE...HEAD"
|
|
exit 1
|
|
fi
|
|
mapfile -t files < added.txt
|
|
if [ ${#files[@]} -eq 0 ]; then
|
|
echo "No added/renamed specs under playwright-tests/ — nothing to check."
|
|
exit 0
|
|
fi
|
|
printf 'Classifying added/renamed files:\n'; printf ' %s\n' "${files[@]}"
|
|
node tests/ui-testing/scripts/classify-specs.js --check "${files[@]}"
|