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. diff --git a/src/errors.rs b/src/errors.rs index dc3557b02..68459aa10 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -201,6 +201,23 @@ 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 + // We have to remove the Send bounds, which current Rust makes a bit annoying + self.next_error.definitely_same(&other.next_error) + } +} + impl DefinitelySame for ErrorKind { fn definitely_same(&self, other: &Self) -> bool { match self { @@ -233,6 +250,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 { @@ -380,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)); + } +} diff --git a/tests/bibtex.rs b/tests/bibtex.rs index 86fa16b10..53c24e7da 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"] @@ -14,20 +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.is_empty()); 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 @@ -35,18 +42,18 @@ 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 } - fn go(&mut self) { + fn go(self) { util::set_test_root(); 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); @@ -64,11 +71,11 @@ impl TestCase { // Check that outputs match expectations. - p.push(&self.stem); + p.push(self.parts[self.parts.len() - 1]); 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,101 +85,120 @@ impl TestCase { expect .file(ExpectedFile::read_with_extension(&mut p, "blg").collection(&files)) .finish(); - - res.unwrap(); } } #[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 @@ -180,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 1f56de445..9b42140a7 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(); @@ -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 @@ -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/trip.rs b/tests/trip.rs index 6a15fffc1..5f0433ba6 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::Errors), res1) + .res(Ok(TexOutcome::Errors), 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::Errors), res1) + .res(Ok(TexOutcome::Errors), 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 fdd5d1d2b..91591d7b5 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: Vec<(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: Vec::new(), + files: Vec::new(), + } + } + + pub fn res( + mut self, + expected: StdResult, + found: StdResult, + ) -> Self { + self.result.push((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(); + for (expected, found) in &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); @@ -128,6 +153,7 @@ impl<'a> Expected<'a> { pub struct ExpectedFile<'a> { name: String, + expect_name: String, contents: Vec, gzipped: bool, @@ -137,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(std::path::is_separator, "_"); let mut f = File::open(path).unwrap_or_else(|_| panic!("failed to open {:?}", path)); let mut contents = Vec::new(); @@ -150,6 +184,7 @@ impl<'a> ExpectedFile<'a> { ExpectedFile { name, + expect_name, contents, gzipped: false, @@ -166,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(std::path::is_separator, "_"); let mut dec = GzDecoder::new(File::open(&pbase).unwrap()); let mut contents = Vec::new(); @@ -191,6 +239,7 @@ impl<'a> ExpectedFile<'a> { ExpectedFile { name, + expect_name, contents, gzipped: true, @@ -217,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))?; @@ -225,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))?;