Skip to content

Commit c4994d6

Browse files
committed
feat: test_pkg_scan: output debugging info, allow custom hooks
First: output the source path of the expected result since it's pain trying to debug this if you're an external user. This states exactly what file caused it. For example: Second: add the ability to use python code to filter out results. This can be used in parallel to the normal mechanism, and there are checks to try and block accidental suppression across the testdata. Custom handlers can be added as /handler.py with a 'handler' functor with the signature of typing.Callable[[Result], bool]. Third: fix #760 since it's causative. Between bash 5.2 and 5.3 the error output improved, can't just store a literal error json. Instead a custom bit of python can be written to do the enforcement, which is added here. Fourth: add as much sanity checks against the handlers as I can think of. Being it's python code, addition of a random new handler can lead to missing/unknowns- thus wire things in a way to try maximally to identify the offender so debugging failures like this are less of a heavy lift. Close: #760 Signed-off-by: Brian Harring <ferringb@gmail.com>
1 parent 6d5010d commit c4994d6

3 files changed

Lines changed: 134 additions & 41 deletions

File tree

testdata/data/repos/eclass/EclassCheck/EclassBashSyntaxError/expected.json

Lines changed: 0 additions & 1 deletion
This file was deleted.
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import copy
2+
3+
# TODO: rip this out and just integrate json registries properly
4+
from pkgcheck.reporters import JsonStream
5+
6+
old_bash_result = list(
7+
JsonStream.from_iter(
8+
[
9+
'{"__class__": "EclassBashSyntaxError", "eclass": "bad", "lineno": "12", "error": "syntax error: unexpected end of file"}'
10+
]
11+
)
12+
)[0]
13+
14+
new_bash_result = copy.deepcopy(old_bash_result)
15+
new_bash_result.error = "syntax error: unexpected end of file from `{' command on line 11" # pyright: ignore[reportAttributeAccessIssue]
16+
17+
18+
def handler(result) -> bool:
19+
return result == old_bash_result or result == new_bash_result

tests/scripts/test_pkgcheck_scan.py

Lines changed: 115 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import contextlib
1+
import importlib
2+
import importlib.machinery
23
import io
34
import os
45
import pathlib
@@ -594,13 +595,37 @@ def _scan_results(self, repo, tmp_path, verbosity):
594595

595596
@dataclass
596597
class _expected_data_result:
597-
expected: typing.Iterable[Result]
598-
expected_verbose: typing.Iterable[Result]
598+
expected: dict[Result, pathlib.Path]
599+
expected_verbose: dict[Result, pathlib.Path]
600+
custom_filter: typing.Callable[[Result], bool] | None
599601

600-
def _load_expected_data(self, path: str) -> _expected_data_result:
602+
def _load_expected_data(self, base: pathlib.Path) -> _expected_data_result:
601603
"""Return the set of result objects from a given json stream file."""
602604

603-
def boilerplate(path, allow_missing: bool) -> list[Result]:
605+
custom_handler = None
606+
try:
607+
with (custom_handler_path := base / "handler.py").open() as f:
608+
# We can't import since it's not a valid python directory layout, nor do
609+
# want to pollute the namespace.
610+
module = importlib.machinery.SourceFileLoader(
611+
"handler", str(custom_handler_path)
612+
).load_module()
613+
if (
614+
custom_handler := typing.cast(
615+
typing.Callable[[Result], bool], getattr(module, "handler")
616+
)
617+
) is None:
618+
pytest.fail(
619+
f"custom python handler {custom_handler_path!r} lacks the necessary handle function or list of handlers"
620+
)
621+
622+
if not callable(custom_handler):
623+
pytest.fail(f"{custom_handler_path} handler isn't invokable")
624+
custom_handler.__source_path__ = custom_handler_path # pyright: ignore[reportFunctionMemberAccess]
625+
except FileNotFoundError:
626+
pass
627+
628+
def boilerplate(path, allow_missing: bool):
604629
try:
605630
with path.open() as f:
606631
data = list(reporters.JsonStream.from_iter(f))
@@ -614,22 +639,25 @@ def boilerplate(path, allow_missing: bool) -> list[Result]:
614639
# Remove this after cleaning the scan/fix logic up to not force duplicate
615640
# renders, and instead just work with a result stream directly.
616641
assert self._render_results(data), f"failed rendering results {data!r}"
617-
return data
642+
return typing.cast(dict[Result, pathlib.Path], {}.fromkeys(data, path))
618643

619644
except FileNotFoundError:
620645
if not allow_missing:
621646
raise
622-
return []
623-
624-
expected_path = self.repos_data / path / "expected.json"
625-
626-
expected = boilerplate(expected_path, False)
627-
assert expected, f"regular results must always exist if the file exists: {expected_path}"
647+
return {}
648+
649+
expected_path = base / "expected.json"
650+
# if a custom handler exists, then the json definitions aren't required.
651+
expected = boilerplate(expected_path, custom_handler is not None)
652+
if custom_handler is None:
653+
assert expected, (
654+
f"regular results must always exist if the file exists: {expected_path}"
655+
)
628656

629-
expected_verbose_path = self.repos_data / path / "expected-verbose.json"
657+
expected_verbose_path = base / "expected-verbose.json"
630658
expected_verbose = boilerplate(expected_verbose_path, True)
631659

632-
return self._expected_data_result(expected, expected_verbose=expected_verbose)
660+
return self._expected_data_result(expected, expected_verbose, custom_handler)
633661

634662
def _render_results(self, results, **kwargs) -> str:
635663
"""Render a given set of result objects into their related string form."""
@@ -642,41 +670,88 @@ def _render_results(self, results, **kwargs) -> str:
642670
@pytest.mark.parametrize("repo", repos)
643671
def test_scan_repo(self, repo, tmp_path_factory):
644672
"""Run pkgcheck against test pkgs in bundled repo, verifying result output."""
645-
expected_results = set()
673+
674+
# _sources is so people debugging failures know where the testdata came from. It matters in regards to devex.
675+
expected_results_sources = {}
646676
scan_results = self._scan_results(repo, tmp_path_factory.mktemp("scan"), verbosity=0)
647677

648-
expected_verbose_results = set()
678+
expected_verbose_results_sources = {}
649679
scan_verbose_results = self._scan_results(repo, tmp_path_factory.mktemp("ver"), verbosity=1)
650680

681+
custom_handlers: list[typing.Callable[[Result], bool]] = []
682+
651683
for check, keywords in self._checks[repo].items():
652684
for keyword in keywords:
653-
data = self._load_expected_data(f"{repo}/{check}/{keyword}")
654-
expected_results.update(data.expected)
685+
path = self.repos_data / repo / check / keyword
686+
data = self._load_expected_data(path)
687+
if conflict := {
688+
k: [v, expected_results_sources[k]]
689+
for k, v in data.expected.items()
690+
if k in expected_results_sources
691+
}:
692+
pytest.fail(
693+
f"conflicting results found in testdata for the following fixtures: {conflict!r}"
694+
)
695+
696+
expected_results_sources.update(data.expected)
655697

656698
if data.expected_verbose:
657-
expected_verbose_results.update(data.expected_verbose)
699+
expected_verbose_results_sources.update(data.expected_verbose)
658700
else:
659-
expected_verbose_results.update(data.expected)
660-
661-
if expected_results != scan_results:
662-
missing = self._render_results(expected_results - scan_results)
663-
unknown = self._render_results(scan_results - expected_results)
664-
error = ["unmatched repo scan results:\n\n"]
665-
if missing:
666-
error.append(f"{repo} repo missing expected results:\n{missing}")
667-
if unknown:
668-
error.append(f"{repo} repo unknown results:\n{unknown}")
669-
pytest.fail("\n".join(error), pytrace=False)
670-
671-
if expected_verbose_results != scan_verbose_results:
672-
missing = self._render_results(expected_verbose_results - scan_verbose_results)
673-
unknown = self._render_results(scan_verbose_results - expected_verbose_results)
674-
error = ["unmatched verbose repo scan results:\n\n"]
675-
if missing:
676-
error.append(f"{repo} repo missing expected results:\n{missing}")
677-
if unknown:
678-
error.append(f"{repo} repo unknown results:\n{unknown}")
679-
pytest.fail("\n".join(error), pytrace=False)
701+
expected_verbose_results_sources.update(data.expected)
702+
703+
if data.custom_filter is not None:
704+
custom_handlers.append(data.custom_filter)
705+
706+
for handler in custom_handlers:
707+
try:
708+
# sanity checks; both that they don't intersect expected results from other testdata,
709+
# and also do the filtering.
710+
for verbose_text, expected, actual in (
711+
("", expected_results_sources, scan_results),
712+
("verbose ", expected_verbose_results_sources, scan_verbose_results),
713+
):
714+
if intersection := list(filter(handler, expected)):
715+
pytest.fail(
716+
f"handler from {handler.__source_file__!r} incorrectly suppresses {verbose_text}test data: {intersection}" # pyright: ignore[reportFunctionMemberAccess]
717+
)
718+
719+
for k in list(filter(handler, actual)):
720+
actual.remove(k)
721+
722+
except Exception as e:
723+
pytest.fail(
724+
f"handler {data.custom_filter.__source_path__!r} threw an exception: {e!r}" # type: ignore
725+
)
726+
727+
def assert_same(sources, results, verbose=False):
728+
expected = set(sources)
729+
errors = []
730+
if missing := expected.difference(results):
731+
lines = [
732+
f"from source: {sources[x]}, expected:\n{self._render_results([x])}"
733+
for x in missing
734+
]
735+
errors.append("\n".join(lines))
736+
for handler in custom_handlers:
737+
for result in missing:
738+
if handler(result):
739+
errors.append(
740+
f"possible cause: handler {handler.__source_file__} matches" # pyright: ignore[reportFunctionMemberAccess]
741+
)
742+
743+
if unknown := results.difference(sources):
744+
text = self._render_results(unknown)
745+
errors.append(f"unknown results:\n{text}")
746+
747+
if errors:
748+
verbose = "verbose " if verbose else ""
749+
pytest.fail(
750+
f"repo {repo} {verbose}scan failures:\n" + "\n\n".join(errors), pytrace=False
751+
)
752+
753+
assert_same(expected_results_sources, scan_results)
754+
assert_same(expected_verbose_results_sources, scan_verbose_results, True)
680755

681756
@staticmethod
682757
def _patch(fix, repo_path):

0 commit comments

Comments
 (0)