[Documentation] Add test coverage guidance for integration tests#967
Conversation
This update adds a new section to the troubleshooting documentation explaining how to collect code coverage from worker subprocesses when using executorlib. It also adds a link to this new section in the main README. The solution involves configuring coverage.py with the subprocess patch and using 'coverage combine' to merge results from the main process and workers. Co-authored-by: jan-janssen <3854739+jan-janssen@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughDocumentation update adding a new troubleshooting section explaining how to configure Coverage.py to track code coverage in integration tests that use executorlib's subprocess execution, with corresponding navigation link added to README. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds documentation guidance for capturing coverage from integration tests when executorlib executes functions in subprocesses, addressing issue #965.
Changes:
- Document how to enable coverage collection for
executorlibworker subprocesses indocs/trouble_shooting.md. - Add a README link to the new troubleshooting section.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docs/trouble_shooting.md | Adds a new “Test Coverage for Integration Tests” section with coverage configuration and commands. |
| README.md | Adds a bullet link pointing to the new troubleshooting section in the hosted docs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| To collect coverage from both the main process and executorlib subprocesses, enable the subprocess patch in your project configuration: | ||
| ```toml | ||
| [tool.coverage.run] |
There was a problem hiding this comment.
The coverage config snippet enables the subprocess patch, but it doesn’t mention enabling parallel data files. Without parallel = true (or an equivalent mechanism), multiple worker processes can end up writing to the same .coverage data file and coverage combine may be ineffective. Consider adding parallel = true to the [tool.coverage.run] example alongside patch = ["subprocess"].
| To collect coverage from both the main process and executorlib subprocesses, enable the subprocess patch in your project configuration: | |
| ```toml | |
| [tool.coverage.run] | |
| To collect coverage from both the main process and executorlib subprocesses, enable the subprocess patch and parallel data files in your project configuration: | |
| ```toml | |
| [tool.coverage.run] | |
| parallel = true |
|
|
||
| Then execute: | ||
| ```bash | ||
| coverage run -m unittest discover |
There was a problem hiding this comment.
The command example uses unittest discover without specifying a start directory. In this repo’s own coverage configuration (pyproject.toml), discovery is run with an explicit tests start directory; making the docs example similarly explicit (e.g., unittest discover tests / -s tests) avoids accidentally discovering non-test modules when invoked from a different working directory.
| coverage run -m unittest discover | |
| coverage run -m unittest discover tests |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/trouble_shooting.md`:
- Around line 39-42: In the [tool.coverage.run] block update the patch setting
to a string instead of an array: replace the current patch = ["subprocess"] with
patch = subprocess so the coverage configuration uses the string-valued patch
key (i.e., change the patch entry under [tool.coverage.run] from an array to the
bare subprocess value).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fcc98b4d-237c-4a2b-aaae-5e3598843533
📒 Files selected for processing (2)
README.mddocs/trouble_shooting.md
| ```toml | ||
| [tool.coverage.run] | ||
| patch = ["subprocess"] | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does coverage.py support a "patch" configuration option for subprocess tracking? What is the correct configuration to track subprocess execution in coverage.py?
💡 Result:
Yes, coverage.py supports a "patch" configuration option under the [run] section for subprocess tracking. The correct configuration to track subprocess execution is: [patch] subprocess = true This automatically sets parallel = true, so use coverage combine before reporting. It works for processes created with subprocess, os.system, or execv/spawnv functions (ensuring COVERAGE_PROCESS_START is passed for env variants). Example .coveragerc: [run] patch = subprocess Then: coverage run ..., coverage combine, coverage report. For multiprocessing, use concurrency = multiprocessing instead.
Citations:
- 1: https://coverage.readthedocs.io/en/latest/subprocess.html
- 2: https://coverage.readthedocs.io/en/latest/config.html
Fix TOML syntax for subprocess tracking configuration.
The correct coverage.py configuration is patch = subprocess (not a list). Change line 41 to:
[tool.coverage.run]
patch = subprocessThe patch option should be a string value, not an array. This enables coverage tracking for subprocesses created via the subprocess module.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/trouble_shooting.md` around lines 39 - 42, In the [tool.coverage.run]
block update the patch setting to a string instead of an array: replace the
current patch = ["subprocess"] with patch = subprocess so the coverage
configuration uses the string-valued patch key (i.e., change the patch entry
under [tool.coverage.run] from an array to the bare subprocess value).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #967 +/- ##
=======================================
Coverage 94.15% 94.15%
=======================================
Files 39 39
Lines 2089 2089
=======================================
Hits 1967 1967
Misses 122 122 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jan-janssen
deleted the
integrate-coverage-documentation-13437585165890297203
branch
Integrates the explanation for collecting code coverage from executorlib subprocesses into the documentation, as discussed in issue #965. Updated
docs/trouble_shooting.mdwith instructions andREADME.mdwith a link to the new section.PR created automatically by Jules for task 13437585165890297203 started by @jan-janssen
Summary by CodeRabbit