From 6a1e60db5107cbbd1445570eb17da0a21c827c04 Mon Sep 17 00:00:00 2001 From: Simon Chopin Date: Fri, 14 Aug 2026 16:18:34 +0200 Subject: [PATCH] tests: use the json visitor to implement a simple path matcher The adhoc visitor in soft_error_stream_content is checking for some basic structure properties without being as strict as assert_soft_errors_in_minidump is. This patch simply hides the struct walking behind a helper, so that the intent is clearly shown in the test itself, and the nitty-gritty of walking through the JSON are hidden away in a helper. --- tests/common/mod.rs | 87 +++++++++++++++++++---- tests/linux_minidump_writer.rs | 6 +- tests/linux_minidump_writer_soft_error.rs | 76 ++++---------------- 3 files changed, 88 insertions(+), 81 deletions(-) diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 99d6af7f..a82d7602 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -5,6 +5,8 @@ use std::{ result, }; +use serde::Serialize; + #[allow(unused)] type Error = Box; #[allow(unused)] @@ -129,25 +131,84 @@ where serde_json::from_str(contents).expect("expected json") } +pub fn visit_json_objects(json: &serde_json::Value, path: &mut Vec, visit: &mut V) +where + V: FnMut(&[String], &serde_json::Value), +{ + if let Some(obj) = json.as_object() { + for (key, value) in obj.iter() { + // visiting the key is useful if you're matching against a complex enum member + // but don't really care about its contents + visit(path, &serde_json::Value::String(key.clone())); + path.push(key.clone()); + visit_json_objects(value, path, visit); + path.pop(); + } + } + if let Some(arr) = json.as_array() { + for value in arr.iter() { + visit_json_objects(value, path, visit); + } + return; + } + visit(path, json); +} + +#[allow(unused)] +#[derive(Debug)] +pub struct ErrorPattern { + value: serde_json::Value, + ancestor: Option, +} + #[allow(unused)] -pub fn assert_soft_errors_in_minidump<'a, 'b, T, I>( +impl ErrorPattern { + pub fn value(value: impl Serialize) -> Self { + Self { + value: serde_json::to_value(value).unwrap(), + ancestor: None, + } + } + pub fn with_ancestor(mut self, ancestor: impl ToString) -> Self { + self.ancestor = Some(ancestor.to_string()); + self + } + + fn matches(&self, value: &serde_json::Value, path: &[String]) -> bool { + &self.value == value + && match &self.ancestor { + None => true, + Some(ancestor) => path.contains(ancestor), + } + } +} + +#[allow(unused)] +pub(crate) fn assert_minidump_soft_errors_match_all<'a, T>( dump: &minidump::Minidump<'a, T>, - expected_errors: I, + patterns: &[ErrorPattern], ) where T: std::ops::Deref + 'a, - I: IntoIterator, { let actual_json = read_minidump_soft_errors_or_panic(dump); - let actual_errors = actual_json.as_array().unwrap(); - - // Ensure that every error we expect is in the actual list somewhere - for expected_error in expected_errors { - assert!( - actual_errors - .iter() - .any(|actual_error| actual_error == expected_error), - "soft error list missing expected error `{expected_error:#?}`\nError_list: {actual_errors:#?}" - ); + + let mut path = vec![]; + // We can't use a HashSet because remove() is too greedy in terms of + // lifetime. + let mut expected: Vec<_> = patterns.iter().collect(); + + visit_json_objects(&actual_json, &mut path, &mut |path, value| { + for i in 0..expected.len() { + let pattern = &expected[i]; + if pattern.matches(value, path) { + expected.remove(i); + return; + } + } + }); + + if !expected.is_empty() { + panic!("Soft errors patterns not found: {:?}", expected); } } diff --git a/tests/linux_minidump_writer.rs b/tests/linux_minidump_writer.rs index 74d870d2..03104c50 100644 --- a/tests/linux_minidump_writer.rs +++ b/tests/linux_minidump_writer.rs @@ -316,9 +316,7 @@ contextual_test! { contextual_test! { fn skip_if_requested(context: Context) { let expected_errors = vec![ - json!({ - "InitErrors": ["PrincipalMappingNotReferenced"] - }), + ErrorPattern::value("PrincipalMappingNotReferenced").with_ancestor("InitErrors") ]; let num_of_threads = 1; @@ -356,7 +354,7 @@ contextual_test! { // Ensure the MozSoftErrors stream contains the expected errors let dump = Minidump::read_path(tmpfile.path()).expect("failed to read minidump"); - assert_soft_errors_in_minidump(&dump, &expected_errors); + assert_minidump_soft_errors_match_all(&dump, &expected_errors); } } diff --git a/tests/linux_minidump_writer_soft_error.rs b/tests/linux_minidump_writer_soft_error.rs index 4f0bbc73..d4a999d7 100644 --- a/tests/linux_minidump_writer_soft_error.rs +++ b/tests/linux_minidump_writer_soft_error.rs @@ -3,7 +3,6 @@ use common::*; use minidump::Minidump; use minidump_writer::{FailSpotName, minidump_writer::MinidumpWriterConfig}; -use serde_json as json; mod common; @@ -32,27 +31,6 @@ fn soft_error_stream() { read_minidump_soft_errors_or_panic(&dump); } -fn visit_json_terminals(json: &json::Value, path: &mut Vec, visit: &mut V) -where - V: FnMut(&[String], &json::Value), -{ - if let Some(obj) = json.as_object() { - for (key, value) in obj.iter() { - path.push(key.clone()); - visit_json_terminals(value, path, visit); - path.pop(); - } - return; - } - if let Some(arr) = json.as_array() { - for value in arr.iter() { - visit_json_terminals(value, path, visit); - } - return; - } - visit(path, json); -} - #[test] fn soft_error_stream_content() { let mut child = start_child_and_wait_for_threads(1); @@ -81,50 +59,20 @@ fn soft_error_stream_content() { child.kill().expect("Failed to kill process"); child.wait().expect("Failed to wait on killed process"); - // Ensure the MozSoftErrors stream contains the expected errors let dump = Minidump::read_path(tmpfile.path()).expect("failed to read minidump"); - let actual_json = read_minidump_soft_errors_or_panic(&dump); - - let mut stop_process_error_found = false; - let mut missing_auxv_error_found = false; - let mut thread_name_error_found = false; - let mut suspend_thread_error_found = false; - let mut cpu_info_error_found = false; - - let mut path = Vec::new(); - - visit_json_terminals(&actual_json, &mut path, &mut |path, value| { - if value.as_i64() == Some(libc::EPERM.into()) - && path.contains(&"StopProcessFailed".to_string()) - { - stop_process_error_found = true; - } - if value.as_str() == Some("InvalidFormat") - && path.contains(&"FillMissingAuxvInfoErrors".to_string()) - { - missing_auxv_error_found = true; - } - if path.contains(&"ReadThreadNameFailed".to_string()) { - thread_name_error_found = true; - } - if value.as_i64() == Some(libc::EPERM.into()) - && path.contains(&"SuspendThreadsErrors".to_string()) - { - suspend_thread_error_found = true; - } - if value.as_i64() == Some(libc::EPERM.into()) - && path.contains(&"WriteCpuInformationFailed".to_string()) - { - cpu_info_error_found = true; - } - }); - - assert!(stop_process_error_found); - assert!(missing_auxv_error_found); - assert!(thread_name_error_found); - assert!(suspend_thread_error_found); - assert!(cpu_info_error_found); + // Ensure the MozSoftErrors stream contains the expected errors + assert_minidump_soft_errors_match_all( + &dump, + &[ + ErrorPattern::value(libc::EPERM).with_ancestor("StopProcessFailed"), + // AuxvError is not reachable so use the string form instead + ErrorPattern::value("InvalidFormat").with_ancestor("FillMissingAuxvInfoErrors"), + ErrorPattern::value("ReadThreadNameFailed"), + ErrorPattern::value(libc::EPERM).with_ancestor("SuspendThreadsErrors"), + ErrorPattern::value(libc::EPERM).with_ancestor("WriteCpuInformationFailed"), + ], + ); } #[test]