From 0111fc19d8bb88b128f6fe08e8537a58ccc257f9 Mon Sep 17 00:00:00 2001 From: John Turner Date: Mon, 19 Feb 2024 19:32:45 -0500 Subject: [PATCH 1/5] dependencies.py: introduce unit testing for graph_reverse_depends This commit introduces a new file with a basic unit test for the graph_reverse_depends method on the Dependencies class. Only 1 test exists right now, but various setup code and the general pattern of the tests are valuable for creating more advanced tests, and more tests for the Dependencies class in general. Pytest is used to run all of the functions in the file that start with test_, and a monkeypatch object is passed into test cases and allows us to mock out methods on the Dependencies class, specifically the "environment" method, which is used to query for packages variables like DEPEND and RDEPEND. We are able to test against a small fake denendency graph by patching the method! Signed-off-by: John Turner --- pym/gentoolkit/test/test_dependencies.py | 71 ++++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 pym/gentoolkit/test/test_dependencies.py diff --git a/pym/gentoolkit/test/test_dependencies.py b/pym/gentoolkit/test/test_dependencies.py new file mode 100644 index 00000000..48e9507f --- /dev/null +++ b/pym/gentoolkit/test/test_dependencies.py @@ -0,0 +1,71 @@ +import portage +from typing import List, Dict, Optional +from pytest import MonkeyPatch +from gentoolkit.dependencies import Dependencies + + +def is_cp_in_cpv(cp: str, cpv: str) -> bool: + other_cp, _, _ = portage.pkgsplit(cpv) + return cp == other_cp + + +def environment( + self: Dependencies, + env_vars: List[str], + fake_depends: Dict[str, Optional[Dict[str, str]]], + fake_pkgs: List[str], +) -> List[str]: + metadata = None + for pkg in fake_pkgs: + if is_cp_in_cpv(self.cp, pkg): + if (metadata := fake_depends[pkg]) is not None: + break + else: + return [""] + results = list() + for env_var in env_vars: + try: + value = metadata[env_var] + except KeyError: + value = "" + results.append(value) + return results + + +def test_basic_revdeps(monkeypatch: MonkeyPatch) -> None: + fake_depends = { + "app-misc/root-1.0": None, + "app-misc/a-1.0": {"DEPEND": "app-misc/root"}, + "app-misc/b-1.0": {"DEPEND": "app-misc/a"}, + "app-misc/c-1.0": {"DEPEND": "app-misc/b"}, + "app-misc/d-1.0": None, + } + fake_pkgs = list(fake_depends.keys()) + + def e(self, env_vars): + return environment(self, env_vars, fake_depends, fake_pkgs) + + monkeypatch.setattr(Dependencies, "environment", e) + + # confirm that monkeypatch is working as expected + assert Dependencies("app-misc/root").environment(["DEPEND"]) == [""] + assert Dependencies("app-misc/a").environment(["DEPEND"]) == ["app-misc/root"] + assert Dependencies("app-misc/b").environment(["DEPEND"]) == ["app-misc/a"] + assert Dependencies("app-misc/c").environment(["DEPEND"]) == ["app-misc/b"] + assert Dependencies("app-misc/d").environment(["DEPEND"]) == [""] + + assert sorted( + pkg.cpv + for pkg in Dependencies("app-misc/root").graph_reverse_depends(pkgset=fake_pkgs) + ) == ["app-misc/a-1.0"] + + assert sorted( + pkg.cpv + for pkg in Dependencies("app-misc/root").graph_reverse_depends( + pkgset=fake_pkgs, only_direct=False + ) + ) == [ + "app-misc/a-1.0", + "app-misc/b-1.0", + "app-misc/c-1.0", + ] From 58913f59f66878b9a611adff3c43ffe4c134d364 Mon Sep 17 00:00:00 2001 From: John Turner Date: Thu, 22 Feb 2024 15:50:40 -0500 Subject: [PATCH 2/5] dependencies.py: rewrite graph_reverse_depends to pass tests The graph_reverse_depends method was not able to pass the unit tests introduced in the previous commits. It has been rewritten to pass them. This also has adding types to the method, and yields the results as an iterator rather than collecting them into a list in one shot. The printer callback parameter has been removed. This callback most likely existed so that results would be shown to the user as soon as they were available instead of delaying printing until the method completed, which could take seconds or minutes depending on the parameters. By making this method an iterator, the same effect is acheived by having the caller print every item as its yielded from the method. Signed-off-by: John Turner --- pym/gentoolkit/dependencies.py | 102 ++++++++++++--------------------- 1 file changed, 36 insertions(+), 66 deletions(-) diff --git a/pym/gentoolkit/dependencies.py b/pym/gentoolkit/dependencies.py index c6abff0b..be5c71fb 100644 --- a/pym/gentoolkit/dependencies.py +++ b/pym/gentoolkit/dependencies.py @@ -14,7 +14,7 @@ import itertools from functools import cache from enum import Enum -from typing import List, Dict +from typing import List, Dict, Iterable, Iterator, Set, Optional, Any, Union import portage from portage.dep import paren_reduce @@ -22,6 +22,7 @@ from gentoolkit import errors from gentoolkit.atom import Atom from gentoolkit.query import Query +from gentoolkit.cpv import CPV # ======= # Classes @@ -52,10 +53,11 @@ class Dependencies(Query): """ - def __init__(self, query, parser=None): + def __init__(self, query: Any, parser: Any = None) -> None: Query.__init__(self, query) - self.use = [] - self.depatom = "" + self.use: List[str] = [] + self.depatom: Optional[Atom] = None + self.depth: Optional[int] = None # Allow a custom parser function: self.parser = parser if parser else self._parser @@ -185,15 +187,13 @@ def graph_depends( def graph_reverse_depends( self, - pkgset=None, - max_depth=-1, - only_direct=True, - printer_fn=None, + pkgset: Iterable[Union[str, CPV]], + max_depth: Optional[int] = None, + only_direct: bool = True, # The rest of these are only used internally: - depth=0, - seen=None, - result=None, - ): + depth: int = 0, + seen: Optional[Set[str]] = None, + ) -> Iterator["Dependencies"]: """Graph direct reverse dependencies for self. Example usage: @@ -201,10 +201,10 @@ def graph_reverse_depends( >>> ffmpeg = Dependencies('media-video/ffmpeg-9999') >>> # I only care about installed packages that depend on me: ... from gentoolkit.helpers import get_installed_cpvs - >>> # I want to pass in a sorted list. We can pass strings or - ... # Package or Atom types, so I'll use Package to sort: + >>> # I want to pass in a list. We can pass strings or + ... # Package or Atom types. ... from gentoolkit.package import Package - >>> installed = sorted(get_installed_cpvs()) + >>> installed = get_installed_cpvs() >>> deptree = ffmpeg.graph_reverse_depends( ... only_direct=False, # Include indirect revdeps ... pkgset=installed) # from installed pkgset @@ -212,82 +212,52 @@ def graph_reverse_depends( 24 @type pkgset: iterable - @keyword pkgset: sorted pkg cpv strings or anything sublassing + @keyword pkgset: pkg cpv strings or anything sublassing L{gentoolkit.cpv.CPV} to use for calculate our revdep graph. - @type max_depth: int + @type max_depth: optional @keyword max_depth: Maximum depth to recurse if only_direct=False. - -1 means no maximum depth; - 0 is the same as only_direct=True; + None means no maximum depth; + 0 is the same as only_direct=True; >0 means recurse only this many times; @type only_direct: bool @keyword only_direct: to recurse or not to recurse - @type printer_fn: callable - @keyword printer_fn: If None, no effect. If set, it will be applied to - each L{gentoolkit.atom.Atom} object as it is added to the results. - @rtype: list + @rtype: iterable @return: L{gentoolkit.dependencies.Dependencies} objects """ - if not pkgset: - err = ( - "%s kwarg 'pkgset' must be set. " - "Can be list of cpv strings or any 'intersectable' object." - ) - raise errors.GentoolkitFatalError(err % (self.__class__.__name__,)) if seen is None: seen = set() - if result is None: - result = list() - if depth == 0: - pkgset = tuple(Dependencies(x) for x in pkgset) - - pkgdep = None - for pkgdep in pkgset: + for pkgdep in (Dependencies(pkg) for pkg in pkgset): if self.cp not in pkgdep.get_raw_depends(): # fast path for obviously non-matching packages. This saves # us the work of instantiating a whole Atom() for *every* # dependency of *every* package in pkgset. continue - all_depends = pkgdep.get_all_depends() - dep_is_displayed = False - for dep in all_depends: - # TODO: Add ability to determine if dep is enabled by USE flag. - # Check portage.dep.use_reduce + found_match = False + for dep in pkgdep.get_all_depends(): if dep.intersects(self): + pkgdep.depatom = dep pkgdep.depth = depth - pkgdep.matching_dep = dep - if printer_fn is not None: - printer_fn(pkgdep, dep_is_displayed=dep_is_displayed) - result.append(pkgdep) - dep_is_displayed = True - - # if --indirect specified, call ourselves again with the dep - # Do not call if we have already called ourselves. + yield pkgdep + found_match = True + if ( - dep_is_displayed - and not only_direct + found_match and pkgdep.cpv not in seen - and (depth < max_depth or max_depth == -1) + and only_direct is False + and (max_depth is None or depth < max_depth) ): seen.add(pkgdep.cpv) - result.append( - pkgdep.graph_reverse_depends( - pkgset=pkgset, - max_depth=max_depth, - only_direct=only_direct, - printer_fn=printer_fn, - depth=depth + 1, - seen=seen, - result=result, - ) + yield from pkgdep.graph_reverse_depends( + pkgset=pkgset, + only_direct=False, + max_depth=max_depth, + depth=depth + 1, + seen=seen, ) - if depth == 0: - return result - return pkgdep - def _parser(self, deps, use_conditional=None, depth=0): """?DEPEND file parser. From 3eedde76f683db83ec365596f220693786353bcf Mon Sep 17 00:00:00 2001 From: John Turner Date: Thu, 22 Feb 2024 20:38:29 -0500 Subject: [PATCH 3/5] equery/depends: print output in module rather than with a callback The depends module can now iterate over the results of the graph_reverse_depends function and print the items as they are yielded. Before, it passed in a callback printer function, and expected the Dependencies class to call it correctly. This setup is nicer because it does not tie together this module and the Dependencies class, and the old setup most likely existed due to performance and interactivity concerns which are now fixed by turning graph_reverse_depends into an iterator. Signed-off-by: John Turner --- pym/gentoolkit/equery/depends.py | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/pym/gentoolkit/equery/depends.py b/pym/gentoolkit/equery/depends.py index 8ec5f755..f92b7b97 100644 --- a/pym/gentoolkit/equery/depends.py +++ b/pym/gentoolkit/equery/depends.py @@ -17,7 +17,6 @@ from gentoolkit.dependencies import Dependencies from gentoolkit.equery import format_options, mod_usage, CONFIG from gentoolkit.helpers import get_cpvs, get_installed_cpvs -from gentoolkit.cpv import CPV from gentoolkit.package import PackageFormatter, Package # ======= @@ -27,7 +26,7 @@ QUERY_OPTS = { "include_masked": False, "only_direct": True, - "max_depth": -1, + "max_depth": None, "package_format": None, } @@ -94,9 +93,9 @@ def format_depend(self, dep, dep_is_displayed): if dep_is_displayed and not self.verbose: return - depth = getattr(dep, "depth", 0) + depth = dep.depth indent = " " * depth - mdep = dep.matching_dep + mdep = dep.depatom use_conditional = "" if QUERY_OPTS["package_format"] != None: @@ -226,17 +225,25 @@ def main(input_args): if CONFIG["verbose"]: print(" * These packages depend on %s:" % pp.emph(pkg.cpv)) - if pkg.graph_reverse_depends( - pkgset=sorted(pkggetter(), key=CPV), - max_depth=QUERY_OPTS["max_depth"], + + first_run = False + + last_seen = None + for pkgdep in pkg.graph_reverse_depends( + pkgset=sorted(pkggetter()), only_direct=QUERY_OPTS["only_direct"], - printer_fn=dep_print, + max_depth=QUERY_OPTS["max_depth"], ): + if last_seen is None or last_seen != pkgdep: + seen = False + else: + seen = True + printer(pkgdep, dep_is_displayed=seen) + last_seen = pkgdep + if last_seen is not None: got_match = True - first_run = False - - if not got_match: + if got_match is None: sys.exit(1) From af42842af7c67a2e8383f7367134ec47d0992dd8 Mon Sep 17 00:00:00 2001 From: John Turner Date: Tue, 27 Feb 2024 15:20:11 -0500 Subject: [PATCH 4/5] depends.py: rename DependPrinter to Printer The DependPrinter class name and documentation indicated that it was meant to be part of the gentoolkit.dependencies API, to print Dependencies objects. This Printer class is used exclusively for printering equery depends output and is not a public API outside of the depends.py module Signed-off-by: John Turner --- pym/gentoolkit/equery/depends.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pym/gentoolkit/equery/depends.py b/pym/gentoolkit/equery/depends.py index f92b7b97..39e0b255 100644 --- a/pym/gentoolkit/equery/depends.py +++ b/pym/gentoolkit/equery/depends.py @@ -35,8 +35,8 @@ # ======= -class DependPrinter: - """Output L{gentoolkit.dependencies.Dependencies} objects.""" +class Printer: + """Output L{gentoolkit.dependencies.Dependencies} objects for equery depends.""" def __init__(self, verbose=True): self.verbose = verbose @@ -83,7 +83,7 @@ def print_formated(pkg): ) def format_depend(self, dep, dep_is_displayed): - """Format a dependency for printing. + """Format a dependency for printing for equery depends. @type dep: L{gentoolkit.dependencies.Dependencies} @param dep: the dependency to display @@ -209,7 +209,7 @@ def main(input_args): # Output # - dep_print = DependPrinter(verbose=CONFIG["verbose"]) + printer = Printer(verbose=CONFIG["verbose"]) first_run = True got_match = False From 63f384810b46fc96db7aa9692541173149f2a2a8 Mon Sep 17 00:00:00 2001 From: John Turner Date: Sun, 3 Mar 2024 19:22:32 -0500 Subject: [PATCH 5/5] switch test framework to pytest Pytest is a testing framework that is backwards compatible with "unittest" tests, but provides new styles of tests that are more ergonomic. Pytest tests do not require wrapping the test in a class, just a top level python function will be automatically picked up. Assertions use the regular python assert built-in and provide greatly enhanced debug output. These features reduce friction in writing new unit tests, and being backwards compatible allows preserving the existing gentoolkit unit tests. Changing the meson test command and installing the pytest package in CI are the only changes required to start using it! Signed-off-by: John Turner --- .github/workflows/ci.yml | 2 +- meson.build | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bcb2d59c..803dfde1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,7 +33,7 @@ jobs: python -m site python -m pip install --upgrade pip # setuptools needed for 3.12+ because of https://github.com/mesonbuild/meson/issues/7702. - python -m pip install meson ninja setuptools + python -m pip install meson ninja setuptools pytest - name: Install portage run: | mkdir portage diff --git a/meson.build b/meson.build index c7717389..c3e83c58 100644 --- a/meson.build +++ b/meson.build @@ -39,12 +39,8 @@ endif subdir('bin') subdir('pym') -test( - 'python-unittest', - py, - args : ['-m', 'unittest', 'discover', meson.current_source_dir() / 'pym'], - timeout : 0 -) +pytest = find_program('pytest') +test('pytest', pytest, args : ['-v', meson.current_source_dir() / 'pym']) if get_option('code-only') subdir_done()