From 6539f0a72ef1daf361a1262de714ce93a0feebda Mon Sep 17 00:00:00 2001 From: Tpt Date: Tue, 3 Jan 2023 20:54:32 +0100 Subject: [PATCH] SPARQL test: displays query results diffs Makes debugging easier --- testsuite/src/report.rs | 41 ++++----- testsuite/src/sparql_evaluator.rs | 141 +++++++++++++++++++++++------- 2 files changed, 131 insertions(+), 51 deletions(-) diff --git a/testsuite/src/report.rs b/testsuite/src/report.rs index 524e3b6d..f1fc35e8 100644 --- a/testsuite/src/report.rs +++ b/testsuite/src/report.rs @@ -13,42 +13,43 @@ pub struct TestResult { } pub fn dataset_diff(expected: &Dataset, actual: &Dataset) -> String { - let (_, changeset) = diff( + format_diff( &normalize_dataset_text(expected), &normalize_dataset_text(actual), - "\n", - ); + "quads", + ) +} + +fn normalize_dataset_text(store: &Dataset) -> String { + let mut quads: Vec<_> = store.iter().map(|q| q.to_string()).collect(); + quads.sort(); + quads.join("\n") +} + +pub(super) fn format_diff(expected: &str, actual: &str, kind: &str) -> String { + let (_, changeset) = diff(expected, actual, "\n"); let mut ret = String::new(); - ret.push_str("Note: missing quads in yellow and extra quads in blue\n"); + writeln!( + &mut ret, + "Note: missing {kind} in yellow and extra {kind} in blue" + ) + .unwrap(); for seq in changeset { match seq { Difference::Same(x) => { - ret.push_str(&x); - ret.push('\n'); + writeln!(&mut ret, "{x}").unwrap(); } Difference::Add(x) => { - ret.push_str("\x1B[94m"); - ret.push_str(&x); - ret.push_str("\x1B[0m"); - ret.push('\n'); + writeln!(&mut ret, "\x1B[94m{x}\x1B[0m").unwrap(); } Difference::Rem(x) => { - ret.push_str("\x1B[93m"); - ret.push_str(&x); - ret.push_str("\x1B[0m"); - ret.push('\n'); + writeln!(&mut ret, "\x1B[93m{x}\x1B[0m").unwrap(); } } } ret } -fn normalize_dataset_text(store: &Dataset) -> String { - let mut quads: Vec<_> = store.iter().map(|q| q.to_string()).collect(); - quads.sort(); - quads.join("\n") -} - #[allow(unused_must_use)] pub fn build_report(results: impl IntoIterator) -> String { let mut buffer = String::new(); diff --git a/testsuite/src/sparql_evaluator.rs b/testsuite/src/sparql_evaluator.rs index e38b5ebc..8713be24 100644 --- a/testsuite/src/sparql_evaluator.rs +++ b/testsuite/src/sparql_evaluator.rs @@ -1,7 +1,7 @@ use crate::evaluator::TestEvaluator; use crate::files::*; use crate::manifest::*; -use crate::report::dataset_diff; +use crate::report::{dataset_diff, format_diff}; use crate::vocab::*; use anyhow::{anyhow, bail, Result}; use oxigraph::model::vocab::*; @@ -9,10 +9,10 @@ use oxigraph::model::*; use oxigraph::sparql::*; use oxigraph::store::Store; use std::collections::HashMap; -use std::io::Cursor; +use std::fmt::Write; +use std::io::{self, Cursor}; use std::str::FromStr; use std::sync::Arc; -use std::{fmt, io}; pub fn register_sparql_tests(evaluator: &mut TestEvaluator) { evaluator.register( @@ -206,7 +206,8 @@ fn evaluate_evaluation_test(test: &Test) -> Result<()> { if !are_query_results_isomorphic(&expected_results, &actual_results) { bail!( - "Failure on {test}.\nExpected file:\n{expected_results}\nOutput file:\n{actual_results}\nParsed query:\n{}\nData:\n{store}\n", + "Failure on {test}.\n{}\nParsed query:\n{}\nData:\n{store}\n", + results_diff(expected_results, actual_results), Query::parse(&read_file_to_string(query_file)?, Some(query_file)).unwrap() ); } @@ -496,33 +497,6 @@ enum StaticQueryResults { Boolean(bool), } -impl fmt::Display for StaticQueryResults { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - StaticQueryResults::Graph(g) => g.fmt(f), - StaticQueryResults::Solutions { - variables, - solutions, - .. - } => { - write!(f, "Variables:")?; - for v in variables { - write!(f, " {v}")?; - } - for solution in solutions { - write!(f, "\n{{")?; - for (k, v) in solution { - write!(f, "{k} = {v} ")?; - } - write!(f, "}}")?; - } - Ok(()) - } - StaticQueryResults::Boolean(b) => b.fmt(f), - } - } -} - impl StaticQueryResults { fn from_query_results(results: QueryResults, with_order: bool) -> Result { Self::from_graph(to_graph(results, with_order)?) @@ -621,3 +595,108 @@ impl StaticQueryResults { } } } + +fn results_diff(expected: StaticQueryResults, actual: StaticQueryResults) -> String { + match expected { + StaticQueryResults::Solutions { + variables: mut expected_variables, + solutions: expected_solutions, + ordered, + } => match actual { + StaticQueryResults::Solutions { + variables: mut actual_variables, + solutions: actual_solutions, + .. + } => { + let mut out = String::new(); + expected_variables.sort_unstable(); + actual_variables.sort_unstable(); + if expected_variables != actual_variables { + write!( + &mut out, + "Variables diff:\n{}", + format_diff( + &expected_variables + .iter() + .map(|v| v.to_string()) + .collect::>() + .join("\n"), + &actual_variables + .iter() + .map(|v| v.to_string()) + .collect::>() + .join("\n"), + "variables", + ) + ) + .unwrap(); + } + write!( + &mut out, + "Solutions diff:\n{}", + format_diff( + &solutions_to_string(expected_solutions, ordered), + &solutions_to_string(actual_solutions, ordered), + "solutions", + ) + ) + .unwrap(); + out + } + StaticQueryResults::Boolean(actual) => { + format!("Expecting solutions but found the boolean {actual}") + } + StaticQueryResults::Graph(actual) => { + format!("Expecting solutions but found the graph:\n{actual}") + } + }, + StaticQueryResults::Graph(expected) => match actual { + StaticQueryResults::Solutions { .. } => "Expecting a graph but found solutions".into(), + StaticQueryResults::Boolean(actual) => { + format!("Expecting a graph but found the boolean {actual}") + } + StaticQueryResults::Graph(actual) => { + let expected = expected + .into_iter() + .map(|t| t.in_graph(GraphNameRef::DefaultGraph)) + .collect(); + let actual = actual + .into_iter() + .map(|t| t.in_graph(GraphNameRef::DefaultGraph)) + .collect(); + dataset_diff(&expected, &actual) + } + }, + StaticQueryResults::Boolean(expected) => match actual { + StaticQueryResults::Solutions { .. } => { + "Expecting a boolean but found solutions".into() + } + StaticQueryResults::Boolean(actual) => { + format!("Expecting {expected} but found {actual}") + } + StaticQueryResults::Graph(actual) => { + format!("Expecting solutions but found the graph:\n{actual}") + } + }, + } +} + +fn solutions_to_string(solutions: Vec>, ordered: bool) -> String { + let mut lines = solutions + .into_iter() + .map(|mut s| { + let mut out = String::new(); + write!(&mut out, "{{").unwrap(); + s.sort_unstable_by(|(v1, _), (v2, _)| v1.cmp(v2)); + for (variable, value) in s { + write!(&mut out, "{variable} = {value} ").unwrap(); + } + write!(&mut out, "}}").unwrap(); + out + }) + .collect::>(); + if !ordered { + lines.sort_unstable(); + } + lines.join("\n") +}