Skip to content

Commit

Permalink
refactor: specialize exceptions
Browse files Browse the repository at this point in the history
CoverageException is fine as a base class, but not good to use for
raising (and catching sometimes).  Introduce specialized exceptions that
allow third-party tools to integrate better.
  • Loading branch information
nedbat committed Nov 14, 2021
1 parent 342e7da commit 1c29ef3
Show file tree
Hide file tree
Showing 25 changed files with 147 additions and 115 deletions.
3 changes: 3 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ Unreleased
- Fix: The HTML report now will not overwrite a .gitignore file that already
exists in the HTML output directory (follow-on for `issue 1244`_).

- API: The exceptions raised by Coverage.py have been specialized, to provide
finer-grained catching of exceptions by third-party code.

- Debug: The `coverage debug data` command will now sniff out combinable data
files, and report on all of them.

Expand Down
10 changes: 5 additions & 5 deletions coverage/collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from coverage import env
from coverage.debug import short_stack
from coverage.disposition import FileDisposition
from coverage.exceptions import CoverageException
from coverage.exceptions import ConfigError
from coverage.misc import human_sorted, isolate_module
from coverage.pytracer import PyTracer

Expand Down Expand Up @@ -116,7 +116,7 @@ def __init__(
# 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 CoverageException(f"Conflicting concurrency settings: {concurrency}")
raise ConfigError(f"Conflicting concurrency settings: {concurrency}")
self.concurrency = these_concurrencies.pop() if these_concurrencies else ''

try:
Expand All @@ -136,9 +136,9 @@ def __init__(
import threading
self.threading = threading
else:
raise CoverageException(f"Don't understand concurrency={concurrency}")
raise ConfigError(f"Don't understand concurrency={concurrency}")
except ImportError as ex:
raise CoverageException(
raise ConfigError(
"Couldn't trace with concurrency={}, the module isn't installed.".format(
self.concurrency,
)
Expand Down Expand Up @@ -245,7 +245,7 @@ def _start_tracer(self):
if hasattr(tracer, 'concur_id_func'):
tracer.concur_id_func = self.concur_id_func
elif self.concur_id_func:
raise CoverageException(
raise ConfigError(
"Can't support concurrency={} with {}, only threads are supported".format(
self.concurrency, self.tracer_name(),
)
Expand Down
18 changes: 9 additions & 9 deletions coverage/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import os.path
import re

from coverage.exceptions import CoverageException
from coverage.exceptions import ConfigError
from coverage.misc import contract, isolate_module, substitute_variables

from coverage.tomlconfig import TomlConfigParser, TomlDecodeError
Expand Down Expand Up @@ -59,7 +59,7 @@ def options(self, section):
real_section = section_prefix + section
if configparser.RawConfigParser.has_section(self, real_section):
return configparser.RawConfigParser.options(self, real_section)
raise configparser.NoSectionError(section)
raise ConfigError(f"No section: {section!r}")

def get_section(self, section):
"""Get the contents of a section, as a dictionary."""
Expand All @@ -83,7 +83,7 @@ def get(self, section, option, *args, **kwargs):
if configparser.RawConfigParser.has_option(self, real_section, option):
break
else:
raise configparser.NoOptionError(option, section)
raise ConfigError(f"No option {option!r} in section: {section!r}")

v = configparser.RawConfigParser.get(self, real_section, option, *args, **kwargs)
v = substitute_variables(v, os.environ)
Expand Down Expand Up @@ -123,7 +123,7 @@ def getregexlist(self, section, option):
try:
re.compile(value)
except re.error as e:
raise CoverageException(
raise ConfigError(
f"Invalid [{section}].{option} value {value!r}: {e}"
) from e
if value:
Expand Down Expand Up @@ -272,7 +272,7 @@ def from_file(self, filename, warn, our_file):
try:
files_read = cp.read(filename)
except (configparser.Error, TomlDecodeError) as err:
raise CoverageException(f"Couldn't read config file {filename}: {err}") from err
raise ConfigError(f"Couldn't read config file {filename}: {err}") from err
if not files_read:
return False

Expand All @@ -285,7 +285,7 @@ def from_file(self, filename, warn, our_file):
if was_set:
any_set = True
except ValueError as err:
raise CoverageException(f"Couldn't read config file {filename}: {err}") from err
raise ConfigError(f"Couldn't read config file {filename}: {err}") from err

# Check that there are no unrecognized options.
all_options = collections.defaultdict(set)
Expand Down Expand Up @@ -443,7 +443,7 @@ def set_option(self, option_name, value):
return

# If we get here, we didn't find the option.
raise CoverageException(f"No such option: {option_name!r}")
raise ConfigError(f"No such option: {option_name!r}")

def get_option(self, option_name):
"""Get an option from the configuration.
Expand Down Expand Up @@ -471,7 +471,7 @@ def get_option(self, option_name):
return self.plugin_options.get(plugin_name, {}).get(key)

# If we get here, we didn't find the option.
raise CoverageException(f"No such option: {option_name!r}")
raise ConfigError(f"No such option: {option_name!r}")

def post_process_file(self, path):
"""Make final adjustments to a file path to make it usable."""
Expand Down Expand Up @@ -546,7 +546,7 @@ def read_coverage_config(config_file, warn, **kwargs):
if config_read:
break
if specified_file:
raise CoverageException(f"Couldn't read {fname!r} as a config file")
raise ConfigError(f"Couldn't read {fname!r} as a config file")

# $set_env.py: COVERAGE_DEBUG - Options for --debug.
# 3) from environment variables:
Expand Down
10 changes: 6 additions & 4 deletions coverage/control.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from coverage.data import CoverageData, combine_parallel_data
from coverage.debug import DebugControl, short_stack, write_formatted_info
from coverage.disposition import disposition_debug_msg
from coverage.exceptions import CoverageException, CoverageWarning
from coverage.exceptions import ConfigError, CoverageException, CoverageWarning, PluginError
from coverage.files import PathAliases, abs_file, relative_filename, set_relative_directory
from coverage.html import HtmlReporter
from coverage.inorout import InOrOut
Expand Down Expand Up @@ -79,6 +79,8 @@ class Coverage:
not part of the public API. They might stop working at any point. Please
limit yourself to documented methods to avoid problems.
Methods can raise any of the exceptions described in :ref:`api_exceptions`.
"""

# The stack of started Coverage instances.
Expand Down Expand Up @@ -449,7 +451,7 @@ def _init_for_start(self):
concurrency = self.config.concurrency or ()
if "multiprocessing" in concurrency:
if not patch_multiprocessing:
raise CoverageException( # pragma: only jython
raise ConfigError( # pragma: only jython
"multiprocessing is not supported on this Python"
)
patch_multiprocessing(rcfile=self.config.config_file)
Expand All @@ -460,7 +462,7 @@ def _init_for_start(self):
elif dycon == "test_function":
context_switchers = [should_start_context_test_function]
else:
raise CoverageException(f"Don't understand dynamic_context setting: {dycon!r}")
raise ConfigError(f"Don't understand dynamic_context setting: {dycon!r}")

context_switchers.extend(
plugin.dynamic_context for plugin in self._plugins.context_switchers
Expand Down Expand Up @@ -835,7 +837,7 @@ def _get_file_reporter(self, morf):
if plugin:
file_reporter = plugin.file_reporter(mapped_morf)
if file_reporter is None:
raise CoverageException(
raise PluginError(
"Plugin {!r} did not provide a file reporter for {!r}.".format(
plugin._coverage_plugin_name, morf
)
Expand Down
8 changes: 4 additions & 4 deletions coverage/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import glob
import os.path

from coverage.exceptions import CoverageException
from coverage.exceptions import CoverageException, NoDataError
from coverage.misc import file_be_gone
from coverage.sqldata import CoverageData

Expand Down Expand Up @@ -72,7 +72,7 @@ def combinable_files(data_file, data_paths=None):
pattern = os.path.join(os.path.abspath(p), f"{local}.*")
files_to_combine.extend(glob.glob(pattern))
else:
raise CoverageException(f"Couldn't combine from non-existent path '{p}'")
raise NoDataError(f"Couldn't combine from non-existent path '{p}'")
return files_to_combine


Expand Down Expand Up @@ -107,7 +107,7 @@ def combine_parallel_data(
files_to_combine = combinable_files(data.base_filename(), data_paths)

if strict and not files_to_combine:
raise CoverageException("No data to combine")
raise NoDataError("No data to combine")

files_combined = 0
for f in files_to_combine:
Expand Down Expand Up @@ -138,4 +138,4 @@ def combine_parallel_data(
file_be_gone(f)

if strict and not files_combined:
raise CoverageException("No usable data files")
raise NoDataError("No usable data files")
23 changes: 21 additions & 2 deletions coverage/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,26 @@


class _BaseCoverageException(Exception):
"""The base of all Coverage exceptions."""
"""The base-base of all Coverage exceptions."""
pass


class CoverageException(_BaseCoverageException):
"""An exception raised by a coverage.py function."""
"""The base class of all exceptions raised by Coverage.py."""
pass


class ConfigError(_BaseCoverageException):
"""A problem with a config file, or a value in one."""
pass


class DataError(CoverageException):
"""An error in using a data file."""
pass

class NoDataError(CoverageException):
"""We didn't have data to work with."""
pass


Expand All @@ -29,6 +43,11 @@ class NotPython(CoverageException):
pass


class PluginError(CoverageException):
"""A plugin misbehaved."""
pass


class _ExceptionDuringRun(CoverageException):
"""An exception happened while running customer code.
Expand Down
4 changes: 2 additions & 2 deletions coverage/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import sys

from coverage import env
from coverage.exceptions import CoverageException
from coverage.exceptions import ConfigError
from coverage.misc import contract, human_sorted, isolate_module, join_regex


Expand Down Expand Up @@ -356,7 +356,7 @@ def add(self, pattern, result):

# The pattern can't end with a wildcard component.
if pattern.endswith("*"):
raise CoverageException("Pattern must not end with wildcards.")
raise ConfigError("Pattern must not end with wildcards.")

# The pattern is meant to match a filepath. Let's make it absolute
# unless it already is, or is meant to match any prefix.
Expand Down
4 changes: 2 additions & 2 deletions coverage/html.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

import coverage
from coverage.data import add_data_to_hash
from coverage.exceptions import CoverageException
from coverage.exceptions import NoDataError
from coverage.files import flat_rootname
from coverage.misc import ensure_dir, file_be_gone, Hasher, isolate_module, format_local_datetime
from coverage.misc import human_sorted, plural
Expand Down Expand Up @@ -208,7 +208,7 @@ def report(self, morfs):
self.html_file(fr, analysis)

if not self.all_files_nums:
raise CoverageException("No data to report.")
raise NoDataError("No data to report.")

self.totals = sum(self.all_files_nums)

Expand Down
4 changes: 2 additions & 2 deletions coverage/inorout.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

from coverage import env
from coverage.disposition import FileDisposition, disposition_init
from coverage.exceptions import CoverageException
from coverage.exceptions import CoverageException, PluginError
from coverage.files import TreeMatcher, FnmatchMatcher, ModuleMatcher
from coverage.files import prep_patterns, find_python_files, canonical_filename
from coverage.misc import sys_modules_saved
Expand Down Expand Up @@ -392,7 +392,7 @@ def nope(disp, reason):

if not disp.has_dynamic_filename:
if not disp.source_filename:
raise CoverageException(
raise PluginError(
f"Plugin {plugin!r} didn't set source_filename for '{disp.original_filename}'"
)
reason = self.check_include_omit_etc(disp.source_filename, frame)
Expand Down
4 changes: 2 additions & 2 deletions coverage/plugin_support.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import os.path
import sys

from coverage.exceptions import CoverageException
from coverage.exceptions import PluginError
from coverage.misc import isolate_module
from coverage.plugin import CoveragePlugin, FileTracer, FileReporter

Expand Down Expand Up @@ -44,7 +44,7 @@ def load_plugins(cls, modules, config, debug=None):

coverage_init = getattr(mod, "coverage_init", None)
if not coverage_init:
raise CoverageException(
raise PluginError(
f"Plugin module {module!r} didn't define a coverage_init function"
)

Expand Down
4 changes: 2 additions & 2 deletions coverage/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import sys

from coverage.exceptions import CoverageException, NotPython
from coverage.exceptions import CoverageException, NoDataError, NotPython
from coverage.files import prep_patterns, FnmatchMatcher
from coverage.misc import ensure_dir_for_file, file_be_gone

Expand Down Expand Up @@ -65,7 +65,7 @@ def get_analysis_to_report(coverage, morfs):
file_reporters = [fr for fr in file_reporters if not matcher.match(fr.filename)]

if not file_reporters:
raise CoverageException("No data to report.")
raise NoDataError("No data to report.")

for fr in sorted(file_reporters):
try:
Expand Down
4 changes: 2 additions & 2 deletions coverage/results.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import collections

from coverage.debug import SimpleReprMixin
from coverage.exceptions import CoverageException
from coverage.exceptions import ConfigError
from coverage.misc import contract, nice_pair


Expand Down Expand Up @@ -337,7 +337,7 @@ def should_fail_under(total, fail_under, precision):
# We can never achieve higher than 100% coverage, or less than zero.
if not (0 <= fail_under <= 100.0):
msg = f"fail_under={fail_under} is invalid. Must be between 0 and 100."
raise CoverageException(msg)
raise ConfigError(msg)

# Special case for fail_under=100, it must really be 100.
if fail_under == 100.0 and total != 100.0:
Expand Down
Loading

0 comments on commit 1c29ef3

Please sign in to comment.