Skip to content

Closes #22093: Run coverage only on a single matrix entry#22117

Merged
pheus merged 1 commit intomainfrom
22093-enable-coverage-calculation-for-only-a-single-test-matrix
May 5, 2026
Merged

Closes #22093: Run coverage only on a single matrix entry#22117
pheus merged 1 commit intomainfrom
22093-enable-coverage-calculation-for-only-a-single-test-matrix

Conversation

@pheus
Copy link
Copy Markdown
Contributor

@pheus pheus commented May 5, 2026

Fixes: #22093

Add explicit CI job names showing the Python and Node versions, with the coverage job clearly marked in the GitHub Actions UI.

Run coverage only for the designated coverage matrix entry to avoid redundant coverage collection and reporting across the full test matrix.

Also add the YAML document marker and clean up trailing whitespace.

@pheus pheus force-pushed the 22093-enable-coverage-calculation-for-only-a-single-test-matrix branch from ab88ee4 to a2c9cdb Compare May 5, 2026 16:23
@pheus
Copy link
Copy Markdown
Contributor Author

pheus commented May 5, 2026

@claude /review Only report on issues of potential concern; we don't need commentary on anything that doesn't require further consideration.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

Claude finished @pheus's task in 2m 39s —— View job


Code Review

One potential concern:

Coverage accuracy with --parallel test runner

The "Run tests with coverage" step uses Django's --parallel flag:

- name: Run tests with coverage
  if: ${{ matrix.coverage }}
  run: >-
    coverage run --source="netbox/"
    netbox/manage.py test netbox/ --parallel

When Django's parallel test runner is active, test work is distributed across subprocess workers (via multiprocessing). coverage run only instruments the parent process — worker processes are forked/spawned without coverage tracing. This means a significant portion of test execution goes unmeasured, likely producing a materially incomplete coverage report.

Additionally, there is no coverage combine step between "Run tests with coverage" and "Show coverage report". If any subprocess coverage files are written (e.g. via COVERAGE_PROCESS_START), they won't be merged before reporting.

To get accurate coverage, one of:

  1. Drop --parallel from the coverage step so all tests run in-process.
  2. Configure subprocess coverage (COVERAGE_PROCESS_START) and add a coverage combine step before coverage report.

Option 1 is simpler but slower; option 2 preserves parallel speed at the cost of extra configuration.


Everything else — the matrix include strategy, the conditional step expressions, the job naming, and the YAML cleanup — looks correct and is not a concern.

@pheus
Copy link
Copy Markdown
Contributor Author

pheus commented May 5, 2026

This PR keeps that behavior unchanged and only limits the existing coverage run to a single matrix entry, which is the scope of #22093. So while the coverage accuracy concern may be real, I don’t think this PR introduces the issue or should expand to solve it.

I’ll open a separate housekeeping issue to track improving coverage collection for parallel test runs.

@pheus pheus marked this pull request as ready for review May 5, 2026 16:49
@pheus pheus requested review from a team and bctiemann and removed request for a team May 5, 2026 16:50
Add explicit CI job names showing the Python and Node versions, with the
coverage job clearly marked in the GitHub Actions UI.

Run coverage only for the designated coverage matrix entry to avoid
redundant coverage collection and reporting across the full test matrix.

Also add the YAML document marker and clean up trailing whitespace.

Fixes #22093
@pheus pheus force-pushed the 22093-enable-coverage-calculation-for-only-a-single-test-matrix branch from a2c9cdb to 34f74ab Compare May 5, 2026 16:53
@pheus pheus removed the request for review from bctiemann May 5, 2026 16:57
@pheus pheus merged commit 7e9eac5 into main May 5, 2026
11 checks passed
@pheus pheus deleted the 22093-enable-coverage-calculation-for-only-a-single-test-matrix branch May 5, 2026 17:06
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.

Enable coverage calculation for only a single test matrix entry

2 participants