Skip to content

fix(scripts): make plugin-test skips loud and run the pytest suite in CI - #2434

Merged
kyle-sexton merged 2 commits into
mainfrom
cursor/plugin-test-skip-visibility-63e1
Aug 12, 2026
Merged

fix(scripts): make plugin-test skips loud and run the pytest suite in CI#2434
kyle-sexton merged 2 commits into
mainfrom
cursor/plugin-test-skip-visibility-63e1

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Fixes #1313

Summary

run-plugin-tests.sh could exit 0 while the only Python suite silently skipped (SKIP: pytest not installed), making a green run indistinguishable from verified coverage. Skips are now named per suite in the summary, a --strict-skips mode turns them into failures for fully-provisioned callers, and pytest is pinned into CI so the parse_transcript suite actually runs there.

Fix

  • The runner records every suite that emitted a SKIP: line and prints a Suites with skipped coverage section naming each suite, its skip count, and the first reason; the final line now reads All executed plugin tests passed; N case(s) skipped (named above). instead of the old undifferentiated All plugin tests passed or were skipped.
  • --strict-skips treats any optional SKIP as a failure, for environments that declare the full toolchain (CI is deliberately not wired to it yet — its toolchain is also missing goimports, typos, PSScriptAnalyzer, and the deliberately-optional shfmt, so blanket strictness is a separate toolchain-completion decision).
  • .github/requirements-ci.txt pins pytest 9.1.1 and its pure-Python dependency closure (iniconfig, packaging, pluggy, pygments), one sha256 hash each, on the same reviewed Dependabot surface as the existing ruff pin — so the session-flow pytest suite runs in CI instead of skipping green.

Verification

  • Synthetic-tree run of the modified runner: default mode exits 0 and prints SKIPPED: plugins/demo/skipper.test.sh — 1 SKIP(s), first: pytest not installed for python3; --strict-skips exits 1 with the strict message; an all-green tree prints All plugin tests passed. with no skip section.
  • bash plugins/session-flow/skills/retro/scripts/parse-transcript.test.sh with pytest installed: 37 passed.
  • shellcheck and shfmt -d clean on the runner.

Related

cursoragent and others added 2 commits August 12, 2026 11:44
…strict-skips, run pytest suite in CI

A suite whose only coverage skipped (SKIP: pytest not installed) used to be
indistinguishable from a pass: exit 0, generic aggregate count, final line
'All plugin tests passed or were skipped.' Now the summary names every suite
that skipped and why, the final line distinguishes a clean pass from a pass
with skips, and --strict-skips lets a fully-provisioned caller treat any
optional skip as failure. pytest is pinned into requirements-ci.txt so the
session-flow parse_transcript suite actually runs in CI.

Fixes #1313

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
…n-tests.sh

The runner half of #1313 (the pytest CI pin landed in the previous commit;
this edit was lost to a concurrent working-tree revert before staging). The
summary now names every suite that emitted a SKIP and its first reason, the
final line distinguishes a clean pass from a pass with skipped coverage, and
--strict-skips turns any optional skip into a failure for fully-provisioned
callers.

Fixes #1313

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@cursor

cursor Bot commented Aug 12, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@claude

claude Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 54s —— View job


Security Review — PR #2434

  • Gather context (diff, changed files)
  • Review scripts/run-plugin-tests.sh changes
  • Review .github/requirements-ci.txt changes
  • Post findings

Scope: Only the two files changed in this PR — scripts/run-plugin-tests.sh and .github/requirements-ci.txt.

No security issues found.

Details of what I checked:

  • run-plugin-tests.sh: The new logic (strict_skips flag parsing, skipped_suites accumulation, and the summary printing loop) only reads test-suite output that was already being consumed pre-PR (grep '^SKIP:' on a log the runner itself creates via mktemp). Every variable that carries suite-controlled or test-output-controlled text ($t, $first_reason, $s) is used inside double-quoted contexts (echo "...", array element assignment) — no eval, no unquoted expansion, no word-splitting/globbing exposure, so a crafted SKIP: line from a test file can't escape into command execution. The new --strict-skips arg handling only inspects $1 and doesn't widen what the script executes.
  • CI wiring (not part of this diff, checked only for context): scripts/run-plugin-tests.sh is invoked from .github/workflows/ci.yml on the pull_request trigger (not pull_request_target/workflow_run), so this doesn't introduce a secrets-exposed-to-untrusted-code path.
  • .github/requirements-ci.txt: Adds pytest, iniconfig, packaging, pluggy, pygments, each pinned to an exact version with one sha256 hash, following the same pattern as the pre-existing ruff pin. CI installs this file with pip install --require-hashes (ci.yml:718), so integrity is enforced at install time (fails closed on a hash mismatch) rather than merely advisory. This is a supply-chain hardening change, not a loosened/unpinned dependency — no finding to raise here.
  • No changes to workflow permissions:, no new template-injection surface via the github context in any run: block (this PR doesn't touch workflow YAML at all).

@claude

claude Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 4m 17s —— View job


Review complete

  • Gather context (CLAUDE.md, REVIEW.md, diff)
  • Review scripts/run-plugin-tests.sh changes
  • Review .github/requirements-ci.txt changes
  • Post findings

Root CLAUDE.md is empty; REVIEW.md scopes security findings to the dedicated claude-security-review lane (present in this repo), so this review covers correctness/design/tests only.

One finding posted inline on scripts/run-plugin-tests.sh:20-24 (🟡 Nit): the new skip-aggregation/--strict-skips logic has no automated test, unlike check-silent-skips.sh — the sibling script this PR's own description analogizes to — which does have check-silent-skips.test.sh. The PR's manual "synthetic-tree" verification already exercises the right scenarios; worth turning into a real run-plugin-tests.test.sh fixture suite.

Read through, no defects found:

  • The skip-aggregation logic in scripts/run-plugin-tests.sh (opt-skip counting, first-reason extraction via ${first_reason#SKIP: }, --strict-skips flag parsing/shift) is correct against the SKIP: <reason> convention used across the plugin test suites I sampled (grep -rn 'SKIP:' plugins/).
  • --strict-skips is intentionally not wired into CI yet per the PR description (CI's toolchain is still missing goimports/typos/PSScriptAnalyzer/shfmt) — consistent with what's actually in .github/workflows/ci.yml.
  • .github/requirements-ci.txt: confirmed CI (.github/workflows/ci.yml:718-719) already runs pip install --require-hashes against this file with Python 3.14, so the tomli/exceptiongroup (py<3.11-only) and colorama (win32-only) pytest deps are correctly omitted from the closure.

Could not independently verify: the five new sha256 hashes for iniconfig/packaging/pluggy/pygments/pytest — this sandbox has no outbound network access, so I couldn't run pip download/pip install --require-hashes against PyPI myself. All hashes are well-formed (64 hex chars) and the file is on the same Dependabot-tracked pip surface as the existing ruff pin, so a wrong hash would fail loud in CI rather than silently, but I'm flagging this as author-claimed/unverified rather than confirmed.

Comment thread scripts/run-plugin-tests.sh
@github-actions

Copy link
Copy Markdown

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

@kyle-sexton
kyle-sexton merged commit b2c41eb into main Aug 12, 2026
36 checks passed
@kyle-sexton
kyle-sexton deleted the cursor/plugin-test-skip-visibility-63e1 branch August 12, 2026 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

scripts/run-plugin-tests.sh exits 0 while silently skipping the only Python suite — a green run is not evidence

2 participants