Merge pull request #1180 from CraftSpider/better-tests

Improve testing infrastructure

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.
 

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<dyn std::error::Error + Send> {
+    /// Hack alert! We only compare stringifications.
+    fn definitely_same(&self, other: &Self) -> bool {
+        self.to_string() == other.to_string()
+    }
+}
+
+impl<T: DefinitelySame> DefinitelySame for Option<T> {
+    fn definitely_same(&self, other: &Self) -> bool {
+        match (self, other) {
+            (None, None) => true,
+            (Some(a), Some(b)) => a.definitely_same(b),
+            _ => false,
+        }
+    }
+}
+
 impl<T: DefinitelySame, E: DefinitelySame> DefinitelySame for StdResult<T, E> {
     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<dyn StdError + Send> = Box::from(NewError::msg("A"));
+        let b: Box<dyn StdError + Send> = Box::from(NewError::msg("B"));
+
+        assert!(a.definitely_same(&a));
+        assert!(!a.definitely_same(&b));
+    }
+}

tests/bibtex.rs

@@ -5,48 +5,55 @@ 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"]
 mod util;
 use crate::util::{test_path, Expected, ExpectedFile};
 
 struct TestCase {
-    stem: String,
-    subdir: Option<String>,
+    parts: &'static [&'static str],
     test_bbl: bool,
+    expected_result: Result<TexOutcome>,
 }
 
 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<TexOutcome>) -> Self {
+        self.expected_result = result;
+        self
+    }
+
     fn test_bbl(mut self, test: bool) -> Self {
         self.test_bbl = test;
         self
     }
 
     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,109 +85,129 @@ 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
 /// citations in the aux and fields in the bst to require more than one allocation of field space
 /// 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();
 }