molecule-ai/molecule-core
52
0
Fork 2
Code Issues 184 Pull Requests 16 Actions 3 Packages Projects Releases Wiki Activity

test(ci): add bats test coverage for .gitea/scripts/review-check.sh #540

New Issue
Closed
opened 2026-05-11 18:47:32 +00:00 by core-devops · 0 comments
core-devops commented 2026-05-11 18:47:32 +00:00
Member
Copy Link
Copy Source

Context

Follow-up to PR #535 (RFC#324 Step 1 of 5 — qa-review + security-review workflow add). hongming-pc review #1421 nit 3.

.gitea/scripts/review-check.sh is 184 lines of CI evaluator logic — jq filter on PR review state, team-membership probe loop with strict 403-fail-closed and 404-skip cases, strict-mode toggle, debug log gating. It is the evaluator on which every PR merge gate will hang once Step 2 lands (branch-protection flip to require qa-review / approved + security-review / approved).

It currently has ZERO test coverage.

Cautionary tale

The harness-replays.yml decide-step iteration #4 saga (#476 → #497 → #499 → #528, four sequential defects across four cycles, all in a workflow with no test) is the canonical "ship a CI evaluator script untested" cost. This script is more load-bearing and the defects would be silent (gate fails open or never-fires).

Related memory: feedback_chained_defects_in_never_tested_workflows.

Acceptance

  • bats test file at .gitea/scripts/tests/review-check.bats.
  • Fixture API responses (in tests/fixtures/): pulls/{N}, pulls/{N}/reviews, teams/{id}/members/{u}.
  • Test cases (asserting exit code + key log lines):
    1. No reviews on the PR → exit 1 (gate red, awaiting non-author APPROVE).
    2. Only author self-approve → exit 1.
    3. APPROVE that is dismissed: true → exit 1.
    4. APPROVE by non-team-member → exit 1 (404 on team probe).
    5. APPROVE by team member → exit 0 (gate green).
    6. 403 on team-probe → exit 1 (internal#325 token-scope fail-closed path).
    7. REVIEW_CHECK_STRICT=1 + APPROVE with non-matching commit_id → exit 1.
    8. PR state = closed → exit 0 (closed PRs do not gate).
  • Wire into shellcheck workflow OR add a dedicated .gitea/workflows/review-check-tests.yml running on PRs that touch .gitea/scripts/review-check.sh or fixtures.
  • Local smoke (bash .gitea/scripts/tests/run.sh) listed in CONTRIBUTING.md (or equivalent) for review-check.sh edits.

Notes

  • Mock the Gitea API via a PATH-shadow curl wrapper (no real network in CI tests). A working pattern was used as a local smoke-test during PR#535 — copy it into the bats fixtures.
  • bats-core is the conventional choice (small, vendorable). Confirm it's in the runner-base image or vendor under .gitea/scripts/tests/lib/.

Tier

tier:low — quality improvement, not blocking any merge. But: this should land BEFORE RFC#324 Step 2 (branch-protection flip) so the gate's evaluator is exercised against fixtures rather than only the smoke-PR.

Owner

core-devops (workflow author) or core-qa (test-coverage owner). Either persona.

Cross-refs

  • PR #535 (RFC#324 Step 1 workflow-add)
  • hongming-pc review #1421 nit 3
  • feedback_chained_defects_in_never_tested_workflows
  • harness-replays.yml iteration #4 saga (#476/#497/#499/#528)
## Context Follow-up to PR #535 (RFC#324 Step 1 of 5 — `qa-review` + `security-review` workflow add). hongming-pc review #1421 nit 3. `.gitea/scripts/review-check.sh` is 184 lines of CI evaluator logic — `jq` filter on PR review state, team-membership probe loop with strict 403-fail-closed and 404-skip cases, strict-mode toggle, debug log gating. It is **the** evaluator on which every PR merge gate will hang once Step 2 lands (branch-protection flip to require `qa-review / approved` + `security-review / approved`). It currently has ZERO test coverage. ## Cautionary tale The `harness-replays.yml` `decide`-step iteration #4 saga (#476 → #497 → #499 → #528, four sequential defects across four cycles, all in a workflow with no test) is the canonical "ship a CI evaluator script untested" cost. This script is more load-bearing and the defects would be silent (gate fails open or never-fires). Related memory: `feedback_chained_defects_in_never_tested_workflows`. ## Acceptance - [ ] `bats` test file at `.gitea/scripts/tests/review-check.bats`. - [ ] Fixture API responses (in `tests/fixtures/`): `pulls/{N}`, `pulls/{N}/reviews`, `teams/{id}/members/{u}`. - [ ] Test cases (asserting exit code + key log lines): 1. No reviews on the PR → exit 1 (gate red, `awaiting non-author APPROVE`). 2. Only author self-approve → exit 1. 3. APPROVE that is `dismissed: true` → exit 1. 4. APPROVE by non-team-member → exit 1 (404 on team probe). 5. APPROVE by team member → exit 0 (gate green). 6. 403 on team-probe → exit 1 (`internal#325` token-scope fail-closed path). 7. `REVIEW_CHECK_STRICT=1` + APPROVE with non-matching `commit_id` → exit 1. 8. PR state = `closed` → exit 0 (closed PRs do not gate). - [ ] Wire into shellcheck workflow OR add a dedicated `.gitea/workflows/review-check-tests.yml` running on PRs that touch `.gitea/scripts/review-check.sh` or fixtures. - [ ] Local smoke (`bash .gitea/scripts/tests/run.sh`) listed in `CONTRIBUTING.md` (or equivalent) for review-check.sh edits. ## Notes - Mock the Gitea API via a PATH-shadow `curl` wrapper (no real network in CI tests). A working pattern was used as a local smoke-test during PR#535 — copy it into the bats fixtures. - `bats-core` is the conventional choice (small, vendorable). Confirm it's in the runner-base image or vendor under `.gitea/scripts/tests/lib/`. ## Tier `tier:low` — quality improvement, not blocking any merge. But: this should land BEFORE RFC#324 Step 2 (branch-protection flip) so the gate's evaluator is exercised against fixtures rather than only the smoke-PR. ## Owner `core-devops` (workflow author) or `core-qa` (test-coverage owner). Either persona. ## Cross-refs - PR #535 (RFC#324 Step 1 workflow-add) - hongming-pc review #1421 nit 3 - `feedback_chained_defects_in_never_tested_workflows` - `harness-replays.yml` iteration #4 saga (#476/#497/#499/#528)
core-devops added the tier:low label 2026-05-11 18:47:32 +00:00
core-devops self-assigned this 2026-05-11 19:23:58 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
approved
area/ci
CI/CD pipeline issues
 do-not-auto-merge
Opt out of autonomous merge-queue merging
 do-not-merge
hold from auto-merge (design review in progress)
 kind/infrastructure
Infrastructure-related issues
 merge-queue
Ready for serialized Gitea merge queue
 merge-queue-hold
Temporarily hold PR in merge queue
 needs-engineer
platform/go
Go platform test issues
 ready-to-build
release-blocker
Blocks the staging→main promotion / a release
 release-test
security
test-label-sre
tier:high
High risk per dev-sop §SOP-6 — ceo only, 24h cooldown
 tier:low
Low risk per dev-sop §SOP-6 — engineers/managers/ceo can approve
 tier:medium
Medium risk per dev-sop §SOP-6 — managers/ceo can approve
 triage-test
test
 wip
Work in progress — do not auto-merge
No Label tier:low
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent-dev-a agent-dev-b agent-pm agent-researcher agent-reviewer agent-reviewer-1 agent-reviewer-cr2 app-fe (Molecule AI · app-fe) app-lead (Molecule AI · app-lead) app-qa (Molecule AI · app-qa) claude-ceo-assistant claude-ci-reader claude-status-reaper core-be (Molecule AI · core-be) core-devops (Molecule AI · core-devops) core-fe (Molecule AI · core-fe) core-lead (Molecule AI · core-lead) core-offsec (Molecule AI · core-offsec) core-qa (Molecule AI · core-qa) core-security (Molecule AI · core-security) core-uiux (Molecule AI · core-uiux) cp-be (Molecule AI · cp-be) cp-lead (Molecule AI · cp-lead) cp-qa (Molecule AI · cp-qa) cp-security (Molecule AI · cp-security) cui (Zhanlin Cui) dev-lead (Molecule AI · dev-lead) devops-engineer documentation-specialist (Molecule AI · documentation-specialist) fullstack-engineer (Molecule AI · fullstack-engineer) godwin hongming hongming-ceo-delegated hongming-codex-laptop hongming-kimi-laptop hongming-pc2 hongming-personal infra-lead (Molecule AI · infra-lead) infra-runtime-be (Molecule AI · infra-runtime-be) infra-sre (Molecule AI · infra-sre) integration-tester (Molecule AI · integration-tester) mc-drift-bot molecule-code-reviewer plugin-dev (Molecule AI · plugin-dev) pm publish-runtime-bot pypi-publisher (Molecule AI PyPI Publisher (RFC#596)) release-manager (Molecule AI · release-manager) sdk-dev (Molecule AI · sdk-dev) sdk-lead (Molecule AI · sdk-lead) sop-drift-bot sop-tier-bot (SOP Tier-Check Bot) technical-writer (Molecule AI · technical-writer) triage-operator (Molecule AI · triage-operator)
No Assignees core-devops
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#540
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?