From b9285fcb781b6c20d12e42f37ca779715b6ad32c Mon Sep 17 00:00:00 2001 From: CraftSpider Date: Sat, 30 Mar 2024 17:00:04 -0700 Subject: [PATCH 1/6] Start test improvements --- src/errors.rs | 33 +++++++++++++++++++++++++++++++++ tests/bibtex.rs | 11 ++++++----- tests/tex-outputs.rs | 26 ++++++++++---------------- tests/util/mod.rs | 31 ++++++++++++++++++++++++++++--- 4 files changed, 77 insertions(+), 24 deletions(-) diff --git a/src/errors.rs b/src/errors.rs index dc3557b02..725e5c1e2 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -201,6 +201,22 @@ pub trait DefinitelySame { // } //} +impl DefinitelySame for Error { + fn definitely_same(&self, other: &Self) -> bool { + if !self.0.definitely_same(&other.0) { + return false; + } + self.1.definitely_same(&other.1) + } +} + +impl DefinitelySame for error_chain::State { + fn definitely_same(&self, other: &Self) -> bool { + // We ignore backtraces + self.next_error.definitely_same(&other.next_error) + } +} + impl DefinitelySame for ErrorKind { fn definitely_same(&self, other: &Self) -> bool { match self { @@ -233,6 +249,23 @@ impl DefinitelySame for NewError { } } +impl DefinitelySame for Box { + /// Hack alert! We only compare stringifications. + fn definitely_same(&self, other: &Self) -> bool { + self.to_string() == other.to_string() + } +} + +impl DefinitelySame for Option { + fn definitely_same(&self, other: &Self) -> bool { + match (self, other) { + (None, None) => true, + (Some(a), Some(b)) => a.definitely_same(b), + _ => false, + } + } +} + impl DefinitelySame for StdResult { fn definitely_same(&self, other: &Self) -> bool { match *self { diff --git a/tests/bibtex.rs b/tests/bibtex.rs index 86fa16b10..742ad8573 100644 --- a/tests/bibtex.rs +++ b/tests/bibtex.rs @@ -5,8 +5,9 @@ use std::collections::HashSet; use std::path::PathBuf; use tectonic::io::{FilesystemIo, IoProvider, IoStack, MemoryIo}; -use tectonic::BibtexEngine; +use tectonic::{errors::Result, BibtexEngine}; use tectonic_bridge_core::{CoreBridgeLauncher, MinimalDriver}; +use tectonic_engine_xetex::TexOutcome; use tectonic_status_base::NoopStatusBackend; #[path = "util/mod.rs"] @@ -17,6 +18,7 @@ struct TestCase { stem: String, subdir: Option, test_bbl: bool, + expected_result: Result, } impl TestCase { @@ -25,6 +27,7 @@ impl TestCase { stem: stem.to_owned(), subdir: subdir.map(String::from), test_bbl: true, + expected_result: Ok(TexOutcome::Spotless), } } @@ -41,7 +44,7 @@ impl TestCase { p } - fn go(&mut self) { + fn go(self) { util::set_test_root(); let mut p = self.test_dir(); @@ -68,7 +71,7 @@ impl TestCase { let files = mem.files.borrow(); - let mut expect = Expected::new(); + let mut expect = Expected::new().res(self.expected_result, res); if self.test_bbl { expect = @@ -78,8 +81,6 @@ impl TestCase { expect .file(ExpectedFile::read_with_extension(&mut p, "blg").collection(&files)) .finish(); - - res.unwrap(); } } diff --git a/tests/tex-outputs.rs b/tests/tex-outputs.rs index 1f56de445..f7371c791 100644 --- a/tests/tex-outputs.rs +++ b/tests/tex-outputs.rs @@ -40,17 +40,17 @@ impl TestCase { } } - fn check_synctex(&mut self, check_synctex: bool) -> &mut Self { + fn check_synctex(mut self, check_synctex: bool) -> Self { self.check_synctex = check_synctex; self } - fn check_pdf(&mut self, check_pdf: bool) -> &mut Self { + fn check_pdf(mut self, check_pdf: bool) -> Self { self.check_pdf = check_pdf; self } - fn with_fs(&mut self, path: &Path) -> &mut Self { + fn with_fs(mut self, path: &Path) -> Self { self.extra_io.push(Box::new(FilesystemIo::new( path, false, @@ -60,21 +60,21 @@ impl TestCase { self } - fn with_unstables(&mut self, unstables: UnstableOptions) -> &mut Self { + fn with_unstables(mut self, unstables: UnstableOptions) -> Self { self.unstables = unstables; self } - fn expect(&mut self, result: Result) -> &mut Self { + fn expect(mut self, result: Result) -> Self { self.expected_result = result; self } - fn expect_msg(&mut self, msg: &str) -> &mut Self { + fn expect_msg(self, msg: &str) -> Self { self.expect(Err(anyhow!("{}", msg))) } - fn go(&mut self) { + fn go(mut self) { util::set_test_root(); let expect_xdv = self.expected_result.is_ok(); @@ -147,19 +147,13 @@ impl TestCase { tex_res }; - if !res.definitely_same(&self.expected_result) { - eprintln!( - "expected TeX result {:?}, got {:?}", - self.expected_result, res - ); - panic!("the TeX engine returned an unexpected result code"); - } - // Check that outputs match expectations. let files = mem.files.borrow(); - let mut expect = Expected::new().file(expected_log.collection(&files)); + let mut expect = Expected::new() + .res(self.expected_result, res) + .file(expected_log.collection(&files)); if expect_xdv { expect = diff --git a/tests/util/mod.rs b/tests/util/mod.rs index fdd5d1d2b..25075315c 100644 --- a/tests/util/mod.rs +++ b/tests/util/mod.rs @@ -11,6 +11,7 @@ #![allow(dead_code)] use flate2::read::GzDecoder; +use std::fmt::Debug; use std::{ collections::HashSet, env, @@ -18,10 +19,13 @@ use std::{ io::{Read, Write}, path::{Path, PathBuf}, }; +use tectonic::errors::DefinitelySame; use tectonic::{errors::Result, io::memory::MemoryFileCollection}; use tectonic_bridge_core::{CoreBridgeLauncher, MinimalDriver}; +use tectonic_engine_xetex::TexOutcome; pub use tectonic::test_util::test_path; +use tectonic_errors::prelude::StdResult; /// Set the magic environment variable that enables the testing infrastructure /// embedded in the main Tectonic crate. This function is separated out from @@ -99,13 +103,26 @@ pub fn ensure_plain_format() -> Result { /// Convenience structure for comparing expected and actual output in various /// tests. #[must_use = "Expectations do nothing if not `finish`ed"] -pub struct Expected<'a> { +pub struct Expected<'a, E> { + result: Option<(StdResult, StdResult)>, files: Vec>, } -impl<'a> Expected<'a> { +impl<'a, E: DefinitelySame + Debug> Expected<'a, E> { pub fn new() -> Self { - Expected { files: Vec::new() } + Expected { + result: None, + files: Vec::new(), + } + } + + pub fn res( + mut self, + expected: StdResult, + found: StdResult, + ) -> Self { + self.result = Some((expected, found)); + self } pub fn file(mut self, file: ExpectedFile<'a>) -> Self { @@ -115,6 +132,14 @@ impl<'a> Expected<'a> { pub fn finish(&self) { let mut failures = Vec::new(); + if let Some((expected, found)) = &self.result { + if !found.definitely_same(expected) { + failures.push(format!( + "Expected engine result {:?}, got {:?}", + expected, found + )); + } + } for file in &self.files { if let Err(msg) = file.finish() { failures.push(msg); From 419e9af9c4c5d887146ddec6d726eb1cc5481adf Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Sat, 30 Mar 2024 18:13:31 -0700 Subject: [PATCH 2/6] Many small changes/improvements to the test utilities - Output PDFs in tex-outcomes when tex_res matches expected_res, not only on spotless - Lazily check results so warnings when expecting spotless doesn't prevent file mismatch checking - Check bibtex results - Output expected/observed files using a longer path that includes components, so tests with the same stem don't overlap --- tests/bibtex.rs | 78 +++++++++++++++++++++++++++++--------------- tests/tex-outputs.rs | 2 +- tests/trip.rs | 25 +++++++------- tests/util/mod.rs | 38 +++++++++++++++++---- 4 files changed, 97 insertions(+), 46 deletions(-) diff --git a/tests/bibtex.rs b/tests/bibtex.rs index 742ad8573..c91b5b1de 100644 --- a/tests/bibtex.rs +++ b/tests/bibtex.rs @@ -15,22 +15,26 @@ mod util; use crate::util::{test_path, Expected, ExpectedFile}; struct TestCase { - stem: String, - subdir: Option, + parts: &'static [&'static str], test_bbl: bool, expected_result: Result, } impl TestCase { - fn new(stem: &str, subdir: Option<&str>) -> Self { + fn new(parts: &'static [&'static str]) -> Self { + assert!(parts.len() >= 1); TestCase { - stem: stem.to_owned(), - subdir: subdir.map(String::from), + parts, test_bbl: true, expected_result: Ok(TexOutcome::Spotless), } } + fn expect(mut self, result: Result) -> Self { + self.expected_result = result; + self + } + fn test_bbl(mut self, test: bool) -> Self { self.test_bbl = test; self @@ -38,8 +42,8 @@ impl TestCase { fn test_dir(&self) -> PathBuf { let mut p = test_path(&["bibtex"]); - if let Some(subdir) = &self.subdir { - p.push(subdir); + for sub in &self.parts[..self.parts.len() - 1] { + p.push(sub); } p } @@ -49,7 +53,7 @@ impl TestCase { let mut p = self.test_dir(); - let auxname = format!("{}.aux", self.stem); + let auxname = format!("{}.aux", self.parts[self.parts.len() - 1]); // MemoryIo layer that will accept the outputs. let mut mem = MemoryIo::new(true); @@ -67,7 +71,7 @@ impl TestCase { // Check that outputs match expectations. - p.push(&self.stem); + p.push(&self.parts[self.parts.len() - 1]); let files = mem.files.borrow(); @@ -86,94 +90,115 @@ impl TestCase { #[test] fn test_single_entry() { - TestCase::new("single_entry", Some("cites")).go() + TestCase::new(&["cites", "single_entry"]).go() } #[test] fn test_brace_string() { - TestCase::new("odd_strings", Some("cites")).go(); + TestCase::new(&["cites", "odd_strings"]).go(); } #[test] fn test_many() { - TestCase::new("many", Some("cites")).go(); + TestCase::new(&["cites", "many"]) + .expect(Ok(TexOutcome::Warnings)) + .go(); } #[test] fn test_colon() { - TestCase::new("colon", Some("cites")).go(); + TestCase::new(&["cites", "colon"]) + .expect(Ok(TexOutcome::Warnings)) + .go(); } #[test] fn test_empty_files() { - TestCase::new("empty", None).test_bbl(false).go() + TestCase::new(&["empty"]) + .expect(Ok(TexOutcome::Errors)) + .test_bbl(false) + .go() } #[test] fn test_mismatched_function() { - TestCase::new("function", Some("mismatched_braces")) + TestCase::new(&["mismatched_braces", "function"]) + .expect(Ok(TexOutcome::Errors)) .test_bbl(false) .go(); } #[test] fn test_mismatched_expr() { - TestCase::new("expr", Some("mismatched_braces")) + TestCase::new(&["mismatched_braces", "expr"]) + .expect(Ok(TexOutcome::Errors)) .test_bbl(false) .go(); } #[test] fn test_mismatched_data() { - TestCase::new("data", Some("mismatched_braces")) + TestCase::new(&["mismatched_braces", "data"]) + .expect(Ok(TexOutcome::Errors)) .test_bbl(false) .go(); } #[test] fn test_mismatched_style() { - TestCase::new("style", Some("mismatched_braces")) + TestCase::new(&["mismatched_braces", "style"]) + .expect(Ok(TexOutcome::Errors)) .test_bbl(false) .go(); } #[test] fn test_duplicated_data() { - TestCase::new("data", Some("duplicated")) + TestCase::new(&["duplicated", "data"]) + .expect(Ok(TexOutcome::Errors)) .test_bbl(false) .go(); } #[test] fn test_duplicated_style() { - TestCase::new("style", Some("duplicated")) + TestCase::new(&["duplicated", "style"]) + .expect(Ok(TexOutcome::Errors)) .test_bbl(false) .go(); } #[test] fn test_bad_crossref() { - TestCase::new("bad", Some("crossref")).go(); + TestCase::new(&["crossref", "bad"]) + .expect(Ok(TexOutcome::Errors)) + .go(); } #[test] fn test_min_crossref() { - TestCase::new("min", Some("crossref")).go(); + TestCase::new(&["crossref", "min"]) + .expect(Ok(TexOutcome::Warnings)) + .go(); } #[test] fn test_single_preamble() { - TestCase::new("single", Some("preamble")).go(); + TestCase::new(&["preamble", "single"]) + .expect(Ok(TexOutcome::Warnings)) + .go(); } #[test] fn test_many_preamble() { - TestCase::new("many", Some("preamble")).go(); + TestCase::new(&["preamble", "many"]) + .expect(Ok(TexOutcome::Warnings)) + .go(); } #[test] fn test_nested_aux() { - TestCase::new("nested", Some("aux_files")).go(); + TestCase::new(&["aux_files", "nested"]).go(); } /// Test for [#1105](https://github.com/tectonic-typesetting/tectonic/issues/1105), with enough @@ -181,7 +206,8 @@ fn test_nested_aux() { /// at once. #[test] fn test_lots_of_cites() { - TestCase::new("lots_of_cites", Some("aux_files")) + TestCase::new(&["aux_files", "lots_of_cites"]) + .expect(Ok(TexOutcome::Warnings)) .test_bbl(false) .go(); } diff --git a/tests/tex-outputs.rs b/tests/tex-outputs.rs index f7371c791..9b42140a7 100644 --- a/tests/tex-outputs.rs +++ b/tests/tex-outputs.rs @@ -125,7 +125,7 @@ impl TestCase { .shell_escape(self.unstables.shell_escape) .process(&mut launcher, "plain.fmt", &texname); - if self.check_pdf && tex_res.definitely_same(&Ok(TexOutcome::Spotless)) { + if self.check_pdf && tex_res.definitely_same(&self.expected_result) { let mut engine = XdvipdfmxEngine::default(); engine diff --git a/tests/trip.rs b/tests/trip.rs index 6a15fffc1..a37d7174e 100644 --- a/tests/trip.rs +++ b/tests/trip.rs @@ -18,6 +18,7 @@ use tectonic::io::testing::SingleInputFileIo; use tectonic::io::{FilesystemPrimaryInputIo, IoProvider, IoStack, MemoryIo}; use tectonic::TexEngine; use tectonic_bridge_core::{CoreBridgeLauncher, MinimalDriver}; +use tectonic_engine_xetex::TexOutcome; use tectonic_status_base::NoopStatusBackend; #[path = "util/mod.rs"] @@ -50,7 +51,7 @@ fn trip_test() { let mut mem = MemoryIo::new(true); // First engine pass -- make the format file. - { + let res1 = { let io = IoStack::new(vec![&mut mem as &mut dyn IoProvider, &mut tex, &mut tfm]); let mut hooks = MinimalDriver::new(io); let mut status = NoopStatusBackend::default(); @@ -60,11 +61,10 @@ fn trip_test() { .halt_on_error_mode(false) .initex_mode(true) .process(&mut launcher, "INITEX", "trip") - .unwrap(); - } + }; // Second pass -- process it - { + let res2 = { let io = IoStack::new(vec![&mut mem as &mut dyn IoProvider, &mut tex, &mut tfm]); let mut hooks = MinimalDriver::new(io); let mut status = NoopStatusBackend::default(); @@ -74,12 +74,13 @@ fn trip_test() { .halt_on_error_mode(false) .initex_mode(false) .process(&mut launcher, "trip.fmt", "trip") - .unwrap(); - } + }; // Check that outputs match expectations. let files = &*mem.files.borrow(); Expected::new() + .res(Ok(TexOutcome::Spotless), res1) + .res(Ok(TexOutcome::Spotless), res2) .file(expected_log.collection(files)) .file(expected_xdv.collection(files)) .file(expected_os.collection(files)) @@ -113,7 +114,7 @@ fn etrip_test() { let files = mem.files.clone(); // First engine pass -- make the format file. - { + let res1 = { let io = IoStack::new(vec![&mut mem as &mut dyn IoProvider, &mut tex, &mut tfm]); let mut hooks = MinimalDriver::new(io); let mut status = NoopStatusBackend::default(); @@ -123,11 +124,10 @@ fn etrip_test() { .halt_on_error_mode(false) .initex_mode(true) .process(&mut launcher, "INITEX", "etrip") - .unwrap(); - } + }; // Second pass -- process it - { + let res2 = { let io = IoStack::new(vec![&mut mem, &mut tex, &mut tfm]); let mut hooks = MinimalDriver::new(io); let mut status = NoopStatusBackend::default(); @@ -137,12 +137,13 @@ fn etrip_test() { .halt_on_error_mode(false) .initex_mode(false) .process(&mut launcher, "etrip.fmt", "etrip") - .unwrap(); - } + }; // Check that outputs match expectations. let files = &*files.borrow(); Expected::new() + .res(Ok(TexOutcome::Spotless), res1) + .res(Ok(TexOutcome::Spotless), res2) .file(expected_log.collection(files)) .file(expected_xdv.collection(files)) .file(expected_out.collection(files)) diff --git a/tests/util/mod.rs b/tests/util/mod.rs index 25075315c..c59b4c8a5 100644 --- a/tests/util/mod.rs +++ b/tests/util/mod.rs @@ -104,14 +104,14 @@ pub fn ensure_plain_format() -> Result { /// tests. #[must_use = "Expectations do nothing if not `finish`ed"] pub struct Expected<'a, E> { - result: Option<(StdResult, StdResult)>, + result: Vec<(StdResult, StdResult)>, files: Vec>, } impl<'a, E: DefinitelySame + Debug> Expected<'a, E> { pub fn new() -> Self { Expected { - result: None, + result: Vec::new(), files: Vec::new(), } } @@ -121,7 +121,7 @@ impl<'a, E: DefinitelySame + Debug> Expected<'a, E> { expected: StdResult, found: StdResult, ) -> Self { - self.result = Some((expected, found)); + self.result.push((expected, found)); self } @@ -132,7 +132,7 @@ impl<'a, E: DefinitelySame + Debug> Expected<'a, E> { pub fn finish(&self) { let mut failures = Vec::new(); - if let Some((expected, found)) = &self.result { + for (expected, found) in &self.result { if !found.definitely_same(expected) { failures.push(format!( "Expected engine result {:?}, got {:?}", @@ -153,6 +153,7 @@ impl<'a, E: DefinitelySame + Debug> Expected<'a, E> { pub struct ExpectedFile<'a> { name: String, + expect_name: String, contents: Vec, gzipped: bool, @@ -162,12 +163,20 @@ pub struct ExpectedFile<'a> { impl<'a> ExpectedFile<'a> { pub fn read>(path: P) -> Self { let path = path.as_ref(); + + let root = test_path(&[]); let name = path .file_name() .unwrap_or_else(|| panic!("couldn't get file name of {:?}", path)) .to_str() .unwrap_or_else(|| panic!("couldn't Unicode-ify file name of {:?}", path)) .to_owned(); + let expect_name = path + .strip_prefix(root) + .unwrap_or_else(|_| panic!("couldn't get path relative to test root {:?}", path)) + .to_str() + .unwrap_or_else(|| panic!("couldn't Unicode-ify file name of {:?}", path)) + .replace(|c| std::path::is_separator(c), "_"); let mut f = File::open(path).unwrap_or_else(|_| panic!("failed to open {:?}", path)); let mut contents = Vec::new(); @@ -175,6 +184,7 @@ impl<'a> ExpectedFile<'a> { ExpectedFile { name, + expect_name, contents, gzipped: false, @@ -191,7 +201,20 @@ impl<'a> ExpectedFile<'a> { /// fill in the absolute paths of the output files (cf. #720) pub fn read_with_extension_rooted_gz(pbase: &mut PathBuf, extension: &str) -> Self { pbase.set_extension(extension); - let name = pbase.file_name().unwrap().to_str().unwrap().to_owned(); + + let root = test_path(&[]); + let name = pbase + .file_name() + .unwrap_or_else(|| panic!("couldn't get file name of {:?}", pbase)) + .to_str() + .unwrap_or_else(|| panic!("couldn't Unicode-ify file name of {:?}", pbase)) + .to_owned(); + let expect_name = pbase + .strip_prefix(root) + .unwrap_or_else(|_| panic!("couldn't get path relative to test root {:?}", pbase)) + .to_str() + .unwrap_or_else(|| panic!("couldn't Unicode-ify file name of {:?}", pbase)) + .replace(|c| std::path::is_separator(c), "_"); let mut dec = GzDecoder::new(File::open(&pbase).unwrap()); let mut contents = Vec::new(); @@ -216,6 +239,7 @@ impl<'a> ExpectedFile<'a> { ExpectedFile { name, + expect_name, contents, gzipped: true, @@ -242,7 +266,7 @@ impl<'a> ExpectedFile<'a> { // changed without being able to do diffs, etc. So, write out the // buffers. { - let mut n = self.name.clone(); + let mut n = self.expect_name.clone(); n.push_str(".expected"); let mut f = File::create(&n) .map_err(|_| format!("failed to create {} for test failure diagnosis", n))?; @@ -250,7 +274,7 @@ impl<'a> ExpectedFile<'a> { .map_err(|_| format!("failed to write {} for test failure diagnosis", n))?; } { - let mut n = self.name.clone(); + let mut n = self.expect_name.clone(); n.push_str(".observed"); let mut f = File::create(&n) .map_err(|_| format!("failed to create {} for test failure diagnosis", n))?; From 533a2f448d15984dd1c239a6e43a75788d428363 Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Sat, 30 Mar 2024 18:27:38 -0700 Subject: [PATCH 3/6] Publish test-failure artifacts. --- dist/azure-generic-build.yml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/dist/azure-generic-build.yml b/dist/azure-generic-build.yml index 370946521..40a376504 100644 --- a/dist/azure-generic-build.yml +++ b/dist/azure-generic-build.yml @@ -62,6 +62,22 @@ steps: - ${{ if parameters.testIt }}: - bash: cargo test --all --target $TARGET --release $CARGO_FEATURES_FLAGS --features="$CARGO_FEATURES_EXPLICIT" displayName: cargo test + - bash: | + artifact_dir="$(Build.ArtifactStagingDirectory)/test_failures" + mkdir -p "$artifact_dir" + mv *.observed "$artifact_dir" + mv *.expected "$artifact_dir" + if [ -n "$(ls -A $artifact_dir)" ]; then + echo "##vso[task.setvariable variable=TEST_FAILURE_ARTIFACTS;]true" + fi + displayName: Package test failure files + condition: failed() + - task: PublishPipelineArtifact@1 + displayName: Publish packaged test failures + condition: eq(variables['TEST_FAILURE_ARTIFACTS'], true) + inputs: + targetPath: '$(Build.ArtifactStagingDirectory)/test_failures' + artifactName: test-failures-$(TARGET) # For non-canary builds, export artifacts. From 5ba9e8695e68ade6b270a46a31983bc7af57e7ab Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Sat, 30 Mar 2024 20:39:53 -0700 Subject: [PATCH 4/6] Appease clippy --- tests/bibtex.rs | 4 ++-- tests/util/mod.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/bibtex.rs b/tests/bibtex.rs index c91b5b1de..53c24e7da 100644 --- a/tests/bibtex.rs +++ b/tests/bibtex.rs @@ -22,7 +22,7 @@ struct TestCase { impl TestCase { fn new(parts: &'static [&'static str]) -> Self { - assert!(parts.len() >= 1); + assert!(!parts.is_empty()); TestCase { parts, test_bbl: true, @@ -71,7 +71,7 @@ impl TestCase { // Check that outputs match expectations. - p.push(&self.parts[self.parts.len() - 1]); + p.push(self.parts[self.parts.len() - 1]); let files = mem.files.borrow(); diff --git a/tests/util/mod.rs b/tests/util/mod.rs index c59b4c8a5..91591d7b5 100644 --- a/tests/util/mod.rs +++ b/tests/util/mod.rs @@ -176,7 +176,7 @@ impl<'a> ExpectedFile<'a> { .unwrap_or_else(|_| panic!("couldn't get path relative to test root {:?}", path)) .to_str() .unwrap_or_else(|| panic!("couldn't Unicode-ify file name of {:?}", path)) - .replace(|c| std::path::is_separator(c), "_"); + .replace(std::path::is_separator, "_"); let mut f = File::open(path).unwrap_or_else(|_| panic!("failed to open {:?}", path)); let mut contents = Vec::new(); @@ -214,7 +214,7 @@ impl<'a> ExpectedFile<'a> { .unwrap_or_else(|_| panic!("couldn't get path relative to test root {:?}", pbase)) .to_str() .unwrap_or_else(|| panic!("couldn't Unicode-ify file name of {:?}", pbase)) - .replace(|c| std::path::is_separator(c), "_"); + .replace(std::path::is_separator, "_"); let mut dec = GzDecoder::new(File::open(&pbase).unwrap()); let mut contents = Vec::new(); From 369fa34046505760366d10905d1832bd18a5cd9b Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Sat, 30 Mar 2024 22:06:57 -0700 Subject: [PATCH 5/6] Fix trip/etrip --- tests/trip.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/trip.rs b/tests/trip.rs index a37d7174e..5f0433ba6 100644 --- a/tests/trip.rs +++ b/tests/trip.rs @@ -79,8 +79,8 @@ fn trip_test() { // Check that outputs match expectations. let files = &*mem.files.borrow(); Expected::new() - .res(Ok(TexOutcome::Spotless), res1) - .res(Ok(TexOutcome::Spotless), res2) + .res(Ok(TexOutcome::Errors), res1) + .res(Ok(TexOutcome::Errors), res2) .file(expected_log.collection(files)) .file(expected_xdv.collection(files)) .file(expected_os.collection(files)) @@ -142,8 +142,8 @@ fn etrip_test() { // Check that outputs match expectations. let files = &*files.borrow(); Expected::new() - .res(Ok(TexOutcome::Spotless), res1) - .res(Ok(TexOutcome::Spotless), res2) + .res(Ok(TexOutcome::Errors), res1) + .res(Ok(TexOutcome::Errors), res2) .file(expected_log.collection(files)) .file(expected_xdv.collection(files)) .file(expected_out.collection(files)) From 1f0fdc2e7ce9a09054bc92bf21683665f2f55e76 Mon Sep 17 00:00:00 2001 From: Rune Tynan Date: Fri, 5 Apr 2024 16:21:53 -0700 Subject: [PATCH 6/6] Add tests for DefinitelySame --- src/errors.rs | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/errors.rs b/src/errors.rs index 725e5c1e2..68459aa10 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -213,6 +213,7 @@ impl DefinitelySame for Error { impl DefinitelySame for error_chain::State { fn definitely_same(&self, other: &Self) -> bool { // We ignore backtraces + // We have to remove the Send bounds, which current Rust makes a bit annoying self.next_error.definitely_same(&other.next_error) } } @@ -413,3 +414,54 @@ where self.with_instance(|i| std::fmt::Debug::fmt(&i, f)) } } + +#[cfg(test)] +mod tests { + use super::*; + use error_chain::{ChainedError, State}; + use std::error::Error as StdError; + + #[test] + fn test_def_same_option() { + let a = Some(Box::from(NewError::msg("A"))); + let b = Some(Box::from(NewError::msg("A"))); + + assert!(a.definitely_same(&b)); + assert!(b.definitely_same(&a)); + + let b = Some(Box::from(NewError::msg("B"))); + assert!(!a.definitely_same(&b)); + + let b = None; + let c = None; + assert!(!a.definitely_same(&b)); + assert!(b.definitely_same(&c)); + } + + #[test] + fn test_def_same_err() { + let a = Error::new(ErrorKind::Msg(String::from("A")), State::default()); + let b = Error::new(ErrorKind::Msg(String::from("A")), State::default()); + + // Different backtraces but should be same + assert!(a.definitely_same(&b)); + + let b = Error::new(ErrorKind::BadLength(0, 0), State::default()); + assert!(!a.definitely_same(&b)); + + let a = Error::new(ErrorKind::NewStyle(NewError::msg("A")), State::default()); + assert!(!a.definitely_same(&b)); + + let b = Error::new(ErrorKind::NewStyle(NewError::msg("A")), State::default()); + assert!(a.definitely_same(&b)); + } + + #[test] + fn test_def_same_box_err() { + let a: Box = Box::from(NewError::msg("A")); + let b: Box = Box::from(NewError::msg("B")); + + assert!(a.definitely_same(&a)); + assert!(!a.definitely_same(&b)); + } +}