From e712ab1b55234e9147ee4eb3c682cc232161313b Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 31 Oct 2024 10:00:46 +0000 Subject: [PATCH 01/11] Test runner. --- ast/src/parsed/types.rs | 6 ++++ cli/Cargo.toml | 1 + cli/src/main.rs | 31 +++++++++++++++++++- cli/src/test_runner.rs | 63 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 cli/src/test_runner.rs diff --git a/ast/src/parsed/types.rs b/ast/src/parsed/types.rs index 3a374d054b..49bed0e11d 100644 --- a/ast/src/parsed/types.rs +++ b/ast/src/parsed/types.rs @@ -416,6 +416,12 @@ impl From>> for FunctionType { } } +impl From for Type { + fn from(value: FunctionType) -> Self { + Type::Function(value) + } +} + #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Serialize, Deserialize, JsonSchema)] pub struct TypeScheme { /// Type variables and their trait bounds. diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 04cc991000..097ee5b4d2 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -24,6 +24,7 @@ powdr.workspace = true clap = { version = "^4.3", features = ["derive"] } env_logger = "0.10.0" +itertools = "0.13" log = "0.4.17" strum = { version = "0.24.1", features = ["derive"] } clap-markdown = "0.1.3" diff --git a/cli/src/main.rs b/cli/src/main.rs index 6668ecb9c9..15f32d80ea 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -1,5 +1,6 @@ //! The powdr CLI tool +mod test_runner; mod util; use clap::{CommandFactory, Parser, Subcommand}; @@ -363,6 +364,24 @@ enum Commands { #[arg(value_parser = clap_enum_variants!(FieldArgument))] field: FieldArgument, }, + + /// Executes all functions starting with `test_` in every module called + /// `test` starting from the given module. + Test { + /// Input file. + file: String, + + /// The field to use + #[arg(long)] + #[arg(default_value_t = FieldArgument::Gl)] + #[arg(value_parser = clap_enum_variants!(FieldArgument))] + field: FieldArgument, + + /// Also run the tests inside the standard library. + #[arg(long)] + #[arg(default_value_t = false)] + include_std_tests: bool, + }, } fn split_inputs(inputs: &str) -> Vec { @@ -469,6 +488,13 @@ fn run_command(command: Commands) { csv_mode )) } + Commands::Test { + file, + field, + include_std_tests, + } => { + call_with_field!(run_test::(&file, include_std_tests)) + } Commands::Prove { file, dir, @@ -688,10 +714,13 @@ fn run( .compute_proof() .unwrap(); } - Ok(()) } +fn run_test(file: &str, include_std_tests: bool) -> Result<(), Vec> { + test_runner::run::(file, include_std_tests) +} + #[allow(clippy::too_many_arguments)] fn read_and_prove( file: &Path, diff --git a/cli/src/test_runner.rs b/cli/src/test_runner.rs new file mode 100644 index 0000000000..52f978de08 --- /dev/null +++ b/cli/src/test_runner.rs @@ -0,0 +1,63 @@ +use std::{ + fs, + path::{Path, PathBuf}, + str::FromStr, +}; + +use itertools::Itertools; + +use powdr::ast::{ + analyzed::FunctionValueDefinition, + parsed::{ + asm::SymbolPath, + types::{FunctionType, Type}, + }, +}; +use powdr::pil_analyzer::evaluator::{self, SymbolLookup}; +use powdr::{FieldElement, Pipeline}; + +/// Executes all functions in the given file that start with `test_` and are +/// inside a module called `test`. +/// +/// @param include_std_tests: Whether to run the tests inside the standard library. +pub fn run(input: &str, include_std_tests: bool) -> Result<(), Vec> { + let mut pipeline = Pipeline::::default().from_file(PathBuf::from(&input)); + + let analyzed = pipeline.compute_analyzed_pil()?; + + let mut symbols = evaluator::Definitions { + definitions: &analyzed.definitions, + solved_impls: &analyzed.solved_impls, + }; + + for (name, (_, val)) in analyzed + .definitions + .iter() + .filter(|(n, _)| n.starts_with("test::test_") || n.contains("::test::test_")) + .filter(|(n, _)| include_std_tests || !n.starts_with("std::")) + .filter(|(n, _)| SymbolPath::from_str(n).unwrap().name().starts_with("test_")) + .sorted_by_key(|(n, _)| *n) + { + let Some(FunctionValueDefinition::Expression(f)) = val else { + continue; + }; + // Require a plain `->()` type. + let type_scheme = f.type_scheme.as_ref().unwrap(); + if !type_scheme.vars.is_empty() + || type_scheme.ty + != (FunctionType { + params: vec![], + value: Box::new(Type::empty_tuple()), + }) + .into() + { + continue; + } + print!("Running test: {name}..."); + let function = symbols.lookup(name, &None).unwrap(); + evaluator::evaluate_function_call::(function, vec![], &mut symbols).unwrap(); + println!(" ok"); + } + + Ok(()) +} From 9e80a189ac1e54c51c7804e4f52015e0a354cbed Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 31 Oct 2024 13:34:45 +0000 Subject: [PATCH 02/11] Rename tests, better formatting and continue running. --- cli/src/test_runner.rs | 63 ++++++++++++++++++++++++++++++------------ std/math/fp2.asm | 12 ++++---- std/math/fp4.asm | 8 +++--- 3 files changed, 55 insertions(+), 28 deletions(-) diff --git a/cli/src/test_runner.rs b/cli/src/test_runner.rs index 52f978de08..b366a90870 100644 --- a/cli/src/test_runner.rs +++ b/cli/src/test_runner.rs @@ -1,4 +1,5 @@ use std::{ + collections::BTreeSet, fs, path::{Path, PathBuf}, str::FromStr, @@ -30,34 +31,60 @@ pub fn run(input: &str, include_std_tests: bool) -> Result<(), solved_impls: &analyzed.solved_impls, }; - for (name, (_, val)) in analyzed + let mut errors = vec![]; + let tests: BTreeSet<&String> = analyzed .definitions .iter() .filter(|(n, _)| n.starts_with("test::test_") || n.contains("::test::test_")) .filter(|(n, _)| include_std_tests || !n.starts_with("std::")) .filter(|(n, _)| SymbolPath::from_str(n).unwrap().name().starts_with("test_")) .sorted_by_key(|(n, _)| *n) - { - let Some(FunctionValueDefinition::Expression(f)) = val else { - continue; - }; - // Require a plain `->()` type. - let type_scheme = f.type_scheme.as_ref().unwrap(); - if !type_scheme.vars.is_empty() - || type_scheme.ty - != (FunctionType { + .filter_map(|(n, (_, val))| { + let Some(FunctionValueDefinition::Expression(f)) = val else { + return None; + }; + // Require a plain `->()` type. + (f.type_scheme.as_ref().unwrap().ty + == (FunctionType { params: vec![], value: Box::new(Type::empty_tuple()), }) - .into() - { - continue; - } - print!("Running test: {name}..."); + .into()) + .then_some(n) + }) + .collect(); + println!("Running {} tests...", tests.len()); + println!("{}", "-".repeat(85)); + for name in &tests { + let name_len = name.len(); + let padding = if name_len >= 75 { + " ".to_string() + } else { + " ".repeat(76 - name_len) + }; + print!("{name}..."); let function = symbols.lookup(name, &None).unwrap(); - evaluator::evaluate_function_call::(function, vec![], &mut symbols).unwrap(); - println!(" ok"); + match evaluator::evaluate_function_call::(function, vec![], &mut symbols) { + Err(e) => { + let msg = e.to_string(); + println!("{padding}failed\n {msg}"); + errors.push((name, msg)); + } + Ok(_) => println!("{padding}ok"), + } } - Ok(()) + println!("{}", "-".repeat(85)); + if errors.is_empty() { + println!("All {} tests passed!", tests.len()); + Ok(()) + } else { + println!( + "Failed tests: {} / {}\n{}", + errors.len(), + tests.len(), + errors.iter().map(|(n, e)| format!(" {n}: {e}")).join("\n") + ); + Err(vec![format!("{} test(s) failed.", errors.len())]) + } } diff --git a/std/math/fp2.asm b/std/math/fp2.asm index 1ad18202dc..12202b0940 100644 --- a/std/math/fp2.asm +++ b/std/math/fp2.asm @@ -163,7 +163,7 @@ mod test { use std::check::assert; use std::array::map; - let add = || { + let test_add = || { let test_add = |a, b, c| assert(eq_ext(add_ext(a, b), c), || "Wrong addition result"); // Test adding 0 @@ -176,11 +176,11 @@ mod test { test_add(Fp2::Fp2(-1, -1), Fp2::Fp2(3, 4), Fp2::Fp2(2, 3)) }; - let sub = || { + let test_sub = || { let test_sub = |a, b, c| assert(eq_ext(sub_ext(a, b), c), || "Wrong subtraction result"); // Test subtracting 0 - let _ = test_sub(from_base(0), from_base(0), from_base(0)); + let _ = test_sub(from_base(0), from_base(0), from_base(9)); let _ = test_sub(Fp2::Fp2(123, 1234), from_base(0), Fp2::Fp2(123, 1234)); // Subtract arbitrary elements @@ -188,7 +188,7 @@ mod test { test_sub(Fp2::Fp2(-1, -1), Fp2::Fp2(0x78000000, 1), Fp2::Fp2(-0x78000000 - 1, -2)) }; - let mul = || { + let test_mul = || { let test_mul = |a, b, c| assert(eq_ext(mul_ext(a, b), c), || "Wrong multiplication result"); // Test multiplication by 1 @@ -207,7 +207,7 @@ mod test { test_mul(Fp2::Fp2(-1, -2), Fp2::Fp2(-3, 4), Fp2::Fp2(3 - 11 * 8, 6 - 4)) }; - let square = || { + let test_square = || { // Tests consistency with mul_ext let test_square = |a| assert(eq_ext(mul_ext(a, a), square_ext(a)), || "Wrong squaring result"); @@ -219,7 +219,7 @@ mod test { test_square(Fp2::Fp2(-1, -2)); }; - let inverse = || { + let test_inverse = || { let test_elements = [ from_base(1), Fp2::Fp2(123, 1234), diff --git a/std/math/fp4.asm b/std/math/fp4.asm index 79a250f256..b23b73479c 100644 --- a/std/math/fp4.asm +++ b/std/math/fp4.asm @@ -146,7 +146,7 @@ mod test { use std::check::assert; use std::array::map; - let add = || { + let test_add = || { let test_add = |a, b, c| assert(eq_ext(add_ext(a, b), c), || "Wrong addition result"); // Test adding 0 @@ -165,7 +165,7 @@ mod test { test_add(Fp4::Fp4(-1, 0, 0, 0), Fp4::Fp4(1, 0, 0, 0), from_base(0)); }; - let sub = || { + let test_sub = || { let test_sub = |a, b, c| assert(eq_ext(sub_ext(a, b), c), || "Wrong subtraction result"); // Test subtracting 0 @@ -177,7 +177,7 @@ mod test { test_sub(Fp4::Fp4(-1, -1, 0, 0), Fp4::Fp4(0x78000000, 1, 0, 0), Fp4::Fp4(-0x78000000 - 1, -2, 0, 0)) }; - let mul = || { + let test_mul = || { let test_mul = |a, b, c| assert(eq_ext(mul_ext(a, b), c), || "Wrong multiplication result"); // Test multiplication by 1 @@ -197,7 +197,7 @@ mod test { test_mul(Fp4::Fp4(-1, -2, -3, -4), Fp4::Fp4(-3, 4, 4, 5), Fp4::Fp4(-415, -339, -223, -13)); }; - let inverse = || { + let test_inverse = || { let test_elements = [ from_base(1), Fp4::Fp4(123, 1234, 1, 2), From 14039f258c7a7229744bfdefa269de13ed90ee82 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 31 Oct 2024 13:39:29 +0000 Subject: [PATCH 03/11] Move to pipeline. --- cli/src/main.rs | 2 +- cli/src/test_runner.rs | 90 ------------------------------------------ pipeline/src/lib.rs | 1 + 3 files changed, 2 insertions(+), 91 deletions(-) delete mode 100644 cli/src/test_runner.rs diff --git a/cli/src/main.rs b/cli/src/main.rs index 15f32d80ea..55fba753a2 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -1,6 +1,5 @@ //! The powdr CLI tool -mod test_runner; mod util; use clap::{CommandFactory, Parser, Subcommand}; @@ -13,6 +12,7 @@ use powdr::number::{ BabyBearField, BigUint, Bn254Field, FieldElement, GoldilocksField, KoalaBearField, Mersenne31Field, }; +use powdr::pipeline::{pipeline, test_runner}; use powdr::Pipeline; use std::io; use std::path::PathBuf; diff --git a/cli/src/test_runner.rs b/cli/src/test_runner.rs deleted file mode 100644 index b366a90870..0000000000 --- a/cli/src/test_runner.rs +++ /dev/null @@ -1,90 +0,0 @@ -use std::{ - collections::BTreeSet, - fs, - path::{Path, PathBuf}, - str::FromStr, -}; - -use itertools::Itertools; - -use powdr::ast::{ - analyzed::FunctionValueDefinition, - parsed::{ - asm::SymbolPath, - types::{FunctionType, Type}, - }, -}; -use powdr::pil_analyzer::evaluator::{self, SymbolLookup}; -use powdr::{FieldElement, Pipeline}; - -/// Executes all functions in the given file that start with `test_` and are -/// inside a module called `test`. -/// -/// @param include_std_tests: Whether to run the tests inside the standard library. -pub fn run(input: &str, include_std_tests: bool) -> Result<(), Vec> { - let mut pipeline = Pipeline::::default().from_file(PathBuf::from(&input)); - - let analyzed = pipeline.compute_analyzed_pil()?; - - let mut symbols = evaluator::Definitions { - definitions: &analyzed.definitions, - solved_impls: &analyzed.solved_impls, - }; - - let mut errors = vec![]; - let tests: BTreeSet<&String> = analyzed - .definitions - .iter() - .filter(|(n, _)| n.starts_with("test::test_") || n.contains("::test::test_")) - .filter(|(n, _)| include_std_tests || !n.starts_with("std::")) - .filter(|(n, _)| SymbolPath::from_str(n).unwrap().name().starts_with("test_")) - .sorted_by_key(|(n, _)| *n) - .filter_map(|(n, (_, val))| { - let Some(FunctionValueDefinition::Expression(f)) = val else { - return None; - }; - // Require a plain `->()` type. - (f.type_scheme.as_ref().unwrap().ty - == (FunctionType { - params: vec![], - value: Box::new(Type::empty_tuple()), - }) - .into()) - .then_some(n) - }) - .collect(); - println!("Running {} tests...", tests.len()); - println!("{}", "-".repeat(85)); - for name in &tests { - let name_len = name.len(); - let padding = if name_len >= 75 { - " ".to_string() - } else { - " ".repeat(76 - name_len) - }; - print!("{name}..."); - let function = symbols.lookup(name, &None).unwrap(); - match evaluator::evaluate_function_call::(function, vec![], &mut symbols) { - Err(e) => { - let msg = e.to_string(); - println!("{padding}failed\n {msg}"); - errors.push((name, msg)); - } - Ok(_) => println!("{padding}ok"), - } - } - - println!("{}", "-".repeat(85)); - if errors.is_empty() { - println!("All {} tests passed!", tests.len()); - Ok(()) - } else { - println!( - "Failed tests: {} / {}\n{}", - errors.len(), - tests.len(), - errors.iter().map(|(n, e)| format!(" {n}: {e}")).join("\n") - ); - Err(vec![format!("{} test(s) failed.", errors.len())]) - } -} diff --git a/pipeline/src/lib.rs b/pipeline/src/lib.rs index 2e0dd27707..30ecbcbb84 100644 --- a/pipeline/src/lib.rs +++ b/pipeline/src/lib.rs @@ -3,6 +3,7 @@ #![deny(clippy::print_stdout)] pub mod pipeline; +pub mod test_runner; pub mod test_util; pub mod util; pub mod verify; From 885c9ace556ae707645220fc4177cc1da29e5e5c Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 31 Oct 2024 13:49:14 +0000 Subject: [PATCH 04/11] Move test runner. --- cli/src/main.rs | 2 +- pipeline/src/test_runner.rs | 87 +++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 pipeline/src/test_runner.rs diff --git a/cli/src/main.rs b/cli/src/main.rs index 55fba753a2..d2e4a04066 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -12,7 +12,7 @@ use powdr::number::{ BabyBearField, BigUint, Bn254Field, FieldElement, GoldilocksField, KoalaBearField, Mersenne31Field, }; -use powdr::pipeline::{pipeline, test_runner}; +use powdr::pipeline::test_runner; use powdr::Pipeline; use std::io; use std::path::PathBuf; diff --git a/pipeline/src/test_runner.rs b/pipeline/src/test_runner.rs new file mode 100644 index 0000000000..8f3654f5de --- /dev/null +++ b/pipeline/src/test_runner.rs @@ -0,0 +1,87 @@ +use std::{collections::BTreeSet, path::PathBuf, str::FromStr}; + +use itertools::Itertools; + +use powdr_ast::{ + analyzed::FunctionValueDefinition, + parsed::{ + asm::SymbolPath, + types::{FunctionType, Type}, + }, +}; +use powdr_number::FieldElement; +use powdr_pil_analyzer::evaluator::{self, SymbolLookup}; + +use crate::Pipeline; + +/// Executes all functions in the given file that start with `test_` and are +/// inside a module called `test`. +/// +/// @param include_std_tests: Whether to run the tests inside the standard library. +pub fn run(input: &str, include_std_tests: bool) -> Result<(), Vec> { + let mut pipeline = Pipeline::::default().from_file(PathBuf::from(&input)); + + let analyzed = pipeline.compute_analyzed_pil()?; + + let mut symbols = evaluator::Definitions { + definitions: &analyzed.definitions, + solved_impls: &analyzed.solved_impls, + }; + + let mut errors = vec![]; + let tests: BTreeSet<&String> = analyzed + .definitions + .iter() + .filter(|(n, _)| n.starts_with("test::test_") || n.contains("::test::test_")) + .filter(|(n, _)| include_std_tests || !n.starts_with("std::")) + .filter(|(n, _)| SymbolPath::from_str(n).unwrap().name().starts_with("test_")) + .sorted_by_key(|(n, _)| *n) + .filter_map(|(n, (_, val))| { + let Some(FunctionValueDefinition::Expression(f)) = val else { + return None; + }; + // Require a plain `->()` type. + (f.type_scheme.as_ref().unwrap().ty + == (FunctionType { + params: vec![], + value: Box::new(Type::empty_tuple()), + }) + .into()) + .then_some(n) + }) + .collect(); + println!("Running {} tests...", tests.len()); + println!("{}", "-".repeat(85)); + for name in &tests { + let name_len = name.len(); + let padding = if name_len >= 75 { + " ".to_string() + } else { + " ".repeat(76 - name_len) + }; + print!("{name}..."); + let function = symbols.lookup(name, &None).unwrap(); + match evaluator::evaluate_function_call::(function, vec![], &mut symbols) { + Err(e) => { + let msg = e.to_string(); + println!("{padding}failed\n {msg}"); + errors.push((name, msg)); + } + Ok(_) => println!("{padding}ok"), + } + } + + println!("{}", "-".repeat(85)); + if errors.is_empty() { + println!("All {} tests passed!", tests.len()); + Ok(()) + } else { + println!( + "Failed tests: {} / {}\n{}", + errors.len(), + tests.len(), + errors.iter().map(|(n, e)| format!(" {n}: {e}")).join("\n") + ); + Err(vec![format!("{} test(s) failed.", errors.len())]) + } +} From 5157d33782315065a9e51c84274b24483ad0279d Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 31 Oct 2024 14:07:41 +0000 Subject: [PATCH 05/11] Group all std tests. --- cli/src/main.rs | 2 +- pipeline/src/test_runner.rs | 14 +++++++-- pipeline/tests/powdr_std.rs | 60 +++++-------------------------------- 3 files changed, 21 insertions(+), 55 deletions(-) diff --git a/cli/src/main.rs b/cli/src/main.rs index d2e4a04066..812f41a6bd 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -718,7 +718,7 @@ fn run( } fn run_test(file: &str, include_std_tests: bool) -> Result<(), Vec> { - test_runner::run::(file, include_std_tests) + test_runner::run_from_file::(file, include_std_tests) } #[allow(clippy::too_many_arguments)] diff --git a/pipeline/src/test_runner.rs b/pipeline/src/test_runner.rs index 8f3654f5de..dee9736911 100644 --- a/pipeline/src/test_runner.rs +++ b/pipeline/src/test_runner.rs @@ -3,7 +3,7 @@ use std::{collections::BTreeSet, path::PathBuf, str::FromStr}; use itertools::Itertools; use powdr_ast::{ - analyzed::FunctionValueDefinition, + analyzed::{Analyzed, FunctionValueDefinition}, parsed::{ asm::SymbolPath, types::{FunctionType, Type}, @@ -18,11 +18,21 @@ use crate::Pipeline; /// inside a module called `test`. /// /// @param include_std_tests: Whether to run the tests inside the standard library. -pub fn run(input: &str, include_std_tests: bool) -> Result<(), Vec> { +pub fn run_from_file( + input: &str, + include_std_tests: bool, +) -> Result<(), Vec> { let mut pipeline = Pipeline::::default().from_file(PathBuf::from(&input)); let analyzed = pipeline.compute_analyzed_pil()?; + run_tests::(&analyzed, include_std_tests) +} +#[allow(clippy::print_stdout)] +pub fn run_tests( + analyzed: &Analyzed, + include_std_tests: bool, +) -> Result<(), Vec> { let mut symbols = evaluator::Definitions { definitions: &analyzed.definitions, solved_impls: &analyzed.solved_impls, diff --git a/pipeline/tests/powdr_std.rs b/pipeline/tests/powdr_std.rs index 2b655719fb..9b87dd8210 100644 --- a/pipeline/tests/powdr_std.rs +++ b/pipeline/tests/powdr_std.rs @@ -4,6 +4,7 @@ use powdr_number::{BabyBearField, BigInt, Bn254Field, GoldilocksField}; use powdr_pil_analyzer::evaluator::Value; use powdr_pipeline::{ + test_runner::run_tests, test_util::{ evaluate_function, evaluate_integer_function, execute_test_file, gen_estark_proof, gen_halo2_proof, make_simple_prepared_pipeline, regular_test, @@ -396,58 +397,10 @@ fn ff_inv_big() { } #[test] -fn fp2() { - let analyzed = std_analyzed::(); - evaluate_function(&analyzed, "std::math::fp2::test::add", vec![]); - evaluate_function(&analyzed, "std::math::fp2::test::sub", vec![]); - evaluate_function(&analyzed, "std::math::fp2::test::mul", vec![]); - evaluate_function(&analyzed, "std::math::fp2::test::square", vec![]); - evaluate_function(&analyzed, "std::math::fp2::test::inverse", vec![]); - - let analyzed = std_analyzed::(); - evaluate_function(&analyzed, "std::math::fp2::test::add", vec![]); - evaluate_function(&analyzed, "std::math::fp2::test::sub", vec![]); - evaluate_function(&analyzed, "std::math::fp2::test::mul", vec![]); - evaluate_function(&analyzed, "std::math::fp2::test::square", vec![]); - evaluate_function(&analyzed, "std::math::fp2::test::inverse", vec![]); - - let analyzed = std_analyzed::(); - evaluate_function(&analyzed, "std::math::fp2::test::add", vec![]); - evaluate_function(&analyzed, "std::math::fp2::test::sub", vec![]); - evaluate_function(&analyzed, "std::math::fp2::test::mul", vec![]); - evaluate_function(&analyzed, "std::math::fp2::test::square", vec![]); - evaluate_function(&analyzed, "std::math::fp2::test::inverse", vec![]); -} - -#[test] -fn fp4() { - let analyzed = std_analyzed::(); - evaluate_function(&analyzed, "std::math::fp4::test::add", vec![]); - evaluate_function(&analyzed, "std::math::fp4::test::sub", vec![]); - evaluate_function(&analyzed, "std::math::fp4::test::mul", vec![]); - evaluate_function(&analyzed, "std::math::fp4::test::inverse", vec![]); - - let analyzed = std_analyzed::(); - evaluate_function(&analyzed, "std::math::fp4::test::add", vec![]); - evaluate_function(&analyzed, "std::math::fp4::test::sub", vec![]); - evaluate_function(&analyzed, "std::math::fp4::test::mul", vec![]); - evaluate_function(&analyzed, "std::math::fp4::test::inverse", vec![]); - - let analyzed = std_analyzed::(); - evaluate_function(&analyzed, "std::math::fp4::test::add", vec![]); - evaluate_function(&analyzed, "std::math::fp4::test::sub", vec![]); - evaluate_function(&analyzed, "std::math::fp4::test::mul", vec![]); - evaluate_function(&analyzed, "std::math::fp4::test::inverse", vec![]); -} - -#[test] -fn fingerprint() { - let analyzed = std_analyzed::(); - evaluate_function( - &analyzed, - "std::protocols::fingerprint::test::test_fingerprint", - vec![], - ); +fn std_tests() { + run_tests(&std_analyzed::(), true).unwrap(); + run_tests(&std_analyzed::(), true).unwrap(); + run_tests(&std_analyzed::(), true).unwrap(); } #[test] @@ -494,6 +447,9 @@ fn sort() { assert_eq!(input_sorted, result); } } + +// TOOD turn this into proper std test + #[test] fn btree() { let f = "std/btree_test.asm"; From 19cf594ef3d16fde8ade8be00f5174c2e9bb4601 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 31 Oct 2024 14:09:18 +0000 Subject: [PATCH 06/11] Remove intentional test failure. --- std/math/fp2.asm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/std/math/fp2.asm b/std/math/fp2.asm index 12202b0940..1d49bfd903 100644 --- a/std/math/fp2.asm +++ b/std/math/fp2.asm @@ -180,7 +180,7 @@ mod test { let test_sub = |a, b, c| assert(eq_ext(sub_ext(a, b), c), || "Wrong subtraction result"); // Test subtracting 0 - let _ = test_sub(from_base(0), from_base(0), from_base(9)); + let _ = test_sub(from_base(0), from_base(0), from_base(0)); let _ = test_sub(Fp2::Fp2(123, 1234), from_base(0), Fp2::Fp2(123, 1234)); // Subtract arbitrary elements From 7a4d8cf838d8993c13a51a7fde2c561ed3567f05 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 31 Oct 2024 14:15:34 +0000 Subject: [PATCH 07/11] Print field. --- number/src/traits.rs | 19 ++++++++++++++++++- pipeline/src/test_runner.rs | 8 ++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/number/src/traits.rs b/number/src/traits.rs index 77192f6896..d15672480b 100644 --- a/number/src/traits.rs +++ b/number/src/traits.rs @@ -1,4 +1,9 @@ -use std::{fmt, hash::Hash, ops::*, str::FromStr}; +use std::{ + fmt::{self, Display}, + hash::Hash, + ops::*, + str::FromStr, +}; use num_traits::{ConstOne, ConstZero, One, Zero}; use schemars::JsonSchema; @@ -90,6 +95,18 @@ impl KnownField { } } +impl Display for KnownField { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + KnownField::BabyBearField => write!(f, "BabyBear"), + KnownField::KoalaBearField => write!(f, "KoalaBear"), + KnownField::Mersenne31Field => write!(f, "Mersenne31"), + KnownField::GoldilocksField => write!(f, "Goldilocks"), + KnownField::Bn254Field => write!(f, "Bn254"), + } + } +} + /// A field element pub trait FieldElement: 'static diff --git a/pipeline/src/test_runner.rs b/pipeline/src/test_runner.rs index dee9736911..0584d72f7b 100644 --- a/pipeline/src/test_runner.rs +++ b/pipeline/src/test_runner.rs @@ -25,7 +25,7 @@ pub fn run_from_file( let mut pipeline = Pipeline::::default().from_file(PathBuf::from(&input)); let analyzed = pipeline.compute_analyzed_pil()?; - run_tests::(&analyzed, include_std_tests) + run_tests::(analyzed, include_std_tests) } #[allow(clippy::print_stdout)] @@ -60,7 +60,11 @@ pub fn run_tests( .then_some(n) }) .collect(); - println!("Running {} tests...", tests.len()); + let field_name = F::known_field().map_or_else( + || format!("with modulus {}", F::modulus()), + |f| f.to_string(), + ); + println!("Running {} tests using field {field_name}...", tests.len()); println!("{}", "-".repeat(85)); for name in &tests { let name_len = name.len(); From 6eeb7ede8c44e21429df70b0188102309c9a911d Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 31 Oct 2024 14:26:49 +0000 Subject: [PATCH 08/11] Also execute test functions in sub-modules. --- cli/src/main.rs | 5 ++- pipeline/src/test_runner.rs | 18 ++++++--- pipeline/tests/powdr_std.rs | 24 ++++++------ std/mod.asm | 1 + .../std/btree_test.asm => std/test/btree.asm | 39 ++++++++++--------- std/test/mod.asm | 1 + 6 files changed, 51 insertions(+), 37 deletions(-) rename test_data/std/btree_test.asm => std/test/btree.asm (74%) create mode 100644 std/test/mod.asm diff --git a/cli/src/main.rs b/cli/src/main.rs index 812f41a6bd..c4f05482e2 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -366,7 +366,7 @@ enum Commands { }, /// Executes all functions starting with `test_` in every module called - /// `test` starting from the given module. + /// `test` (or sub-module thereof) starting from the given module. Test { /// Input file. file: String, @@ -718,7 +718,8 @@ fn run( } fn run_test(file: &str, include_std_tests: bool) -> Result<(), Vec> { - test_runner::run_from_file::(file, include_std_tests) + test_runner::run_from_file::(file, include_std_tests)?; + Ok(()) } #[allow(clippy::too_many_arguments)] diff --git a/pipeline/src/test_runner.rs b/pipeline/src/test_runner.rs index 0584d72f7b..9ab9ceebe1 100644 --- a/pipeline/src/test_runner.rs +++ b/pipeline/src/test_runner.rs @@ -15,13 +15,13 @@ use powdr_pil_analyzer::evaluator::{self, SymbolLookup}; use crate::Pipeline; /// Executes all functions in the given file that start with `test_` and are -/// inside a module called `test`. +/// inside a module called `test` (or a sub-module thereof). /// /// @param include_std_tests: Whether to run the tests inside the standard library. pub fn run_from_file( input: &str, include_std_tests: bool, -) -> Result<(), Vec> { +) -> Result> { let mut pipeline = Pipeline::::default().from_file(PathBuf::from(&input)); let analyzed = pipeline.compute_analyzed_pil()?; @@ -29,10 +29,16 @@ pub fn run_from_file( } #[allow(clippy::print_stdout)] +/// Executes all functions in the given file that start with `test_` and are +/// inside a module called `test` (or a sub-module thereof). +/// +/// @param include_std_tests: Whether to run the tests inside the standard library. +/// +/// Returns the number of tests executed. pub fn run_tests( analyzed: &Analyzed, include_std_tests: bool, -) -> Result<(), Vec> { +) -> Result> { let mut symbols = evaluator::Definitions { definitions: &analyzed.definitions, solved_impls: &analyzed.solved_impls, @@ -42,7 +48,9 @@ pub fn run_tests( let tests: BTreeSet<&String> = analyzed .definitions .iter() - .filter(|(n, _)| n.starts_with("test::test_") || n.contains("::test::test_")) + .filter(|(n, _)| { + (n.starts_with("test::") || n.contains("::test::")) && n.contains("::test_") + }) .filter(|(n, _)| include_std_tests || !n.starts_with("std::")) .filter(|(n, _)| SymbolPath::from_str(n).unwrap().name().starts_with("test_")) .sorted_by_key(|(n, _)| *n) @@ -88,7 +96,7 @@ pub fn run_tests( println!("{}", "-".repeat(85)); if errors.is_empty() { println!("All {} tests passed!", tests.len()); - Ok(()) + Ok(tests.len()) } else { println!( "Failed tests: {} / {}\n{}", diff --git a/pipeline/tests/powdr_std.rs b/pipeline/tests/powdr_std.rs index 9b87dd8210..76a3a55635 100644 --- a/pipeline/tests/powdr_std.rs +++ b/pipeline/tests/powdr_std.rs @@ -398,9 +398,19 @@ fn ff_inv_big() { #[test] fn std_tests() { - run_tests(&std_analyzed::(), true).unwrap(); - run_tests(&std_analyzed::(), true).unwrap(); - run_tests(&std_analyzed::(), true).unwrap(); + let test_count = 9; + assert_eq!( + test_count, + run_tests(&std_analyzed::(), true).unwrap() + ); + assert_eq!( + test_count, + run_tests(&std_analyzed::(), true).unwrap() + ); + assert_eq!( + test_count, + run_tests(&std_analyzed::(), true).unwrap(), + ); } #[test] @@ -448,14 +458,6 @@ fn sort() { } } -// TOOD turn this into proper std test - -#[test] -fn btree() { - let f = "std/btree_test.asm"; - execute_test_file(f, Default::default(), vec![]).unwrap(); -} - mod reparse { use powdr_pipeline::test_util::run_reparse_test_with_blacklist; diff --git a/std/mod.asm b/std/mod.asm index cb3e670db7..9c9f1b35e6 100644 --- a/std/mod.asm +++ b/std/mod.asm @@ -10,5 +10,6 @@ mod math; mod prelude; mod protocols; mod prover; +mod test; mod utils; mod well_known; diff --git a/test_data/std/btree_test.asm b/std/test/btree.asm similarity index 74% rename from test_data/std/btree_test.asm rename to std/test/btree.asm index 3de0ee6441..552b218c76 100644 --- a/test_data/std/btree_test.asm +++ b/std/test/btree.asm @@ -14,21 +14,6 @@ let cmp: int, int -> CmpResult = |a, b| CmpResult::Greater } }; -let b1 = new::(); -let b2 = insert(b1, (1, "a"), cmp); -let b3 = insert(b2, (8, "b"), cmp); -let b4 = insert(b3, (4, "c"), cmp); -let b5 = insert(b4, (2, "d"), cmp); -let b6 = insert(b5, (9, "e"), cmp); -let b7 = insert(b6, (3, "f"), cmp); -let b8 = insert(b7, (7, "g"), cmp); -let b9 = insert(b8, (5, "h"), cmp); -let b10 = insert(b9, (6, "i"), cmp); -let b11 = insert(b10, (0, "j"), cmp); -let b12 = insert(b11, (10, "k"), cmp); -// this one replaces -let b13 = insert(b12, (4, "l"), cmp); -let b14 = insert(b13, (6, "m"), cmp); let one: int = 1; let false: bool = one == 0; @@ -82,7 +67,23 @@ let do_print: BTree -> () = |b_tree| { () }; -machine Main { +let test_btree = || { + let b1 = new::(); + let b2 = insert(b1, (1, "a"), cmp); + let b3 = insert(b2, (8, "b"), cmp); + let b4 = insert(b3, (4, "c"), cmp); + let b5 = insert(b4, (2, "d"), cmp); + let b6 = insert(b5, (9, "e"), cmp); + let b7 = insert(b6, (3, "f"), cmp); + let b8 = insert(b7, (7, "g"), cmp); + let b9 = insert(b8, (5, "h"), cmp); + let b10 = insert(b9, (6, "i"), cmp); + let b11 = insert(b10, (0, "j"), cmp); + let b12 = insert(b11, (10, "k"), cmp); + // this one replaces + let b13 = insert(b12, (4, "l"), cmp); + let b14 = insert(b13, (6, "m"), cmp); + expect(b14, 6, "m"); expect(b13, 6, "i"); expect(b14, 4, "l"); @@ -91,8 +92,8 @@ machine Main { expect(b14, 7, "g"); expect(b14, 0, "j"); expect(b14, 9, "e"); - do_print(b14); + //do_print(b14); let skewed = std::utils::fold(200, |i| i, new::(), |tree, i| insert(tree, (i, ""), cmp)); - do_print(skewed); -} + //do_print(skewed); +}; diff --git a/std/test/mod.asm b/std/test/mod.asm new file mode 100644 index 0000000000..c9ca01e0b8 --- /dev/null +++ b/std/test/mod.asm @@ -0,0 +1 @@ +mod btree; From c5174be46078526f4461d46c6cede95838c88d8e Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 31 Oct 2024 14:29:13 +0000 Subject: [PATCH 09/11] Remove old test utility. --- pipeline/src/test_util.rs | 13 ------------- pipeline/tests/powdr_std.rs | 7 +++---- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/pipeline/src/test_util.rs b/pipeline/src/test_util.rs index 0236b00dc5..10ef64789b 100644 --- a/pipeline/src/test_util.rs +++ b/pipeline/src/test_util.rs @@ -21,19 +21,6 @@ pub fn resolve_test_file(file_name: &str) -> PathBuf { PathBuf::from(format!("../test_data/{file_name}")) } -pub fn execute_test_file( - file_name: &str, - inputs: Vec, - external_witness_values: Vec<(String, Vec)>, -) -> Result<(), Vec> { - Pipeline::default() - .from_file(resolve_test_file(file_name)) - .with_prover_inputs(inputs) - .add_external_witness_values(external_witness_values) - .compute_witness() - .map(|_| ()) -} - /// Makes a new pipeline for the given file. All steps until witness generation are /// already computed, so that the test can branch off from there, without having to re-compute /// these steps. diff --git a/pipeline/tests/powdr_std.rs b/pipeline/tests/powdr_std.rs index 76a3a55635..c3216acd02 100644 --- a/pipeline/tests/powdr_std.rs +++ b/pipeline/tests/powdr_std.rs @@ -6,10 +6,9 @@ use powdr_pil_analyzer::evaluator::Value; use powdr_pipeline::{ test_runner::run_tests, test_util::{ - evaluate_function, evaluate_integer_function, execute_test_file, gen_estark_proof, - gen_halo2_proof, make_simple_prepared_pipeline, regular_test, - regular_test_without_small_field, std_analyzed, test_halo2, test_pilcom, test_plonky3, - BackendVariant, + evaluate_function, evaluate_integer_function, gen_estark_proof, gen_halo2_proof, + make_simple_prepared_pipeline, regular_test, regular_test_without_small_field, + std_analyzed, test_halo2, test_pilcom, test_plonky3, BackendVariant, }, Pipeline, }; From 28b5f462f2a6c9ef3e842d7e8e648dd52302b356 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 31 Oct 2024 15:09:36 +0000 Subject: [PATCH 10/11] Remove the "also std" option. --- cli/src/main.rs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/cli/src/main.rs b/cli/src/main.rs index c4f05482e2..b426c173d9 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -376,11 +376,6 @@ enum Commands { #[arg(default_value_t = FieldArgument::Gl)] #[arg(value_parser = clap_enum_variants!(FieldArgument))] field: FieldArgument, - - /// Also run the tests inside the standard library. - #[arg(long)] - #[arg(default_value_t = false)] - include_std_tests: bool, }, } @@ -488,12 +483,8 @@ fn run_command(command: Commands) { csv_mode )) } - Commands::Test { - file, - field, - include_std_tests, - } => { - call_with_field!(run_test::(&file, include_std_tests)) + Commands::Test { file, field } => { + call_with_field!(run_test::(&file)) } Commands::Prove { file, @@ -717,7 +708,8 @@ fn run( Ok(()) } -fn run_test(file: &str, include_std_tests: bool) -> Result<(), Vec> { +fn run_test(file: &str) -> Result<(), Vec> { + let include_std_tests = false; test_runner::run_from_file::(file, include_std_tests)?; Ok(()) } From f42e9ba35e26586f1ed059ef907b97bfc4fd8c91 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 31 Oct 2024 15:41:05 +0000 Subject: [PATCH 11/11] simplify test count check. --- pipeline/tests/powdr_std.rs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/pipeline/tests/powdr_std.rs b/pipeline/tests/powdr_std.rs index c3216acd02..fc67dc75d0 100644 --- a/pipeline/tests/powdr_std.rs +++ b/pipeline/tests/powdr_std.rs @@ -397,19 +397,12 @@ fn ff_inv_big() { #[test] fn std_tests() { - let test_count = 9; - assert_eq!( - test_count, - run_tests(&std_analyzed::(), true).unwrap() - ); - assert_eq!( - test_count, - run_tests(&std_analyzed::(), true).unwrap() - ); - assert_eq!( - test_count, - run_tests(&std_analyzed::(), true).unwrap(), - ); + let count1 = run_tests(&std_analyzed::(), true).unwrap(); + let count2 = run_tests(&std_analyzed::(), true).unwrap(); + let count3 = run_tests(&std_analyzed::(), true).unwrap(); + assert_eq!(count1, count2); + assert_eq!(count2, count3); + assert!(count1 >= 9); } #[test]