Skip to content

CI: measure coverage in Python subprocesses via patch = subprocess - #1953

Merged
CodyCBakerPhD merged 1 commit into
masterfrom
claude/sweet-franklin-hj00ck
Oct 7, 2026
Merged

CodyCBakerPhD merged 1 commit into
masterfrom
claude/sweet-franklin-hj00ck

Conversation

@CodyCBakerPhD

@CodyCBakerPhD CodyCBakerPhD commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #1951, modeled on datalad/datalad#7952.

Some tests run dandi as a subprocess (test_helptext.py, test_shell_completion.py), so the CLI code they exercise was never recorded. coverage.py's patch = subprocess starts measurement in child Python processes. Each child writes a parallel data file (parallel = True is already set), and the coverage combine that #1951 added picks them up.

  • tox.ini: add patch = subprocess to [coverage:run]
  • pyproject.toml: raise the coverage floor from 7.0.0 to 7.10.6. patch first appeared in 7.10, and datalad picked 7.10.6 for its fixes to this feature. .coverage.* is already in .gitignore.

Effect

The gain across the full CI matrix is modest. Once all uploads were in, Codecov showed +14 lines (90.73% → 90.83%) against master. Most of the code these subprocess tests reach is already covered by in-process CliRunner tests. The main lasting value is that any future test which shells out to dandi (or another Python entry point) is counted automatically.

In isolation the mechanism is clearly visible. With coverage run -m pytest on test_helptext.py, cli/tests/test_command.py, cli/tests/test_shell_completion.py and test_organize.py, covered lines went from 3781 to 3838 (+57, in cli/command.py, utils.py, cli/cmd_shell_completion.py and _version.py). Results were the same on coverage 7.10.6 and 7.16.2.

test_no_heavy_imports checks a Python subprocess's stderr and loaded modules. It still passes, because coverage starting up in the child doesn't import numpy, h5py or pynwb.

The downsides are covered in the discussion below: Python subprocesses run slower during tests, and the test-extra floor is higher.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UkDjoCcCmBNuhFkmTyyohk

@CodyCBakerPhD CodyCBakerPhD added internal Changes only affect the internal API infrastructure Tools, build systems, CI/CD, development workflows labels Oct 6, 2026 — with Claude
@CodyCBakerPhD CodyCBakerPhD self-assigned this Oct 6, 2026
@codecov

codecov Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.83%. Comparing base (092c448) to head (c8d56c3).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1953      +/-   ##
==========================================
+ Coverage   90.73%   90.83%   +0.09%     
==========================================
  Files          92       92              
  Lines       14130    14130              
==========================================
+ Hits        12821    12835      +14     
+ Misses       1309     1295      -14     
Flag Coverage Δ
unittests 90.83% <ø> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@CodyCBakerPhD
CodyCBakerPhD added this pull request to stack #1954 October 6, 2026 21:09
Base automatically changed from claude/coverage-run-before-plugins to master October 6, 2026 21:48
Tests such as test_helptext and test_shell_completion run `dandi` as a
subprocess, so the CLI code they exercise was never recorded.  coverage's
`patch = subprocess` starts measurement in child Python processes, writing
parallel data files that the existing `coverage combine` picks up.  Bump the
floor to coverage 7.10.6, which carries fixes for this feature.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UkDjoCcCmBNuhFkmTyyohk
@yarikoptic
yarikoptic force-pushed the claude/sweet-franklin-hj00ck branch from 930685b to c8d56c3 Compare October 6, 2026 21:48
@CodyCBakerPhD
CodyCBakerPhD marked this pull request as ready for review October 6, 2026 22:49
@CodyCBakerPhD

Copy link
Copy Markdown
Contributor Author

Hmm this one doesn't seem to do 'as much' as the other, but maybe still catching a new line or two?

I leave this one up to you @yarikoptic

@yarikoptic

Copy link
Copy Markdown
Member

yeah, not sure. I wonder if thereare any downsides?

Copy link
Copy Markdown
Contributor Author

@yarikoptic, the downsides I could find, and what I checked:

How it works: coverage ≥ 7.10 installs a1_coverage.pth into site-packages along with the package. At every Python startup that file checks for COVERAGE_PROCESS_CONFIG and does nothing if it isn't set. patch = subprocess only sets that variable inside coverage run, so normal pytest/dandi use outside coverage run is unaffected. Any Python child process started during the tests inherits the variable and is measured.

Actual downsides:

  • Slower Python subprocesses during tests, since each one starts coverage tracing. On the subprocess-heavy subset I timed (helptext, command, shell_completion, organize; 76 tests) one run went from 30.3 s to 35.4 s. That's a single noisy sample. Tests that don't spawn Python are unaffected.
  • Higher minimum coverage (7.0.0 → 7.10.6), but only in the test extra. lowest-deps passes with it. It would matter only to someone, e.g. a distro packager, running the test suite against an older system coverage.

Things I checked that turned out not to be problems:

  • No stderr noise in child processes, including ones that never import dandi. That matters for test_no_heavy_imports, which asserts on a subprocess's stderr, and it still passes.
  • A child running in a different cwd still writes its data file next to the parent's, so coverage combine picks it up and nothing is left lying around.

Gain: +14 lines on the final full-matrix comparison (90.73% → 90.83%). Mostly code that only dandi ... subprocesses reach, plus any future test that shells out.


Generated by Claude Code

@yarikoptic

Copy link
Copy Markdown
Member
  • since each one starts coverage tracing.

well - that's the goal after all ;)

Gain: +14 lines on the final full-matrix comparison (90.73% → 90.83%). Mostly code that only dandi ... subprocesses reach, plus any future test that shells out.

sounds like a gain then. Lets get it

@CodyCBakerPhD
CodyCBakerPhD merged commit ad8f64d into master Oct 7, 2026
41 checks passed
@CodyCBakerPhD
CodyCBakerPhD deleted the claude/sweet-franklin-hj00ck branch October 7, 2026 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infrastructure Tools, build systems, CI/CD, development workflows internal Changes only affect the internal API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants