Drop in coverage with 6.2 vs 6.1.2

[jcpunk]

Describe the bug When switching to Coverage.py 6.2 the multiprocess detection doesn't seem to work as well as 6.1.2

To Reproduce How can we reproduce the problem? Please be specific. Don't just link to a failing CI job.

Checkout https://github.com/HEPCloud/decisionengine Run the unit tests with 6.1.2, note coverage in the 95% range. ( https://github.com/HEPCloud/decisionengine/runs/4796168815?check_suite_focus=true ) Run the unit tests with 6.2, note coverage in the 85% range. ( https://github.com/HEPCloud/decisionengine/runs/4795949215?check_suite_focus=true )

Answer the questions below:

1. What version of Python are you using? 3.6 and 3.10
2. What version of coverage.py shows the problem? The output of coverage debug sys is helpful.

Regression noted in 6.2

3. What versions of what packages do you have installed? The output of pip freeze is helpful.

Package Version Editable project location

---

alabaster 0.7.12 amqp 5.0.9 astroid 2.9.3 Babel 2.9.1 binaryornot 0.4.4 boolean.py 3.8 cached-property 1.5.2 certifi 2021.10.8 cfgv 3.3.1 chardet 4.0.0 charset-normalizer 2.0.10 cheroot 8.6.0 CherryPy 18.6.1 coverage 6.2 decisionengine 1.7.1.dev106+g6e3765d2 /home/runner/work/decisionengine/decisionengine/src Deprecated 1.2.13 distlib 0.3.4 docutils 0.17.1 execnet 1.9.0 filelock 3.4.1 flake8 4.0.1 greenlet 1.1.2 identify 2.4.3 idna 3.3 imagesize 1.3.0 importlib-metadata 4.2.0 importlib-resources 5.2.3 isort 5.10.1 jaraco.classes 3.2.1 jaraco.collections 3.4.0 jaraco.functools 3.4.0 jaraco.text 3.6.0 Jinja2 3.0.3 jsonnet 0.18.0 kombu 5.1.0 lazy-object-proxy 1.7.1 license-expression 21.6.14 MarkupSafe 2.0.1 mccabe 0.6.1 mirakuru 2.3.0 more-itertools 8.12.0 nodeenv 1.6.0 numpy 1.19.5 packaging 21.3 pandas 1.1.5 platformdirs 2.4.0 port-for 0.5.0 portend 3.0.0 pre-commit 2.16.0 prometheus-client 0.12.0 psutil 5.9.0 psycopg2-binary 2.9.3 pycodestyle 2.8.0 pyflakes 2.4.0 Pygments 2.11.2 pylint 2.12.2 pyparsing 3.0.6 pytest-cov 3.0.0 pytest-flake8 1.0.7 pytest-forked 1.4.0 pytest-postgresql 3.0.0 pytest-timeout 2.0.2 pytest-xdist 2.5.0 python-dateutil 2.8.2 python-debian 0.1.42 pytz 2021.3 PyYAML 6.0 redis 4.1.0 requests 2.27.1 reuse 0.14.0 setuptools-scm 6.3.2 six 1.16.0 snowballstemmer 2.2.0 Sphinx 4.3.2 sphinx-rtd-theme 1.0.0 sphinxcontrib-applehelp 1.0.2 sphinxcontrib-devhelp 1.0.2 sphinxcontrib-htmlhelp 2.0.0 sphinxcontrib-jsmath 1.0.1 sphinxcontrib-qthelp 1.0.3 sphinxcontrib-serializinghtml 1.1.5 SQLAlchemy 1.4.29 structlog 21.5.0 tabulate 0.8.9 tempora 4.1.2 tomli 1.2.3 toposort 1.7 typed-ast 1.5.1 typing_extensions 4.0.1 urllib3 1.26.8 vine 5.0.0 virtualenv 20.13.0 wrapt 1.13.3 zc.lockfile 2.0 zipp 3.6.0

4. What code are you running? Give us a specific commit of a specific repo that we can check out.

https://github.com/jcpunk/decisionengine/commit/e8a32134ed65c182a66dc1ff13fb3a3f867e5fd4

6. If you've already worked around the problem, please provide a commit before that fix.

Revert to 6.1.2

Expected behavior A clear and concise description of what you expected to happen.

Coverage to remain largely consistent.

Additional context Add any other context about the problem here.

This is probably related to the new support of additional concurrency things.

[nedbat]

@jcpunk are you still seeing this? Sorry I've let it sit so long.

[jcpunk]

I am still seeing this.

[nedbat]

@jcpunk Can you provide specific instructions for running the tests locally to show the problem? The GitHub Action steps are a bit involved.

[jcpunk]

In theory this should get you running the tests (on linux):

podman run --rm -d -p 127.0.0.1:6379:6379 redis:6
git clone [email protected]:HEPCloud/decisionengine.git
cd decisionengine
python3 -m pip install --user decisionengine[develop]
pytest --cov-report term --cov-report xml --cov=decisionengine --no-cov-on-fail

You'll also need the postgresql server binaries for the test service to fire up.

[antonagestam]

I just experienced a significant coverage dip when attempting to upgrade from 5.5 to 6.4, I suspect it could be the same issue. It's on a large proprietary code-base so I can't share code, but the coverage dips seems random to me, both entire functions and single blocks are affected.

[johanneswilm]

I am still seeing this. Coverage beyond 6.1.2 doesn't pick up on some lines being used. I am seeing this on Python 3.9.12 with a Django project (tried Django 3.2, 4.0 and 4.1).

[nedbat]

@johanneswilm can you provide me with a way to reproduce what you are seeing?

[johanneswilm]

Hey @nedbat , I think I solved it just now.

I had to replace this:

coverage run --concurrency=multiprocessing manage.py setup --no-static
coverage run --concurrency=multiprocessing manage.py test ${{ matrix.test }}
coverage combine

With:

coverage run --concurrency=multiprocessing,thread manage.py setup --no-static
coverage run --concurrency=multiprocessing,thread manage.py test ${{ matrix.test }}
coverage combine

If you still want to experiment with it, it happened in this OSS project: https://github.com/fiduswriter/fiduswriter . The github actions of main-branch is pinned to coverage==6.1.2, while in the develop branch just uses the latest release. The problem occurred in the develop-branch. Coverage as reported by coverage/coveralls fell from 90% to around 70% when I upgraded beyond coverage 6.1.2. The percentage is back where is was earlier when using the updated argument.

[itamaro]

Hello! I ran into this issue as well, when upgrading from 5.4.0 to 7.4.0 (yes, I know, big jump..), and I believe I've identified the root cause and why using --concurrency=multiprocessing,thread instead of --concurrency=multiprocessing fixes it.

TLDR: before 6.2, using --concurrency=multiprocessing also implied thread, but 6.2 fixed it (don't know if intentionally or not, as it is not called out in the changelog).

More details

A colleague was able to bisect the issue to this commit, and after carefully reading through the commit, I found that when the concurrency config is set to multiprocessing:

1. Before 6.2 (specifically, before this commit)
   1. these_concurrencies is assigned an empty set here
   2. self.concurrency is set to an empty string here
   3. since an empty string is falsy, this condition evaluates to True, which is equivalent to self.concurrency == "thread"
2. After 6.2:
   1. concurrencies is assigned the set {"multiprocessing"} here
   2. do_threading is set to False here
   3. since "thread" in not in concurrencies, do_threading is not set to True here
   4. since do_threading is False, and concurrencies is truthy (non-empty set), this conditions evaluates to False

As mentioned in the comment above, adding "thread" to the concurrency config fixes the issue. Perhaps at this point the best thing to do about it is add a note to the 6.2 changelog? Unless "multiprocessing implies thread" was intentional, so this is a bug that can be fixed.

[nedbat]

Thanks for the meticulous diagnosis! I'm not sure I intended the change in behavior, but I'd rather not change it again, and explicit is better than implicit. I've updated the changelog entry in commit e68b37e022ca1935b7550ac0e22329c46ccb1372 as well as added an entry to the Migrating page in the docs.