From d626f8544e0b4ec530df0012d4e329d9634a4805 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20M=C3=B6ller?= Date: Wed, 10 Apr 2024 15:53:02 +0200 Subject: [PATCH 1/3] Add flag to build package passthru.tests --- README.md | 4 ++++ nixpkgs_review/cli/__init__.py | 5 ++++ nixpkgs_review/cli/pr.py | 1 + nixpkgs_review/nix.py | 8 +++++-- nixpkgs_review/nix/evalAttrs.nix | 19 ++++++++++++++-- nixpkgs_review/report.py | 2 +- nixpkgs_review/review.py | 39 ++++++++++++++++++++++++++++++-- 7 files changed, 71 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index b45e27ca..fa7c1daf 100644 --- a/README.md +++ b/README.md @@ -299,6 +299,10 @@ attribute set: $ nixpkgs-review pr -p nixosTests.ferm 47077 ``` +Many packages also specify their associated `nixosTests` in the `passthru.tests` +attribute set. Adding the `--tests` argument will run those tests for all +packages that will be rebuild. + ## Ignoring ofborg evaluations By default, nixpkgs-review will use ofborg's evaluation result if available to diff --git a/nixpkgs_review/cli/__init__.py b/nixpkgs_review/cli/__init__.py index a87c4cd1..c9a31e9f 100644 --- a/nixpkgs_review/cli/__init__.py +++ b/nixpkgs_review/cli/__init__.py @@ -182,6 +182,11 @@ def common_flags() -> list[CommonFlag]: type=regex_type, help="Regular expression that package attributes have to match (can be passed multiple times)", ), + CommonFlag( + "--tests", + action="store_true", + help="For all packages to be built, also build their `passthru.tests`", + ), CommonFlag( "-r", "--remote", diff --git a/nixpkgs_review/cli/pr.py b/nixpkgs_review/cli/pr.py index aef9e38b..6daeccf1 100644 --- a/nixpkgs_review/cli/pr.py +++ b/nixpkgs_review/cli/pr.py @@ -72,6 +72,7 @@ def pr_command(args: argparse.Namespace) -> str: build_graph=args.build_graph, nixpkgs_config=nixpkgs_config, extra_nixpkgs_config=args.extra_nixpkgs_config, + build_passthru_tests=args.tests, ) contexts.append((pr, builddir.path, review.build_pr(pr))) except NixpkgsReviewError as e: diff --git a/nixpkgs_review/nix.py b/nixpkgs_review/nix.py index 002f6f0c..ed0d423e 100644 --- a/nixpkgs_review/nix.py +++ b/nixpkgs_review/nix.py @@ -39,7 +39,7 @@ def was_build(self) -> bool: return self._path_verified def is_test(self) -> bool: - return self.name.startswith("nixosTests") + return self.name.startswith("nixosTests") or ".passthru.tests." in self.name REVIEW_SHELL: Final[str] = str(ROOT.joinpath("nix/review-shell.nix")) @@ -217,6 +217,7 @@ def nix_eval( system: str, allow: AllowedFeatures, nix_path: str, + include_passthru_tests: bool = False, ) -> list[Attr]: attr_json = NamedTemporaryFile(mode="w+", delete=False) delete = True @@ -239,7 +240,10 @@ def nix_eval( if allow.ifd else "--no-allow-import-from-derivation", "--expr", - f"(import {eval_script} {{ attr-json = {attr_json.name}; }})", + f"""(import {eval_script} {{ + attr-json = {attr_json.name}; + include-passthru-tests = {str(include_passthru_tests).lower()}; + }})""", ] nix_eval = subprocess.run(cmd, stdout=subprocess.PIPE, text=True) diff --git a/nixpkgs_review/nix/evalAttrs.nix b/nixpkgs_review/nix/evalAttrs.nix index 01e5ff3a..5cc7e736 100644 --- a/nixpkgs_review/nix/evalAttrs.nix +++ b/nixpkgs_review/nix/evalAttrs.nix @@ -1,4 +1,6 @@ -{ attr-json }: +{ attr-json +, include-passthru-tests +}: with builtins; let @@ -27,7 +29,7 @@ let }) ] else - lib.flip map pkg.outputs or [ "out" ] (output: + (lib.flip map pkg.outputs or [ "out" ] (output: let # some packages are set to null if they aren't compatible with a platform or package set maybePath = tryEval "${lib.getOutput output pkg}"; @@ -40,6 +42,19 @@ let path = if !broken then maybePath.value else null; drvPath = if !broken then pkg.drvPath else null; } + )) + ++ lib.optionals include-passthru-tests ( + lib.flip lib.mapAttrsToList pkg.passthru.tests or { } (test: drv: + let + maybePath = tryEval "${drv}"; + broken = !exists || !maybePath.success; + in + lib.nameValuePair "${name}.passthru.tests.${test}" { + inherit exists broken; + path = if !broken then maybePath.value else null; + drvPath = if !broken then pkg.drvPath else null; + } + ) ); in diff --git a/nixpkgs_review/report.py b/nixpkgs_review/report.py index 12b77662..5cbce93f 100644 --- a/nixpkgs_review/report.py +++ b/nixpkgs_review/report.py @@ -113,7 +113,7 @@ def __init__( self.blacklisted.append(a) elif not a.exists: self.non_existent.append(a) - elif a.name.startswith("nixosTests."): + elif a.is_test(): self.tests.append(a) elif not a.was_build(): self.failed.append(a) diff --git a/nixpkgs_review/review.py b/nixpkgs_review/review.py index 84e72f26..cc8390ae 100644 --- a/nixpkgs_review/review.py +++ b/nixpkgs_review/review.py @@ -100,6 +100,7 @@ def __init__( skip_packages_regex: list[Pattern[str]] = [], checkout: CheckoutOption = CheckoutOption.MERGE, sandbox: bool = False, + build_passthru_tests: bool = False, ) -> None: self.builddir = builddir self.build_args = build_args @@ -119,6 +120,7 @@ def __init__( self.build_graph = build_graph self.nixpkgs_config = nixpkgs_config self.extra_nixpkgs_config = extra_nixpkgs_config + self.build_passthru_tests = build_passthru_tests def worktree_dir(self) -> str: return str(self.builddir.worktree_dir) @@ -205,6 +207,7 @@ def build(self, packages: set[str], args: str) -> list[Attr]: self.system, self.allow, self.builddir.nix_path, + self.build_passthru_tests, ) return nix_build( packages, @@ -409,12 +412,19 @@ def package_attrs( allow: AllowedFeatures, nix_path: str, ignore_nonexisting: bool = True, + build_passthru_tests: bool = False, ) -> dict[str, Attr]: attrs: dict[str, Attr] = {} nonexisting = [] - for attr in nix_eval(package_set, system, allow, nix_path): + for attr in nix_eval( + package_set, + system, + allow, + nix_path, + build_passthru_tests, + ): if not attr.exists: nonexisting.append(attr.name) elif not attr.broken: @@ -434,14 +444,21 @@ def join_packages( system: str, allow: AllowedFeatures, nix_path: str, + build_passthru_tests: bool, ) -> set[str]: - changed_attrs = package_attrs(changed_packages, system, allow, nix_path) + changed_attrs = package_attrs( + changed_packages, + system, + allow, + nix_path, + ) specified_attrs = package_attrs( specified_packages, system, allow, nix_path, ignore_nonexisting=False, + build_passthru_tests=build_passthru_tests, ) tests: dict[str, Attr] = {} @@ -472,9 +489,24 @@ def filter_packages( system: str, allow: AllowedFeatures, nix_path: str, + build_passthru_tests: bool, ) -> set[str]: packages: set[str] = set() + # reduce number of times the passthru.tests are evaluated, by either doing + # it here or in join_packages + if build_passthru_tests and len(specified_packages) == 0: + changed_attrs = package_attrs( + changed_packages, + system, + allow, + nix_path, + build_passthru_tests=build_passthru_tests, + ) + changed_packages = set( + changed_attrs[path].name for path in changed_attrs.keys() + ) + if ( len(specified_packages) == 0 and len(package_regexes) == 0 @@ -490,6 +522,7 @@ def filter_packages( system, allow, nix_path, + build_passthru_tests, ) for attr in changed_packages: @@ -560,6 +593,7 @@ def review_local_revision( commit: str | None, staged: bool = False, print_result: bool = False, + build_passthru_tests: bool = False, ) -> Path: with Builddir(builddir_path) as builddir: review = Review( @@ -578,6 +612,7 @@ def review_local_revision( build_graph=args.build_graph, nixpkgs_config=nixpkgs_config, extra_nixpkgs_config=args.extra_nixpkgs_config, + build_passthru_tests=args.tests, ) review.review_commit(builddir.path, args.branch, commit, staged, print_result) return builddir.path From eb947a4bc11d92dbc78efb8545ca3b5f66785604 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20M=C3=B6ller?= Date: Sun, 14 Apr 2024 13:15:21 +0200 Subject: [PATCH 2/3] Report failed tests as failed build Until now, failed tests would not be reported separately and simply shown in the "tests build" list. --- nixpkgs_review/report.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nixpkgs_review/report.py b/nixpkgs_review/report.py index 5cbce93f..2556f916 100644 --- a/nixpkgs_review/report.py +++ b/nixpkgs_review/report.py @@ -113,10 +113,10 @@ def __init__( self.blacklisted.append(a) elif not a.exists: self.non_existent.append(a) - elif a.is_test(): - self.tests.append(a) elif not a.was_build(): self.failed.append(a) + elif a.is_test(): + self.tests.append(a) else: self.built.append(a) From fccd11aaa29367e351f572270ef0068f2d7ff170 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20M=C3=B6ller?= Date: Sun, 14 Apr 2024 13:15:45 +0200 Subject: [PATCH 3/3] Fix typo in console report --- nixpkgs_review/report.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nixpkgs_review/report.py b/nixpkgs_review/report.py index 2556f916..2dc3833f 100644 --- a/nixpkgs_review/report.py +++ b/nixpkgs_review/report.py @@ -189,5 +189,5 @@ def print_console(self, pr: int | None) -> None: ) print_number(self.blacklisted, "blacklisted") print_number(self.failed, "failed to build") - print_number(self.tests, "built", what="tests", log=print) + print_number(self.tests, "built", what="test", log=print) print_number(self.built, "built", log=print)