Tests should not return Result for clearer errors.

It's kinda' sorta' explained here:
<rust-lang/rust#69517>. Basically, though,
tests that return a `Result<_>` will have less helpful backtraces than
tests that just use asserts and `unwrap()/expect(_)` all over the place.

I recently ran into an undiagnosable test failure that this commit makes
diagnosable, so this is now "the way things are done" for this project,
going forwards.

fhir-bench-orchestrator/src/sample_data.rs

@@ -393,15 +393,15 @@ fn find_sample_data(data_dir: PathBuf) -> Result<SampleData> {
 #[cfg(test)]
 mod tests {
     use crate::sample_data::{SampleResource, SampleResourceMetadata};
-    use eyre::Result;
     use std::collections::HashMap;
 
     /// Verifies that [crate::sample_data::generate_data] works as expected.
     #[tracing::instrument(level = "info")]
     #[test_env_log::test(tokio::test)]
-    async fn generate_data() -> Result<()> {
-        let benchmark_dir = crate::config::benchmark_dir()?;
-        let target_dir = tempfile::tempdir()?;
+    async fn generate_data() {
+        let benchmark_dir =
+            crate::config::benchmark_dir().expect("Unable to get benchmark directory.");
+        let target_dir = tempfile::tempdir().expect("Unable to create temp target directory.");
         let sample_data = super::generate_data(
             benchmark_dir.join("synthetic-data").as_path(),
             10,
@@ -414,23 +414,30 @@ mod tests {
             "Sample data generation failed: {:?}",
             sample_data
         );
-        assert_ne!(0, sample_data?.patients.len(), "No patient files found");
-
-        Ok(())
+        assert_ne!(
+            0,
+            sample_data
+                .expect("Failed to generate sample data.")
+                .patients
+                .len(),
+            "No patient files found"
+        );
     }
 
     /// Verifies that [crate::sample_data::SampleData::iter_orgs] works as expected.
     #[tracing::instrument(level = "info")]
     #[test_env_log::test(tokio::test)]
-    async fn iter_orgs() -> Result<()> {
-        let benchmark_dir = crate::config::benchmark_dir()?;
-        let target_dir = tempfile::tempdir()?;
+    async fn iter_orgs() {
+        let benchmark_dir =
+            crate::config::benchmark_dir().expect("Unable to get benchmark directory.");
+        let target_dir = tempfile::tempdir().expect("Unable to create temp target directory.");
         let sample_data = super::generate_data(
             benchmark_dir.join("synthetic-data").as_path(),
             10,
             target_dir.path(),
         )
-        .await?;
+        .await
+        .expect("Failed to generate sample data.");
         let orgs: Vec<SampleResource> = sample_data.iter_orgs().collect();
 
         // Synthea generates randomized output, but our default config should always produce at least this
@@ -463,7 +470,5 @@ mod tests {
                 orgs_with_id
             );
         }
-
-        Ok(())
     }
 }

fhir-bench-orchestrator/src/servers/firely_spark.rs

@@ -215,12 +215,10 @@ impl ServerHandle for SparkFhirServerHandle {
 mod tests {
     use std::{ffi::OsStr, path::Path};
 
-    use eyre::{eyre, Result};
-
     #[tracing::instrument(level = "info")]
     #[test_env_log::test(tokio::test)]
     #[serial_test::serial(sample_data)]
-    async fn verify_server_launch() -> Result<()> {
+    async fn verify_server_launch() {
         let log_target = std::env::temp_dir().join(format!(
             "{}_verify_server_launch.log",
             Path::new(file!())
@@ -244,18 +242,14 @@ mod tests {
         };
 
         // Clean up the log if things went well, otherwise return an error with the path to it.
-        match launch_result {
-            Ok(_) => {
-                if log_target.exists() {
-                    std::fs::remove_file(log_target)?;
-                }
-                Ok(())
-            }
-            Err(err) => Err(eyre!(
-                "Server launch test failed due to error: {:?}. Log output: {:?}",
-                err,
-                log_target
-            )),
+        assert!(
+            launch_result.is_ok(),
+            "Server launch test failed due to error: {:?}. Log output: {:?}",
+            launch_result.unwrap_err(),
+            log_target
+        );
+        if log_target.exists() {
+            std::fs::remove_file(log_target).expect("Unable to remove temp file.");
         }
     }
 }

fhir-bench-orchestrator/src/servers/hapi_jpa.rs

@@ -218,12 +218,10 @@ impl ServerHandle for HapiJpaFhirServerHandle {
 mod tests {
     use std::{ffi::OsStr, path::Path};
 
-    use eyre::{eyre, Result};
-
     #[tracing::instrument(level = "info")]
     #[test_env_log::test(tokio::test)]
     #[serial_test::serial(sample_data)]
-    async fn verify_server_launch() -> Result<()> {
+    async fn verify_server_launch() {
         let log_target = std::env::temp_dir().join(format!(
             "{}_verify_server_launch.log",
             Path::new(file!())
@@ -247,18 +245,14 @@ mod tests {
         };
 
         // Clean up the log if things went well, otherwise return an error with the path to it.
-        match launch_result {
-            Ok(_) => {
-                if log_target.exists() {
-                    std::fs::remove_file(log_target)?;
-                }
-                Ok(())
-            }
-            Err(err) => Err(eyre!(
-                "Server launch test failed due to error: {:?}. Log output: {:?}",
-                err,
-                log_target
-            )),
+        assert!(
+            launch_result.is_ok(),
+            "Server launch test failed due to error: {:?}. Log output: {:?}",
+            launch_result.unwrap_err(),
+            log_target
+        );
+        if log_target.exists() {
+            std::fs::remove_file(log_target).expect("Unable to remove temp file.");
         }
     }
 }

fhir-bench-orchestrator/src/servers/ibm_fhir.rs

@@ -207,12 +207,10 @@ impl ServerHandle for IbmFhirServerHandle {
 mod tests {
     use std::{ffi::OsStr, path::Path};
 
-    use eyre::{eyre, Result};
-
     #[tracing::instrument(level = "info")]
     #[test_env_log::test(tokio::test)]
     #[serial_test::serial(sample_data)]
-    async fn verify_server_launch() -> Result<()> {
+    async fn verify_server_launch() {
         let log_target = std::env::temp_dir().join(format!(
             "{}_verify_server_launch.log",
             Path::new(file!())
@@ -236,18 +234,14 @@ mod tests {
         };
 
         // Clean up the log if things went well, otherwise return an error with the path to it.
-        match launch_result {
-            Ok(_) => {
-                if log_target.exists() {
-                    std::fs::remove_file(log_target)?;
-                }
-                Ok(())
-            }
-            Err(err) => Err(eyre!(
-                "Server launch test failed due to error: {:?}. Log output: {:?}",
-                err,
-                log_target
-            )),
+        assert!(
+            launch_result.is_ok(),
+            "Server launch test failed due to error: {:?}. Log output: {:?}",
+            launch_result.unwrap_err(),
+            log_target
+        );
+        if log_target.exists() {
+            std::fs::remove_file(log_target).expect("Unable to remove temp file.");
         }
     }
 }

fhir-bench-orchestrator/src/test_framework/mod.rs

@@ -439,7 +439,6 @@ mod tests {
     use crate::{config::AppConfig, test_framework::FrameworkMetadata};
     use chrono::prelude::*;
     use chrono::Duration;
-    use eyre::Result;
     use hdrhistogram::Histogram;
     use serde_json::json;
 
@@ -470,7 +469,7 @@ mod tests {
     /// Verifies that `ServerOperationMetrics` serializes as expected.
     #[tracing::instrument(level = "info")]
     #[test_env_log::test(tokio::test)]
-    async fn serialize_server_operation_metrics() -> Result<()> {
+    async fn serialize_server_operation_metrics() {
         let expected = json!({
             "throughput_per_second": 42.0,
             "latency_millis_mean": 1.0,
@@ -491,13 +490,11 @@ mod tests {
             latency_millis_p99: 1,
             latency_millis_p999: 1,
             latency_millis_p100: 1,
-            latency_histogram: Histogram::<u64>::new(3)?,
+            latency_histogram: Histogram::<u64>::new(3).expect("Error creating histogram."),
             latency_histogram_hgrm_gzip: "foo".into(),
         };
         let actual = serde_json::to_string(&actual).unwrap();
         assert_eq!(expected, actual);
-
-        Ok(())
     }
 
     /// Verifies that `ServerOperationLog` serializes as expected.
@@ -522,7 +519,7 @@ mod tests {
     /// Verifies that [ServerOperationMeasurement] serializes as expected.
     #[tracing::instrument(level = "info")]
     #[test_env_log::test(tokio::test)]
-    async fn serialize_server_operation_measurement() -> Result<()> {
+    async fn serialize_server_operation_measurement() {
         let expected = json!({
             "concurrent_users": 10,
             "started": "2020-01-01T15:00:00Z",
@@ -558,20 +555,18 @@ mod tests {
                 latency_millis_p99: 1,
                 latency_millis_p999: 1,
                 latency_millis_p100: 1,
-                latency_histogram: Histogram::<u64>::new(3)?,
+                latency_histogram: Histogram::<u64>::new(3).expect("Error creating histogram."),
                 latency_histogram_hgrm_gzip: "foo".into(),
             },
         };
         let actual = serde_json::to_string(&actual).unwrap();
         assert_eq!(expected, actual);
-
-        Ok(())
     }
 
     /// Verifies that `FrameworkResults` serializes as expected.
     #[tracing::instrument(level = "info")]
     #[test_env_log::test(tokio::test)]
-    async fn serialize_framework_results() -> Result<()> {
+    async fn serialize_framework_results() {
         let expected = json!({
             "started": "2020-01-01T12:00:00Z",
             "completed": "2020-01-01T19:00:00Z",
@@ -679,7 +674,8 @@ mod tests {
                             latency_millis_p99: 1,
                             latency_millis_p999: 1,
                             latency_millis_p100: 1,
-                            latency_histogram: Histogram::<u64>::new(3)?,
+                            latency_histogram: Histogram::<u64>::new(3)
+                                .expect("Error creating histogram."),
                             latency_histogram_hgrm_gzip: "foo".into(),
                         },
                     }],
@@ -693,7 +689,5 @@ mod tests {
         };
         let actual = serde_json::to_string(&actual).unwrap();
         assert_eq!(expected, actual);
-
-        Ok(())
     }
 }

fhir-bench-orchestrator/src/util/serde_histogram.rs

@@ -58,7 +58,6 @@ where
 /// Unit tests for the [Duration] serializer & deserialzer.
 #[cfg(test)]
 mod tests {
-    use eyre::Result;
     use hdrhistogram::Histogram;
     use serde::{Deserialize, Serialize};
     use serde_json::json;
@@ -73,36 +72,38 @@ mod tests {
     /// Verifies that [Duration] values serialize as expected.
     #[tracing::instrument(level = "info")]
     #[test_env_log::test(tokio::test)]
-    async fn serialize() -> Result<()> {
+    async fn serialize() {
         let expected = json!({
             "histogram": "HISTFAAAABx4nJNpmSzMwMDAxAABzFCaEUoz2X+AsQA/awKA",
         });
         let expected = serde_json::to_string(&expected).unwrap();
         let mut actual = DurationStruct {
-            histogram: Histogram::<u64>::new(3)?,
+            histogram: Histogram::<u64>::new(3).expect("Error creating histogram."),
         };
-        actual.histogram.record(1)?;
+        actual
+            .histogram
+            .record(1)
+            .expect("Error recording into histogram.");
         let actual = serde_json::to_string(&actual).unwrap();
         assert_eq!(expected, actual);
-
-        Ok(())
     }
 
     /// Verifies that [Duration] values deserialize as expected.
     #[tracing::instrument(level = "info")]
     #[test_env_log::test(tokio::test)]
-    async fn deserialize() -> Result<()> {
+    async fn deserialize() {
         let mut expected = DurationStruct {
-            histogram: Histogram::<u64>::new(3)?,
+            histogram: Histogram::<u64>::new(3).expect("Error creating histogram."),
         };
-        expected.histogram.record(1)?;
+        expected
+            .histogram
+            .record(1)
+            .expect("Error recording into histogram.");
         let actual = json!({
             "histogram": "HISTFAAAABx4nJNpmSzMwMDAxAABzFCaEUoz2X+AsQA/awKA",
         });
         let actual = serde_json::to_string(&actual).unwrap();
         let actual: DurationStruct = serde_json::from_str(&actual).unwrap();
         assert_eq!(expected.histogram, actual.histogram);
-
-        Ok(())
     }
 }