From ac190d8eddbf5a4ba3d87f5bba65041bd53ad222 Mon Sep 17 00:00:00 2001 From: Itamar Oren Date: Sat, 20 Jan 2024 12:28:10 -0800 Subject: [PATCH] Revert "feat: multiple --concurrency values. #1012 #1082" This reverts commit c9d821deba6f7ee5eef30fef5355f7c93808b4f9. --- coverage/cmdline.py | 17 +++--- coverage/collector.py | 100 ++++++++++++++++------------------ coverage/config.py | 2 - coverage/control.py | 2 +- doc/cmd.rst | 16 ++---- requirements/light-threads.in | 1 - tests/test_cmdline.py | 13 +---- tests/test_concurrency.py | 4 +- 8 files changed, 65 insertions(+), 90 deletions(-) diff --git a/coverage/cmdline.py b/coverage/cmdline.py index 5379c7c5f..c35a52f40 100644 --- a/coverage/cmdline.py +++ b/coverage/cmdline.py @@ -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", @@ -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, @@ -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, diff --git a/coverage/collector.py b/coverage/collector.py index 1fd06dbb5..ab0905200 100644 --- a/coverage/collector.py +++ b/coverage/collector.py @@ -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, @@ -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() @@ -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: @@ -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"" @@ -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'): diff --git a/coverage/config.py b/coverage/config.py index 24d5642b2..a91a8b14b 100644 --- a/coverage/config.py +++ b/coverage/config.py @@ -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_="") diff --git a/coverage/control.py b/coverage/control.py index d33ef769a..423d2c9be 100644 --- a/coverage/control.py +++ b/coverage/control.py @@ -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") diff --git a/doc/cmd.rst b/doc/cmd.rst index 2162cc84e..9e88f21ac 100644 --- a/doc/cmd.rst +++ b/doc/cmd.rst @@ -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] @@ -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 ` measurement, use the ``--branch`` flag. Otherwise only statement coverage is measured. @@ -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 `. -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 diff --git a/requirements/light-threads.in b/requirements/light-threads.in index 6085b6b0f..683fa9e0d 100644 --- a/requirements/light-threads.in +++ b/requirements/light-threads.in @@ -6,7 +6,6 @@ # The light-threads packages we test against. eventlet -gevent greenlet # gevent needs cffi, but only on Windows, not sure why. diff --git a/tests/test_cmdline.py b/tests/test_cmdline.py index d27f6a4aa..37293a7ca 100644 --- a/tests/test_cmdline.py +++ b/tests/test_cmdline.py @@ -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() @@ -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() diff --git a/tests/test_concurrency.py b/tests/test_concurrency.py index db2856538..a4a7d9e95 100644 --- a/tests/test_concurrency.py +++ b/tests/test_concurrency.py @@ -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 @@ -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 @@ -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"