Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report data file errors in more detail #1782

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions coverage/cmdline.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import sys
import textwrap
import traceback
from contextlib import suppress

from typing import cast, Any, NoReturn

Expand All @@ -24,7 +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
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 Expand Up @@ -882,9 +885,10 @@ def do_debug(self, args: list[str]) -> int:
print(info_header("data"))
data_file = self.coverage.config.data_file
debug_data_file(data_file)
for filename in combinable_files(data_file):
print("-----")
debug_data_file(filename)
with suppress(NoDataFilesFoundError):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to suppress this error here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not, but I tried to limit how many outputs I changed. Would you like this to display a friendly error instead?

for filename in combinable_files(data_file):
print("-----")
debug_data_file(filename)
elif args[0] == "config":
write_formatted_info(print, "config", self.coverage.config.debug_info())
elif args[0] == "premain":
Expand Down
23 changes: 15 additions & 8 deletions coverage/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
import glob
import hashlib
import os.path

from typing import Callable, Iterable

from coverage.exceptions import CoverageException, NoDataError
from coverage.exceptions import CoverageException, DataFileOrDirectoryNotFoundError, \
NoDataFilesFoundError, UnusableDataFilesError
from coverage.files import PathAliases
from coverage.misc import Hasher, file_be_gone, human_sorted, plural
from coverage.sqldata import CoverageData
Expand Down Expand Up @@ -82,12 +82,17 @@ 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 NoDataError(f"Couldn't combine from non-existent path '{p}'")
raise DataFileOrDirectoryNotFoundError.new(
p, is_combining=True
)

# SQLite might have made journal files alongside our database files.
# We never want to combine those.
files_to_combine = [fnm for fnm in files_to_combine if not fnm.endswith("-journal")]

if not files_to_combine:
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
# debugging more understandable when things go wrong.
Expand Down Expand Up @@ -129,10 +134,12 @@ def combine_parallel_data(
`message` is a function to use for printing messages to the user.

"""
files_to_combine = combinable_files(data.base_filename(), data_paths)

if strict and not files_to_combine:
raise NoDataError("No data to combine")
try:
files_to_combine = combinable_files(data.base_filename(), data_paths)
except NoDataFilesFoundError:
if strict:
raise
return

file_hashes = set()
combined_any = False
Expand Down Expand Up @@ -190,7 +197,7 @@ def combine_parallel_data(
file_be_gone(f)

if strict and not combined_any:
raise NoDataError("No usable data files")
raise UnusableDataFilesError.new(*files_to_combine)


def debug_data_file(filename: str) -> None:
Expand Down
60 changes: 60 additions & 0 deletions coverage/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,16 @@

from __future__ import annotations

import os.path


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."
return message


class _BaseCoverageException(Exception):
"""The base-base of all Coverage exceptions."""
pass
Expand All @@ -24,11 +34,61 @@ class DataError(CoverageException):
"""An error in using a data file."""
pass


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


class DataFileOrDirectoryNotFoundError(NoDataError):
"""A data file or data directory could be found."""
@classmethod
def new(
cls, data_file_or_directory_path: str, *, is_combining: bool = False
) -> DataFileOrDirectoryNotFoundError:
"""
Create a new instance.
"""
message = (
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(
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."
)
return cls(_message_append_combine_hint(message, is_combining))


class UnusableDataFilesError(NoDataError):
"""The given data files are unusable."""
@classmethod
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:"
)
for data_file_path in data_file_paths:
message += f"\n- '{os.path.abspath(data_file_path)}'"

return cls(message)


class NoSource(CoverageException):
"""We couldn't find the source for a module."""
pass
Expand Down
6 changes: 4 additions & 2 deletions coverage/html.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import coverage
from coverage.data import CoverageData, add_data_to_hash
from coverage.exceptions import NoDataError
from coverage.exceptions import DataFileOrDirectoryNotFoundError
from coverage.files import flat_rootname
from coverage.misc import (
ensure_dir, file_be_gone, Hasher, isolate_module, format_local_datetime,
Expand Down Expand Up @@ -317,7 +317,9 @@ def report(self, morfs: Iterable[TMorf] | None) -> float:
file_be_gone(os.path.join(self.directory, ftr.html_filename))

if not have_data:
raise NoDataError("No data to report.")
raise DataFileOrDirectoryNotFoundError.new(
os.path.dirname(self.coverage.get_data().base_filename())
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to think through whether the message here is correct. Is it always that the file doesn't exist?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's good that the exceptions centralize the messages, but everywhere this exception is created it uses the same fairly complex expression. Can we centralize that as well?

)

self.make_directory()
self.make_local_static_report_files()
Expand Down
7 changes: 5 additions & 2 deletions coverage/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@

from __future__ import annotations

import os
import sys

from typing import Any, IO, Iterable, TYPE_CHECKING

from coverage.exceptions import ConfigError, NoDataError
from coverage.exceptions import ConfigError, DataFileOrDirectoryNotFoundError
from coverage.misc import human_sorted_items
from coverage.plugin import FileReporter
from coverage.report_core import get_analysis_to_report
Expand Down Expand Up @@ -182,7 +183,9 @@ 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 NoDataError("No data to report.")
raise DataFileOrDirectoryNotFoundError.new(
os.path.dirname(self.coverage.get_data().base_filename())
)

if self.output_format == "total":
self.write(self.total.pc_covered_str)
Expand Down
7 changes: 5 additions & 2 deletions coverage/report_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@

from __future__ import annotations

import os
import sys

from typing import (
Callable, Iterable, Iterator, IO, Protocol, TYPE_CHECKING,
)

from coverage.exceptions import NoDataError, NotPython
from coverage.exceptions import NotPython, DataFileOrDirectoryNotFoundError
from coverage.files import prep_patterns, GlobMatcher
from coverage.misc import ensure_dir_for_file, file_be_gone
from coverage.plugin import FileReporter
Expand Down Expand Up @@ -93,7 +94,9 @@ 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 NoDataError("No data to report.")
raise DataFileOrDirectoryNotFoundError.new(
os.path.dirname(coverage.get_data().base_filename())
)

for fr, morf in sorted(fr_morfs):
try:
Expand Down
26 changes: 22 additions & 4 deletions tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,13 @@ def test_empty_reporting(self) -> None:
# empty summary reports raise exception, just like the xml report
cov = coverage.Coverage()
cov.erase()
with pytest.raises(NoDataError, match="No data to report."):
with pytest.raises(
NoDataError,
match=(
r"^The data file or directory '(.+?)' could not be found\. Perhaps 'coverage "
+ r"combine' must be run first\.$"
)
):
cov.report()

def test_completely_zero_reporting(self) -> None:
Expand Down Expand Up @@ -446,7 +452,13 @@ def test_combining_twice(self) -> None:
self.assert_exists(".coverage")

cov2 = coverage.Coverage()
with pytest.raises(NoDataError, match=r"No data to combine"):
with pytest.raises(
NoDataError,
match=(
r"^The data directory '(.+?)' does not contain any data files. Perhaps 'coverage "
+ r"combine' must be run first.$"
)
):
cov2.combine(strict=True, keep=False)

cov3 = coverage.Coverage()
Expand Down Expand Up @@ -1326,7 +1338,7 @@ def test_combine_parallel_data(self) -> None:
# Running combine again should fail, because there are no parallel data
# files to combine.
cov = coverage.Coverage()
with pytest.raises(NoDataError, match=r"No data to combine"):
with pytest.raises(NoDataError):
cov.combine(strict=True)

# And the originally combined data is still there.
Expand Down Expand Up @@ -1376,7 +1388,13 @@ def test_combine_no_usable_files(self) -> None:
# Combine the parallel coverage data files into .coverage, but nothing is readable.
cov = coverage.Coverage()
with pytest.warns(Warning) as warns:
with pytest.raises(NoDataError, match=r"No usable data files"):
with pytest.raises(
NoDataError,
match=(
r"^The following data files are unusable, perhaps because they do not contain "
+ r"valid coverage information:\n- '(.+?)'\n- '(.+?)'$"
)
):
cov.combine(strict=True)

warn_rx = re.compile(
Expand Down
24 changes: 21 additions & 3 deletions tests/test_coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -1635,19 +1635,37 @@ class ReportingTest(CoverageTest):
def test_no_data_to_report_on_annotate(self) -> None:
# Reporting with no data produces a nice message and no output
# directory.
with pytest.raises(NoDataError, match="No data to report."):
with pytest.raises(
NoDataError,
match=(
r"^The data file or directory '(.+?)' could not be found\. Perhaps 'coverage "
+ r"combine' must be run first\.$"
)
):
self.command_line("annotate -d ann")
self.assert_doesnt_exist("ann")

def test_no_data_to_report_on_html(self) -> None:
# Reporting with no data produces a nice message and no output
# directory.
with pytest.raises(NoDataError, match="No data to report."):
with pytest.raises(
NoDataError,
match=(
r"^The data file or directory '(.+?)' could not be found\. Perhaps 'coverage "
+ r"combine' must be run first\.$"
)
):
self.command_line("html -d htmlcov")
self.assert_doesnt_exist("htmlcov")

def test_no_data_to_report_on_xml(self) -> None:
# Reporting with no data produces a nice message.
with pytest.raises(NoDataError, match="No data to report."):
with pytest.raises(
NoDataError,
match=(
r"^The data file or directory '(.+?)' could not be found\. Perhaps 'coverage "
+ r"combine' must be run first\.$"
)
):
self.command_line("xml")
self.assert_doesnt_exist("coverage.xml")
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 = "Couldn't combine from non-existent path 'xyzzy'"
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
8 changes: 7 additions & 1 deletion tests/test_html.py
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,13 @@ def test_dothtml_not_python(self) -> None:
self.make_file("innocuous.html", "<h1>This isn't python at all!</h1>")
cov = coverage.Coverage()
cov.load()
with pytest.raises(NoDataError, match="No data to report."):
with pytest.raises(
NoDataError,
match=(
r"^The data file or directory '(.+?)' could not be found\. Perhaps 'coverage "
+ r"combine' must be run first\.$"
)
):
cov.html_report()

def test_execed_liar_ignored(self) -> None:
Expand Down
8 changes: 7 additions & 1 deletion tests/test_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -1143,7 +1143,13 @@ class FailUnderNoFilesTest(CoverageTest):
def test_report(self) -> None:
self.make_file(".coveragerc", "[report]\nfail_under = 99\n")
st, out = self.run_command_status("coverage report")
assert 'No data to report.' in out
assert re.match(
(
r"The data file or directory '([^']+?)' could not be found\. Perhaps 'coverage "
+ r"combine' must be run first\."
),
out
)
assert st == 1


Expand Down
Loading