Follow requirement to allow finalizers to run #108
Conversation
Tests on some platforms and Python versions (Python 3.7, 3.8, and 3.9 on Ubuntu and macOS) were hanging with coverage==6.3. See nedbat/coveragepy#1310 and nedbat/coveragepy#1312 for some discussion about this issue. Some people have pinned coverage==6.2 as a temporary workaround. As discussed in #1312, coverage fixes this issue in 00da68ef1a0b4d43c003babae0cb8f91beaf06d2 by only setting signal handlers from the main thread; I confirmed that using the latest commit of coverage (aad5ece47bf12bceff4296516f23171a06b34bb5) fixes the issue of tests hanging without any modification to our code. However, according to the pytest-cov documentation (https://pytest-cov.readthedocs.io/en/latest/subprocess-support.html#if-you-use-multiprocessing-pool), when using `multiprocessing.Pool`, we are supposed to ensure that `join()` is called on the pool, rather than `terminate()`. Following this requirement makes the tests work with coverage==6.3 as well.
Codecov Report
@@ Coverage Diff @@
## master #108 +/- ##
==========================================
+ Coverage 86.50% 86.59% +0.09%
==========================================
Files 12 12
Lines 956 955 -1
Branches 163 162 -1
==========================================
Hits 827 827
+ Misses 115 114 -1
Partials 14 14
Continue to review full report at Codecov.
|
|
lgtm, but will defer final review to @cgnorthcutt It's a bit weird that we need to change our source code to accommodate the coverage tests though, hopefully that doesn't happen again going forward... |
|
If we don't care about coverage on Windows, we don't need this. Maybe that's a fine trade-off, and we can ignore this PR (and pin an old version of coverage until a new version is released). #109 |
Tests on some platforms and Python versions (Python 3.7, 3.8, and 3.9 on Ubuntu and macOS) were hanging with coverage==6.3.
See nedbat/coveragepy#1310 and nedbat/coveragepy#1312 for some discussion about this issue. Some people have pinned coverage==6.2 as a temporary workaround. As discussed in #1312, coverage fixes this issue in 00da68ef1a0b4d43c003babae0cb8f91beaf06d2 by only setting signal handlers from the main thread; I confirmed that using the latest commit of coverage (aad5ece47bf12bceff4296516f23171a06b34bb5) fixes the issue of tests hanging without any modification to our code.
However, according to the pytest-cov documentation (https://pytest-cov.readthedocs.io/en/latest/subprocess-support.html#if-you-use-multiprocessing-pool), when using
multiprocessing.Pool, we are supposed to ensure thatjoin()is called on the pool, rather thanterminate(). Following this requirement makes the tests work with coverage==6.3 as well.