Skip to content

Commit

Permalink
Revert "feat: multiple --concurrency values. nedbat#1012 nedbat#1082"
Browse files Browse the repository at this point in the history
This reverts commit c9d821d.
  • Loading branch information
itamaro committed Jan 20, 2024
1 parent 1a416b6 commit ac190d8
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 90 deletions.
17 changes: 8 additions & 9 deletions coverage/cmdline.py
Expand Up @@ -46,12 +46,16 @@ class Opts:
"", "--branch", action="store_true",
help="Measure branch coverage in addition to statement coverage.",
)
CONCURRENCY_CHOICES = [
"thread", "gevent", "greenlet", "eventlet", "multiprocessing",
]
concurrency = optparse.make_option(
"", "--concurrency", action="store", metavar="LIBS",
'', '--concurrency', action='store', metavar="LIB",
choices=CONCURRENCY_CHOICES,
help=(
"Properly measure code using a concurrency library. " +
"Valid values are: {}, or a comma-list of them."
).format(", ".join(sorted(CoverageConfig.CONCURRENCY_CHOICES))),
"Valid values are: {}."
).format(", ".join(CONCURRENCY_CHOICES)),
)
context = optparse.make_option(
"", "--context", action="store", metavar="LABEL",
Expand Down Expand Up @@ -647,11 +651,6 @@ def command_line(self, argv: List[str]) -> int:
debug = unshell_list(options.debug)
contexts = unshell_list(options.contexts)

if options.concurrency is not None:
concurrency = options.concurrency.split(",")
else:
concurrency = None

# Do something.
self.coverage = Coverage(
data_file=options.data_file or DEFAULT_DATAFILE,
Expand All @@ -664,7 +663,7 @@ def command_line(self, argv: List[str]) -> int:
omit=omit,
include=include,
debug=debug,
concurrency=concurrency,
concurrency=options.concurrency,
check_preimported=True,
context=options.context,
messages=not options.quiet,
Expand Down
100 changes: 46 additions & 54 deletions coverage/collector.py
Expand Up @@ -73,7 +73,7 @@ class Collector:
_collectors: List[Collector] = []

# The concurrency settings we support here.
LIGHT_THREADS = {"greenlet", "eventlet", "gevent"}
SUPPORTED_CONCURRENCIES = {"greenlet", "eventlet", "gevent", "thread"}

def __init__(
self,
Expand Down Expand Up @@ -119,18 +119,17 @@ def __init__(
`concurrency` is a list of strings indicating the concurrency libraries
in use. Valid values are "greenlet", "eventlet", "gevent", or "thread"
(the default). "thread" can be combined with one of the other three.
Other values are ignored.
(the default). Of these four values, only one can be supplied. Other
values are ignored.
"""
self.should_trace = should_trace
self.check_include = check_include
self.should_start_context = should_start_context
self.file_mapper = file_mapper
self.branch = branch
self.warn = warn
self.concurrency = concurrency
assert isinstance(self.concurrency, list), f"Expected a list: {self.concurrency!r}"
# self.concurrency = concurrency
# assert isinstance(self.concurrency, list), f"Expected a list: {self.concurrency!r}"

self.pid = os.getpid()

Expand All @@ -146,6 +145,40 @@ def __init__(
self.file_disposition_class: Type[TFileDisposition]

core: Optional[str]

# We can handle a few concurrency options here, but only one at a time.
these_concurrencies = self.SUPPORTED_CONCURRENCIES.intersection(concurrency)
if len(these_concurrencies) > 1:
raise ConfigError(f"Conflicting concurrency settings: {concurrency}")
self.concurrency = these_concurrencies.pop() if these_concurrencies else ''

try:
if self.concurrency == "greenlet":
import greenlet
self.concur_id_func = greenlet.getcurrent
elif self.concurrency == "eventlet":
import eventlet.greenthread # pylint: disable=import-error,useless-suppression
self.concur_id_func = eventlet.greenthread.getcurrent
elif self.concurrency == "gevent":
import gevent # pylint: disable=import-error,useless-suppression
self.concur_id_func = gevent.getcurrent
elif self.concurrency == "thread" or not self.concurrency:
# It's important to import threading only if we need it. If
# it's imported early, and the program being measured uses
# gevent, then gevent's monkey-patching won't work properly.
import threading
self.threading = threading
else:
raise ConfigError(f"Don't understand concurrency={concurrency}")
except ImportError as ex:
raise ConfigError(
"Couldn't trace with concurrency={}, the module isn't installed.".format(
self.concurrency,
)
) from ex

self.reset()

if timid:
core = "pytrace"
else:
Expand Down Expand Up @@ -183,54 +216,6 @@ def __init__(
else:
raise ConfigError(f"Unknown core value: {core!r}")

# We can handle a few concurrency options here, but only one at a time.
concurrencies = set(self.concurrency)
unknown = concurrencies - CoverageConfig.CONCURRENCY_CHOICES
if unknown:
show = ", ".join(sorted(unknown))
raise ConfigError(f"Unknown concurrency choices: {show}")
light_threads = concurrencies & self.LIGHT_THREADS
if len(light_threads) > 1:
show = ", ".join(sorted(light_threads))
raise ConfigError(f"Conflicting concurrency settings: {show}")
do_threading = False

tried = "nothing" # to satisfy pylint
try:
if "greenlet" in concurrencies:
tried = "greenlet"
import greenlet
self.concur_id_func = greenlet.getcurrent
elif "eventlet" in concurrencies:
tried = "eventlet"
import eventlet.greenthread # pylint: disable=import-error,useless-suppression
self.concur_id_func = eventlet.greenthread.getcurrent
elif "gevent" in concurrencies:
tried = "gevent"
import gevent # pylint: disable=import-error,useless-suppression
self.concur_id_func = gevent.getcurrent

if "thread" in concurrencies:
do_threading = True
except ImportError as ex:
msg = f"Couldn't trace with concurrency={tried}, the module isn't installed."
raise ConfigError(msg) from ex

if self.concur_id_func and not hasattr(self._trace_class, "concur_id_func"):
raise ConfigError(
"Can't support concurrency={} with {}, only threads are supported.".format(
tried, self.tracer_name(),
)
)

if do_threading or not concurrencies:
# It's important to import threading only if we need it. If
# it's imported early, and the program being measured uses
# gevent, then gevent's monkey-patching won't work properly.
import threading
self.threading = threading

self.reset()

def __repr__(self) -> str:
return f"<Collector at {id(self):#x}: {self.tracer_name()}>"
Expand Down Expand Up @@ -311,6 +296,13 @@ def _start_tracer(self) -> TTraceFn | None:

if hasattr(tracer, 'concur_id_func'):
tracer.concur_id_func = self.concur_id_func
elif self.concur_id_func:
raise ConfigError(
"Can't support concurrency={} with {}, only threads are supported".format(
self.concurrency, self.tracer_name(),
)
)

if hasattr(tracer, 'file_tracers'):
tracer.file_tracers = self.file_tracers
if hasattr(tracer, 'threading'):
Expand Down
2 changes: 0 additions & 2 deletions coverage/config.py
Expand Up @@ -358,8 +358,6 @@ def copy(self) -> CoverageConfig:
"""Return a copy of the configuration."""
return copy.deepcopy(self)

CONCURRENCY_CHOICES = {"thread", "gevent", "greenlet", "eventlet", "multiprocessing"}

CONFIG_FILE_OPTIONS = [
# These are *args for _set_attr_from_config_option:
# (attr, where, type_="")
Expand Down
2 changes: 1 addition & 1 deletion coverage/control.py
Expand Up @@ -511,7 +511,7 @@ def load(self) -> None:
def _init_for_start(self) -> None:
"""Initialization for start()"""
# Construct the collector.
concurrency: List[str] = self.config.concurrency or []
concurrency = self.config.concurrency or ()
if "multiprocessing" in concurrency:
if self.config.config_file is None:
raise ConfigError("multiprocessing requires a configuration file")
Expand Down
16 changes: 6 additions & 10 deletions doc/cmd.rst
Expand Up @@ -114,9 +114,9 @@ There are many options:
clean each time.
--branch Measure branch coverage in addition to statement
coverage.
--concurrency=LIBS Properly measure code using a concurrency library.
Valid values are: eventlet, gevent, greenlet,
multiprocessing, thread, or a comma-list of them.
--concurrency=LIB Properly measure code using a concurrency library.
Valid values are: thread, gevent, greenlet, eventlet,
multiprocessing.
--context=LABEL The context label to record for this coverage run.
--data-file=OUTFILE Write the recorded coverage data to this file.
Defaults to '.coverage'. [env: COVERAGE_FILE]
Expand All @@ -143,7 +143,7 @@ There are many options:
--rcfile=RCFILE Specify configuration file. By default '.coveragerc',
'setup.cfg', 'tox.ini', and 'pyproject.toml' are
tried. [env: COVERAGE_RCFILE]
.. [[[end]]] (checksum: b1a0fffe2768fc142f1d97ae556b621d)
.. [[[end]]] (checksum: 869a31153b3cf401c52523ae9b52c7ab)
If you want :ref:`branch coverage <branch>` measurement, use the ``--branch``
flag. Otherwise only statement coverage is measured.
Expand All @@ -165,17 +165,13 @@ but before the program invocation::


Coverage.py can measure multi-threaded programs by default. If you are using
more other concurrency support, with the `multiprocessing`_, `greenlet`_,
`eventlet`_, or `gevent`_ libraries, then coverage.py can get confused. Use the
more exotic concurrency, with the `multiprocessing`_, `greenlet`_, `eventlet`_,
or `gevent`_ libraries, then coverage.py will get very confused. Use the
``--concurrency`` switch to properly measure programs using these libraries.
Give it a value of ``multiprocessing``, ``thread``, ``greenlet``, ``eventlet``,
or ``gevent``. Values other than ``thread`` require the :ref:`C extension
<install_extension>`.

You can combine multiple values for ``--concurrency``, separated with commas.
You can specify ``thread`` and also one of ``eventlet``, ``gevent``, or
``greenlet``.

If you are using ``--concurrency=multiprocessing``, you must set other options
in the configuration file. Options on the command line will not be passed to
the processes that multiprocessing creates. Best practice is to use the
Expand Down
1 change: 0 additions & 1 deletion requirements/light-threads.in
Expand Up @@ -6,7 +6,6 @@
# The light-threads packages we test against.

eventlet
gevent
greenlet

# gevent needs cffi, but only on Windows, not sure why.
Expand Down
13 changes: 2 additions & 11 deletions tests/test_cmdline.py
Expand Up @@ -727,7 +727,7 @@ def test_run(self) -> None:
cov.save()
""")
self.cmd_executes("run --concurrency=gevent foo.py", """\
cov = Coverage(concurrency=['gevent'])
cov = Coverage(concurrency='gevent')
runner = PyRunner(['foo.py'], as_module=False)
runner.prepare()
cov.start()
Expand All @@ -736,16 +736,7 @@ def test_run(self) -> None:
cov.save()
""")
self.cmd_executes("run --concurrency=multiprocessing foo.py", """\
cov = Coverage(concurrency=['multiprocessing'])
runner = PyRunner(['foo.py'], as_module=False)
runner.prepare()
cov.start()
runner.run()
cov.stop()
cov.save()
""")
self.cmd_executes("run --concurrency=gevent,thread foo.py", """\
cov = Coverage(concurrency=['gevent', 'thread'])
cov = Coverage(concurrency='multiprocessing')
runner = PyRunner(['foo.py'], as_module=False)
runner.prepare()
cov.start()
Expand Down
4 changes: 2 additions & 2 deletions tests/test_concurrency.py
Expand Up @@ -24,7 +24,6 @@
import coverage
from coverage import env
from coverage.data import line_counts
from coverage.exceptions import ConfigError
from coverage.files import abs_file
from coverage.misc import import_local_file

Expand Down Expand Up @@ -193,7 +192,7 @@ def cant_trace_msg(concurrency: str, the_module: Optional[ModuleType]) -> Option
expected_out = None
else:
expected_out = (
f"Can't support concurrency={concurrency} with PyTracer, only threads are supported.\n"
f"Can't support concurrency={concurrency} with PyTracer, only threads are supported\n"
)
return expected_out

Expand All @@ -218,6 +217,7 @@ def try_some_code(
is the text we expect the code to produce.
"""

self.make_file("try_it.py", code)

cmd = f"coverage run --concurrency={concurrency} try_it.py"
Expand Down

0 comments on commit ac190d8

Please sign in to comment.