fix(coverage): collect coverage from multiprocess child processes#1123
fix(coverage): collect coverage from multiprocess child processes#1123
Conversation
1e348b5 to
792ea97
Compare
There was a problem hiding this comment.
Pull request overview
This PR implements coverage collection for child processes spawned using the third-party multiprocess library. Since OpenWPM uses multiprocess (a dill-based fork of stdlib multiprocessing) rather than the standard library, coverage.py's built-in concurrency settings don't automatically instrument child processes. Additionally, multiprocess uses os._exit() which bypasses atexit handlers, preventing coverage data from being saved normally.
Changes:
- Added a pytest fixture to set
COVERAGE_PROCESS_STARTandCOVERAGE_FILEenvironment variables during coverage runs - Implemented a template method pattern in the Process wrapper, with
run()handling coverage startup/shutdown and subclasses overridingrun_impl() - Updated BrowserManager to use the common Process wrapper and override
run_impl()instead ofrun()
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/conftest.py | Added session-scoped autouse fixture to configure coverage environment variables for child processes |
| openwpm/utilities/multiprocess_utils.py | Implemented template method pattern with coverage.process_startup() at the start and explicit cov.stop()/cov.save() in finally block; added run_impl() method for subclasses |
| openwpm/browser_manager.py | Changed BrowserManager to import Process from multiprocess_utils and renamed run() to run_impl() to work with the template method pattern |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1123 +/- ##
===========================================
+ Coverage 37.32% 56.24% +18.91%
===========================================
Files 35 36 +1
Lines 3475 3540 +65
===========================================
+ Hits 1297 1991 +694
+ Misses 2178 1549 -629 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
OpenWPM uses the third-party multiprocess library (not stdlib multiprocessing), so coverage.py built-in concurrency and patch settings do not instrument child processes. Additionally, multiprocess uses os._exit() after Process.run(), which skips atexit handlers and prevents coverage from saving data. Fix by: - Calling coverage.process_startup() at the start of each child process - Explicitly saving coverage data in a finally block before os._exit() - Using a template method pattern (run/run_impl) so coverage logic lives in one place in the Process wrapper - Adding a pytest fixture to set COVERAGE_PROCESS_START and COVERAGE_FILE environment variables during test runs - Having BrowserManager use the common Process wrapper and override run_impl() instead of run()
2c281ad to
22e5e8f
Compare
Summary
multiprocesslibrary (adill-based fork of stdlibmultiprocessing), so coverage.py's built-inconcurrencyandpatchsettings don't instrument child processesmultiprocessusesos._exit()afterProcess.run(), which skips atexit handlers and prevents coverage from saving datacoverage.process_startup()and explicitcov.stop()/cov.save()in theProcesswrapper, using a template method pattern (run/run_impl) so the logic lives in one placeCOVERAGE_PROCESS_STARTandCOVERAGE_FILEenv vars during coverage runsProcesswrapper and overridesrun_impl()instead ofrun()Fixes #1121