From 37f8f6681c761a5788b633f1da8f1f8a940bfabc Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 19 Oct 2023 02:24:29 +0200 Subject: [PATCH] tests.nixpkgs-check-by-name: Intermediate error type refactoring prep Currently the tool prints problems right as it is checking the code without an intermediate error representation. However for various reasons it would be beneficial to have an intermediate error type: - It makes the code cleaner, having all errors in one place - It allows printing the error in different ways, e.g. for a future --json mode This commit prepares for an incremental refactoring for an intermediate error/problem representation. Most notable is that we want to be able to collect multiple errors/problems and not just exit on the first one. We introduce the type alias CheckResult and CheckError (later renamed to NixpkgsProblem), where CheckError allows collecting multiple CheckErrors using the utility function flatten_check_results (later renamed to check_result::sequence) The write_check_result function is only temporarily introduced to help refactoring, it's removed again in later commits. --- pkgs/test/nixpkgs-check-by-name/Cargo.lock | 16 +++++ pkgs/test/nixpkgs-check-by-name/Cargo.toml | 1 + .../nixpkgs-check-by-name/src/check_result.rs | 61 +++++++++++++++++++ pkgs/test/nixpkgs-check-by-name/src/main.rs | 1 + 4 files changed, 79 insertions(+) create mode 100644 pkgs/test/nixpkgs-check-by-name/src/check_result.rs diff --git a/pkgs/test/nixpkgs-check-by-name/Cargo.lock b/pkgs/test/nixpkgs-check-by-name/Cargo.lock index aa4459c7cff8..fc3aeb9fd79b 100644 --- a/pkgs/test/nixpkgs-check-by-name/Cargo.lock +++ b/pkgs/test/nixpkgs-check-by-name/Cargo.lock @@ -162,6 +162,12 @@ version = "3.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7704b5fdd17b18ae31c4c1da5a2e0305a2bf17b5249300a9ee9ed7b72114c636" +[[package]] +name = "either" +version = "1.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a26ae43d7bcc3b814de94796a5e736d4029efb0ee900c12e2d54c993ad1a1e07" + [[package]] name = "errno" version = "0.3.2" @@ -218,6 +224,15 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "itertools" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1c173a5686ce8bfa551b3563d0c2170bf24ca44da99c7ca4bfdab5418c3fe57" +dependencies = [ + "either", +] + [[package]] name = "itoa" version = "1.0.9" @@ -274,6 +289,7 @@ dependencies = [ "anyhow", "clap", "colored", + "itertools", "lazy_static", "regex", "rnix", diff --git a/pkgs/test/nixpkgs-check-by-name/Cargo.toml b/pkgs/test/nixpkgs-check-by-name/Cargo.toml index 70b44d048200..1e6eaa1106d5 100644 --- a/pkgs/test/nixpkgs-check-by-name/Cargo.toml +++ b/pkgs/test/nixpkgs-check-by-name/Cargo.toml @@ -13,6 +13,7 @@ serde = { version = "1.0.185", features = ["derive"] } anyhow = "1.0" lazy_static = "1.4.0" colored = "2.0.4" +itertools = "0.11.0" [dev-dependencies] temp-env = "0.3.5" diff --git a/pkgs/test/nixpkgs-check-by-name/src/check_result.rs b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs new file mode 100644 index 000000000000..df894df45c71 --- /dev/null +++ b/pkgs/test/nixpkgs-check-by-name/src/check_result.rs @@ -0,0 +1,61 @@ +use crate::ErrorWriter; +use itertools::{Either, Itertools}; +use std::fmt; +use std::io; +use std::path::PathBuf; + +pub enum CheckError {} + +impl CheckError { + pub fn into_result(self) -> CheckResult { + Ok(Either::Left(vec![self])) + } +} + +impl fmt::Display for CheckError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match *self {} + } +} + +pub fn write_check_result( + error_writer: &mut ErrorWriter, + check_result: CheckResult, +) -> anyhow::Result> { + match check_result? { + Either::Left(errors) => { + for error in errors { + error_writer.write(&error.to_string())? + } + Ok(None) + } + Either::Right(value) => Ok(Some(value)), + } +} + +pub fn pass(value: A) -> CheckResult { + Ok(Either::Right(value)) +} + +pub type CheckResult = anyhow::Result, A>>; + +pub fn flatten_check_results( + check_results: impl IntoIterator>, + value_transform: impl Fn(Vec) -> O, +) -> CheckResult { + let (errors, values): (Vec<_>, Vec<_>) = check_results + .into_iter() + .collect::>>()? + .into_iter() + .partition_map(|r| r); + + // To combine the errors from the results we flatten all the error Vec's into a new Vec + // This is not very efficient, but doesn't matter because generally we should have no errors + let flattened_errors = errors.into_iter().flatten().collect::>(); + + if flattened_errors.is_empty() { + Ok(Either::Right(value_transform(values))) + } else { + Ok(Either::Left(flattened_errors)) + } +} diff --git a/pkgs/test/nixpkgs-check-by-name/src/main.rs b/pkgs/test/nixpkgs-check-by-name/src/main.rs index 5d28077ae4ae..ee98c5a1a893 100644 --- a/pkgs/test/nixpkgs-check-by-name/src/main.rs +++ b/pkgs/test/nixpkgs-check-by-name/src/main.rs @@ -1,3 +1,4 @@ +mod check_result; mod eval; mod references; mod structure;