Skip to content

Commit

Permalink
Implement most of @nedbat's feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
bartfeenstra committed May 16, 2024
1 parent e1dd118 commit 8bf5006
Show file tree
Hide file tree
Showing 13 changed files with 44 additions and 43 deletions.
5 changes: 3 additions & 2 deletions coverage/cmdline.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@
from coverage.control import DEFAULT_DATAFILE
from coverage.data import combinable_files, debug_data_file
from coverage.debug import info_header, short_stack, write_formatted_info
from coverage.exceptions import _BaseCoverageException, _ExceptionDuringRun, NoSource, \
NoDataFilesFoundError
from coverage.exceptions import (
_BaseCoverageException, _ExceptionDuringRun, NoSource, NoDataFilesFoundError,
)
from coverage.execfile import PyRunner
from coverage.results import display_covered, should_fail_under
from coverage.version import __url__
Expand Down
6 changes: 3 additions & 3 deletions coverage/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def combinable_files(data_file: str, data_paths: Iterable[str] | None = None) ->
pattern = glob.escape(os.path.join(os.path.abspath(p), local)) +".*"
files_to_combine.extend(glob.glob(pattern))
else:
raise DataFileOrDirectoryNotFoundError.new_for_data_file_or_directory(
raise DataFileOrDirectoryNotFoundError.new(
p, is_combining=True
)

Expand All @@ -91,7 +91,7 @@ def combinable_files(data_file: str, data_paths: Iterable[str] | None = None) ->
files_to_combine = [fnm for fnm in files_to_combine if not fnm.endswith("-journal")]

if not files_to_combine:
raise NoDataFilesFoundError.new_for_data_directory(data_dir)
raise NoDataFilesFoundError.new(data_dir)

# Sorting isn't usually needed, since it shouldn't matter what order files
# are combined, but sorting makes tests more predictable, and makes
Expand Down Expand Up @@ -197,7 +197,7 @@ def combine_parallel_data(
file_be_gone(f)

if strict and not combined_any:
raise UnusableDataFilesError.new_for_data_files(*files_to_combine)
raise UnusableDataFilesError.new(*files_to_combine)


def debug_data_file(filename: str) -> None:
Expand Down
22 changes: 11 additions & 11 deletions coverage/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
def _message_append_combine_hint(message: str, is_combining: bool) -> str:
"""Append information about the combine command to error messages."""
if not is_combining:
message += " Perhaps `coverage combine` must be run first."
message += " Perhaps 'coverage combine' must be run first."
return message


Expand Down Expand Up @@ -43,48 +43,48 @@ class NoDataError(CoverageException):
class DataFileOrDirectoryNotFoundError(NoDataError):
"""A data file or data directory could be found."""
@classmethod
def new_for_data_file_or_directory(
def new(
cls, data_file_or_directory_path: str, *, is_combining: bool = False
) -> 'DataFileOrDirectoryNotFoundError':
) -> DataFileOrDirectoryNotFoundError:
"""
Create a new instance.
"""
message = (
f"The data file or directory `{os.path.abspath(data_file_or_directory_path)}` could not"
" be found."
f"The data file or directory '{os.path.abspath(data_file_or_directory_path)}' could not"
+ " be found."
)
return cls(_message_append_combine_hint(message, is_combining))


class NoDataFilesFoundError(NoDataError):
"""No data files could be found in a data directory."""
@classmethod
def new_for_data_directory(
def new(
cls, data_directory_path: str, *, is_combining: bool = False
) -> 'NoDataFilesFoundError':
"""
Create a new instance.
"""
message = (
f"The data directory `{os.path.abspath(data_directory_path)}` does not contain any data"
" files."
f"The data directory '{os.path.abspath(data_directory_path)}' does not contain any data"
+ " files."
)
return cls(_message_append_combine_hint(message, is_combining))


class UnusableDataFilesError(NoDataError):
"""The given data files are unusable."""
@classmethod
def new_for_data_files(cls, *data_file_paths: str) -> 'UnusableDataFilesError':
def new(cls, *data_file_paths: str) -> 'UnusableDataFilesError':
"""
Create a new instance.
"""
message = (
"The following data files are unusable, perhaps because they do not contain valid"
" coverage information:"
+ " coverage information:"
)
for data_file_path in data_file_paths:
message += f"\n- `{os.path.abspath(data_file_path)}`"
message += f"\n- '{os.path.abspath(data_file_path)}'"

return cls(message)

Expand Down
2 changes: 1 addition & 1 deletion coverage/html.py
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ def report(self, morfs: Iterable[TMorf] | None) -> float:
file_be_gone(os.path.join(self.directory, ftr.html_filename))

if not have_data:
raise DataFileOrDirectoryNotFoundError.new_for_data_file_or_directory(
raise DataFileOrDirectoryNotFoundError.new(
os.path.dirname(self.coverage.get_data().base_filename())
)

Expand Down
2 changes: 1 addition & 1 deletion coverage/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ def report(self, morfs: Iterable[TMorf] | None, outfile: IO[str] | None = None)
self.report_one_file(fr, analysis)

if not self.total.n_files and not self.skipped_count:
raise DataFileOrDirectoryNotFoundError.new_for_data_file_or_directory(
raise DataFileOrDirectoryNotFoundError.new(
os.path.dirname(self.coverage.get_data().base_filename())
)

Expand Down
2 changes: 1 addition & 1 deletion coverage/report_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def get_analysis_to_report(
fr_morfs = [(fr, morf) for (fr, morf) in fr_morfs if not matcher.match(fr.filename)]

if not fr_morfs:
raise DataFileOrDirectoryNotFoundError.new_for_data_file_or_directory(
raise DataFileOrDirectoryNotFoundError.new(
os.path.dirname(coverage.get_data().base_filename())
)

Expand Down
10 changes: 5 additions & 5 deletions tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,8 @@ def test_empty_reporting(self) -> None:
with pytest.raises(
NoDataError,
match=(
r"^The data file or directory `(.+?)` could not be found\. Perhaps `coverage "
r"combine` must be run first\.$"
r"^The data file or directory '(.+?)' could not be found\. Perhaps 'coverage "
r"combine' must be run first\.$"
)
):
cov.report()
Expand Down Expand Up @@ -455,8 +455,8 @@ def test_combining_twice(self) -> None:
with pytest.raises(
NoDataError,
match=(
r"^The data directory `(.+?)` does not contain any data files. Perhaps `coverage "
r"combine` must be run first.$"
r"^The data directory '(.+?)' does not contain any data files. Perhaps 'coverage "
r"combine' must be run first.$"
)
):
cov2.combine(strict=True, keep=False)
Expand Down Expand Up @@ -1392,7 +1392,7 @@ def test_combine_no_usable_files(self) -> None:
NoDataError,
match=(
r"^The following data files are unusable, perhaps because they do not contain "
r"valid coverage information:\n- `(.+?)`\n- `(.+?)`$"
r"valid coverage information:\n- '(.+?)'\n- '(.+?)'$"
)
):
cov.combine(strict=True)
Expand Down
12 changes: 6 additions & 6 deletions tests/test_coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -1638,8 +1638,8 @@ def test_no_data_to_report_on_annotate(self) -> None:
with pytest.raises(
NoDataError,
match=(
r"^The data file or directory `(.+?)` could not be found\. Perhaps `coverage "
r"combine` must be run first\.$"
r"^The data file or directory '(.+?)' could not be found\. Perhaps 'coverage "
r"combine' must be run first\.$"
)
):
self.command_line("annotate -d ann")
Expand All @@ -1651,8 +1651,8 @@ def test_no_data_to_report_on_html(self) -> None:
with pytest.raises(
NoDataError,
match=(
r"^The data file or directory `(.+?)` could not be found\. Perhaps `coverage "
r"combine` must be run first\.$"
r"^The data file or directory '(.+?)' could not be found\. Perhaps 'coverage "
r"combine' must be run first\.$"
)
):
self.command_line("html -d htmlcov")
Expand All @@ -1663,8 +1663,8 @@ def test_no_data_to_report_on_xml(self) -> None:
with pytest.raises(
NoDataError,
match=(
r"^The data file or directory `(.+?)` could not be found\. Perhaps `coverage "
r"combine` must be run first\.$"
r"^The data file or directory '(.+?)' could not be found\. Perhaps 'coverage "
r"combine' must be run first\.$"
)
):
self.command_line("xml")
Expand Down
2 changes: 1 addition & 1 deletion tests/test_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -915,7 +915,7 @@ def test_combining_from_files(self) -> None:

def test_combining_from_nonexistent_directories(self) -> None:
covdata = DebugCoverageData()
msg = r"^The data file or directory `(.+?)` could not be found.$"
msg = r"^The data file or directory '(.+?)' could not be found.$"
with pytest.raises(NoDataError, match=msg):
combine_parallel_data(covdata, data_paths=['xyzzy'])

Expand Down
4 changes: 2 additions & 2 deletions tests/test_html.py
Original file line number Diff line number Diff line change
Expand Up @@ -429,8 +429,8 @@ def test_dothtml_not_python(self) -> None:
with pytest.raises(
NoDataError,
match=(
r"^The data file or directory `(.+?)` could not be found\. Perhaps `coverage "
r"combine` must be run first\.$"
r"^The data file or directory '(.+?)' could not be found\. Perhaps 'coverage "
r"combine' must be run first\.$"
)
):
cov.html_report()
Expand Down
4 changes: 2 additions & 2 deletions tests/test_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -1145,8 +1145,8 @@ def test_report(self) -> None:
st, out = self.run_command_status("coverage report")
assert re.match(
(
r"The data file or directory `([^`]+?)` could not be found\. Perhaps `coverage "
r"combine` must be run first\."
r"The data file or directory '([^']+?)' could not be found\. Perhaps 'coverage "
+ r"combine' must be run first\."
),
out
)
Expand Down
12 changes: 6 additions & 6 deletions tests/test_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -554,8 +554,8 @@ def test_report_skip_covered_no_data(self) -> None:
with pytest.raises(
NoDataError,
match=(
r"^The data file or directory `(.+?)` could not be found\. Perhaps `coverage "
r"combine` must be run first\.$"
r"^The data file or directory '(.+?)' could not be found\. Perhaps 'coverage "
r"combine' must be run first\.$"
)
):
self.get_report(cov, skip_covered=True)
Expand Down Expand Up @@ -753,8 +753,8 @@ def test_dotpy_not_python_ignored(self) -> None:
with pytest.raises(
NoDataError,
match=(
r"^The data file or directory `(.+?)` could not be found\. Perhaps `coverage "
r"combine` must be run first\.$"
r"^The data file or directory '(.+?)' could not be found\. Perhaps 'coverage "
r"combine' must be run first\.$"
)
):
with pytest.warns(Warning) as warns:
Expand All @@ -776,8 +776,8 @@ def test_dothtml_not_python(self) -> None:
with pytest.raises(
NoDataError,
match=(
r"^The data file or directory `(.+?)` could not be found\. Perhaps `coverage "
r"combine` must be run first\.$"
r"^The data file or directory '(.+?)' could not be found\. Perhaps 'coverage "
r"combine' must be run first\.$"
)
):
self.get_report(cov, morfs=["mycode.html"])
Expand Down
4 changes: 2 additions & 2 deletions tests/test_xml.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ def test_no_data(self) -> None:
with pytest.raises(
NoDataError,
match=(
r"^The data file or directory `(.+?)` could not be found\. Perhaps `coverage "
r"combine` must be run first\.$"
r"^The data file or directory '(.+?)' could not be found\. Perhaps 'coverage "
r"combine' must be run first\.$"
)
):
self.run_xml_report()
Expand Down

0 comments on commit 8bf5006

Please sign in to comment.