Skip to content

test: honor capability overrides during example collection - #1451

Open
snowyukitty wants to merge 2 commits into
generative-computing:mainfrom
snowyukitty:fix/example-collection-sanity
Open

test: honor capability overrides during example collection#1451
snowyukitty wants to merge 2 commits into
generative-computing:mainfrom
snowyukitty:fix/example-collection-sanity

Conversation

@snowyukitty

@snowyukitty snowyukitty commented Jul 28, 2026

Copy link
Copy Markdown

Pull Request

Issue

Fixes #1346.

Description

test_example_collection_sanity is intended to detect regressions that silently prevent most
examples from being collected. On main at 8774ad5, however, the test collects only 7 examples
on a machine without Ollama, a GPU, or API credentials and fails its floor of 50. The result is
therefore a property of the host rather than only of the collection hooks.

This change keeps the existing floor, support-file checks, and duplicate guard while making the
sanity probe capability-independent:

  • all three example collection hooks pass their active pytest config to
    _should_skip_collection;
  • collection honours the existing aggregate and individual GPU, Ollama, API-key, and resource
    overrides;
  • skip_always, explicit skip, qualitative-in-CI, and SKIP_SLOW remain authoritative;
  • the sanity subprocess runs from the repository root with capability checks disabled, the
    repository's not slow selection preserved, vLLM excluded defensively, nested coverage
    disabled, and non-zero collection exits rejected;
  • a write-only direct-file capability probe is removed, making that path use the helper's existing
    fail-open behaviour as well;
  • deterministic tests cover available and unavailable capability gates, while direct subprocess
    tests cover module-hook wiring, exact nodeids, duplicate collection, and real CLI option
    registration.

The vLLM exclusion currently deselects no additional item because all vLLM examples are also
explicitly skip-marked. It prevents a future unskipped vLLM example from reaching
pytest_collection_finish, which can launch multiple files even during --collect-only.

Default collection without an override is unchanged. An explicit override may allow an example to
run far enough to fail when the bypassed service, hardware, or credential is genuinely absent;
that is the documented purpose of these options.

Observed collection counts in the no-service test environment:

default                  7
--ignore-ollama-check   62
--ignore-gpu-check      20
--skip-resource-checks  20
--ignore-api-key-check   7
--ignore-all-checks     75

Sibling #1345 was reported in the same two-failure baseline, but it concerns a different test and
root cause. It remains separate, so this PR does not claim that the reporter's complete baseline
becomes green.

Testing

  • Tests added to the respective file if code was changed
  • New code has 100% coverage if code was added
  • Ensure existing tests and github automation passes (a maintainer will kick off the github automation when the rest of the PR is populated)

Additional local validation:

  • focused regression file: 11 passed;
  • the same tests with inherited PYTEST_ADDOPTS="--rootdir=C:/w -m unit": 11 passed;
  • default collection nodeids are byte-identical to the audited base;
  • 14 targeted mutations covering capability gates, option registration, all three hook paths,
    support-file leakage, duplicate collection, and large collection loss were killed after their
    intermediate states were confirmed;
  • Ruff format and lint, targeted mypy (including possibly-undefined), SPDX, and
    git diff --check pass.

The full Linux pre-commit and unit matrix could not be run on the Windows review host and remains
for repository CI.

Attribution

  • AI coding assistants used

Adding a new component, requirement, sampling strategy, or tool?

If your PR adds or modifies one of the types below, check the matching box. A checklist of type-specific review items will be posted as a comment.

  • Component
  • Requirement
  • Sampling Strategy
  • Tool

NOTE: Please ensure you have an issue that has been acknowledged by a core contributor and routed you to open a pull request against this repository. Otherwise, please open an issue before continuing with this pull request.

Signed-off-by: snowyukitty <270071858+snowyukitty@users.noreply.github.com>
Assisted-by: Claude Code
Assisted-by: Codex
@snowyukitty
snowyukitty requested a review from a team as a code owner July 28, 2026 01:28
Signed-off-by: snowyukitty <270071858+snowyukitty@users.noreply.github.com>
Assisted-by: Claude Code
Assisted-by: Codex

@planetf1 planetf1 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.

Thanks for the thorough write-up and the mutation coverage — the per-flag count table is a nice touch.

One thing before merge: the original failure was on BlueVela (LSF job 1683402) and the full Linux matrix couldn't run on the Windows review host. I'll kick off an LSF job to confirm it holds in the exact environment that filed the bug. The fix should be solid there — --ignore-all-checks makes the subprocess count environment-independent, and even with CICD=1 filtering qualitative examples the floor of 50 has plenty of headroom (86 collected). Just want to close the loop on the reported platform before merging.

@planetf1

Copy link
Copy Markdown
Contributor

BlueVela result (LSF job 464348, p1-r19-n3, Linux, Python 3.12.13, no Ollama, no GPU — the exact environment from #1346): 11 passed in 31.65s. Happy to approve.

@planetf1 planetf1 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.

Ran three jobs on BlueVela (the platform from #1346):

  • Focused regression file: 11/11 passed, no-service environment
  • Fast loop (test/ -m "not qualitative"): 3,589 passed
  • Full suite with Ollama (WITH_VLLM=0): 3,619 passed — 2 OOM errors on test_groundedness_requirement are pre-existing GPU memory pressure on that node, not introduced by this PR

All good.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_example_collection_sanity fails on upstream/main

2 participants