Skip to content

fix(ci): fail tests-ok when workspace discovery fails - #1807

Open
MakerFlyFly wants to merge 1 commit into
apache:mainfrom
MakerFlyFly:fix/tests-ok-members-failure-upstream
Open

MakerFlyFly wants to merge 1 commit into
apache:mainfrom
MakerFlyFly:fix/tests-ok-members-failure-upstream

Conversation

@MakerFlyFly

@MakerFlyFly MakerFlyFly commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Require successful workspace discovery before tests-ok accepts the pytest result. Failed discovery currently skips pytest and incorrectly passes the required umbrella check.
  • Accept skipped pytest only with explicit empty batches output, supplied through the environment. Preserve successful matrices, legitimate empty selections, and the required check name.
  • Test the workflow's actual Bash assertion and diagnostics, including unavailable or environment-dropping Bash launchers, and align the workflow / .asf.yaml comments.

Type of change

  • Python package (tools/dev/ regression tests)
  • CI / dev loop (workflow gate and configuration comments)

Test plan

  • Baseline regression on dcfe0ad5: 24 failed / 23 passed. Local selector-failure injection gives the original gate exit 0 and the fixed gate exit 1; canonical empty selection still passes.
  • Linux gate module: 47 passed, zero skipped. Complete dev suite: 338 passed.
  • Dev Ruff, formatting (44 files), mypy (34 source files), and offline zizmor pass.
  • prek run --all-files --skip workspace-pytest (including whole-tree lychee) and prek run workspace-pytest --from-ref upstream/main --to-ref HEAD pass.
  • Real commit hook and Magpie pre-first-pr-check pass without bypasses; all five checklist categories pass with 0 blocking / 0 advisory items.
  • Upstream CI on d83db1002591efcefdea693335a9f8cc3da6c83b succeeds, including the Ready-event reruns: eight-batch tests, prek / lychee, zizmor, RAT, CodeQL workflow, and the independent CodeQL aggregation check. Discovery selects 52 members; all eight pytest steps execute successfully. Hosted logs record 338 passing dev tests and a successful tests-ok assertion.

Existing hosted evidence for the same gate behavior: successful eight-batch matrix on the audited fork revision and documentation-only empty-selection run. Failure-path evidence comes from local execution of the actual shell and modelled job results.

RFC-AI-0004 compliance

  • HITL / write-access discipline — upstream publication was explicitly requested by the contributor. The PR was opened as Draft and marked ready for review only after its real CI passed; the Ready-event reruns also pass.

Linked issues

Closes #1806.

Notes for reviewers

This is one concern in three files, prepared from upstream dcfe0ad562b55d4df41bc69aee1f0f8289ee205b. Public audit provenance: fork PR #19. The Bash probe checks the required environment path as well as launcher availability.

BATCHES deliberately follows the producer's existing literal [] contract. No dependency or public API is added. The small field reader documents its supported literal YAML forms.

Suggested classification: family:ci, capability:fix. The automatic labeler has also classified the dev-test substrate; manual label changes require maintainer permissions.

Generative AI disclosure

AI-generated with Codex (GPT-6). The contributor requested publication after the fork audit; this PR is submitted for maintainer review.

Require successful member discovery before accepting the pytest result.
Only accept a skipped matrix when discovery explicitly produced [].
Exercise the actual gate shell and its diagnostics, probe the required
Bash environment, and align the branch-protection explanation.

Generated-by: Codex (GPT-6)
@github-actions github-actions Bot added family:tools tools/* family:ci .github workflows, prek, validators substrate:framework-dev Tool substrate: build / validate / eval the framework itself labels Oct 25, 2026
@MakerFlyFly
MakerFlyFly marked this pull request as ready for review October 25, 2026 06:46

@Kaap10 Kaap10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Very thorough and well-engineered fix for #1806.

  • The dependency expansion (needs: [members, pytest]) and strict result assertions close the umbrella check loophole cleanly.
  • Passing BATCHES via env: rather than inline shell expansion is great security hygiene against Actions script injection.
  • The 47-case regression test in test_ci_tests_gate.py provides complete coverage across result combinations and gracefully handles non-POSIX environments via _working_bash().
  • Verified all CI workflows and matrix batches are passing cleanly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

family:ci .github workflows, prek, validators family:tools tools/* substrate:framework-dev Tool substrate: build / validate / eval the framework itself

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tests-ok passes when workspace member discovery fails

2 participants