Skip to content

ci: dedicated pytest --cov job to take coverage off the ansible-test fork path #587

Open

Description

@allenrobel

Follow-up to #585 / #586. This is the "dedicated coverage job" item from #585's follow-ups, promoted to its own issue so the trade-offs can be discussed against real numbers.

Where #586 leaves us

#586 gates coverage to one units matrix cell. Two CI runs of that PR, identical code and commands:

develop (before) #586 run 1 #586 run 2
Non-coverage cell, total 13m – 25m ~2m ~2m
Non-coverage cell, pytest itself 7m 10s 68s 68s
Coverage cell: Run unit tests 12m 33s 11m 39s 15m 14s
Coverage cell: Generate coverage report (combine) 9m 05s 7m 10s 12m 31s
Coverage cell, total 22m 08s 19m 21s 28m 43s
Whole run, runner-minutes 309 96 ~105

Runs: 35527908847 (develop), 35763080776, 35765703701.

Two things stand out:

  1. Coverage tracing under ansible_forked is the expensive part, not pytest. The same 4,994 tests take 68s without coverage and 11–15 min with it. The combine then adds another 7–12 min because ansible-test forks per test and writes ~5,000 data files. Coverage in one ansible-test cell costs roughly 20–30 runner-minutes for work that a single process would do in about two.
  2. Variance is large and now sets the wall clock. The coverage cell swung 19m → 29m between two runs of the same commit, and it is the only thing left on the units stage's critical path. Every other cell is done at 2 minutes. Reviewers wait on that one cell.

Proposal

Add a coverage job that runs plain pytest with pytest-cov, outside ansible-test, and drop --coverage from the units matrix entirely:

  coverage:
    name: Unit Test Coverage
    needs:
      - ansible-galaxy-importer
    runs-on: ubuntu-latest
    permissions:
      contents: read
      id-token: write  # Codecov OIDC upload
    steps:
      - uses: actions/checkout@v6
        with:
          path: ansible_collections/cisco/nd
      - uses: actions/setup-python@v6
        with:
          python-version: "3.13"
      - run: pip install https://github.com/ansible/ansible/archive/stable-2.19.tar.gz pytest pytest-ansible pytest-cov -r ansible_collections/cisco/nd/tests/unit/requirements.txt
      - run: pytest --ansible-unit-inject-only --cov=plugins --cov-branch --cov-report=xml tests/unit
        working-directory: ansible_collections/cisco/nd
        env:
          ANSIBLE_COLLECTIONS_PATH: ${{ github.workspace }}
      - uses: codecov/codecov-action@v7
        with:
          files: ansible_collections/cisco/nd/coverage.xml
          flags: unit
          use_oidc: true
          fail_ci_if_error: true

This is the same invocation developers run locally today (pytest --ansible-unit-inject-only with ANSIBLE_COLLECTIONS_PATH set), so the CI number is reproducible on a laptop.

Pros

  • Removes the 19–29 min tail from the units stage. With no coverage cell, the units stage finishes when the slowest 2-minute cell does. Expected coverage job time: pytest in one process under tracing, no fork, no combine, so a few minutes at most.
  • Removes the variance. Fork-per-test under tracing is the part that swung 10 minutes between runs. A single-process run has far less to vary.
  • Another ~20 runner-minutes per run. The coverage cell costs 19–29 min; the replacement should cost 3–5.
  • Branch coverage (--cov-branch), which ansible-test does not enable. Meaningful for the state-machine and orchestrator code.
  • Local parity. Same command as ndpytest, same environment shape, same number.

Cons

  • A second test runner in CI. The job needs its own Python, ansible-core, pytest-ansible, pytest-cov, pydantic, and the ansible_collections/cisco/nd directory layout. All solved for local runs, but it is a second recipe that can drift from what ansible-test units does (different pytest config, no ansible_forked isolation, no default-test-container).
  • Path mapping to Codecov. ansible-test rewrites coverage paths to the collection root. Plain pytest-cov reports paths relative to the checkout, which is why the sketch checks out into ansible_collections/cisco/nd and hands Codecov that path. If this is wrong, Codecov silently shows 0% for plugins/. One run will tell.
  • The number will move. Fork-per-test attributes module-level code to every test that imports it; a single process counts it once. Totals will be close to the 87.68% Codecov shows today, not identical. Worth saying once in the PR so nobody chases it.
  • Duplicate red on a real test failure. A broken test fails this job and 12 matrix cells. Harmless, but this job should not become a required status check.
  • -n auto is a separate risk. Our tests are designed to be isolated (ResponseGenerator fixtures, unique fixture keys per test), but parallel workers outside ansible_forked would be the first time that isolation is actually exercised. Land without it first; add it in its own PR.

Weighing them

Before #586 the case for this job was weak: coverage cost was spread across 12 cells and no single cell stood out. After #586 the evidence is one cell that costs 20–30 runner-minutes, varies by 50% run to run, and is the only thing anyone waits on. The cons are all one-time setup or things a first run will expose. Recommendation: do it, in this order:

  1. Add the coverage job as described, leave --coverage on the one units cell for one PR so both numbers land on the same commit and Codecov's path mapping can be checked side by side.
  2. Once the new job's report looks right on Codecov, drop coverage: true and the gated steps from the units job.
  3. Consider -n auto afterwards, separately.

Open questions

  • Is ~87% branch-aware coverage from a single process acceptable as "the" number, or does anyone rely on the ansible-test per-fork semantics?
  • Should the coverage job pin stable-2.19 / 3.13 like the current coverage cell, or follow the newest supported pair automatically?
  • Does anyone want the job to publish the HTML report as an artifact for local browsing?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

enhancementNew feature or requesttestsTests added/updated/modified

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions