Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0015: Refactor Tests to Not Return Result<_> for Clearer Errors #45

Merged
merged 1 commit into from
Nov 23, 2021
Merged

0015: Refactor Tests to Not Return Result<_> for Clearer Errors #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 18 additions & 13 deletions fhir-bench-orchestrator/src/sample_data.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -463,7 +470,5 @@ mod tests {
orgs_with_id
);
}

Ok(())
}
}
24 changes: 9 additions & 15 deletions fhir-bench-orchestrator/src/servers/firely_spark.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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!())
Expand All @@ -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.");
}
}
}
24 changes: 9 additions & 15 deletions fhir-bench-orchestrator/src/servers/hapi_jpa.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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!())
Expand All @@ -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.");
}
}
}
24 changes: 9 additions & 15 deletions fhir-bench-orchestrator/src/servers/ibm_fhir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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!())
Expand All @@ -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.");
}
}
}
20 changes: 7 additions & 13 deletions fhir-bench-orchestrator/src/test_framework/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand All @@ -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.
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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(),
},
}],
Expand All @@ -693,7 +689,5 @@ mod tests {
};
let actual = serde_json::to_string(&actual).unwrap();
assert_eq!(expected, actual);

Ok(())
}
}
23 changes: 12 additions & 11 deletions fhir-bench-orchestrator/src/util/serde_histogram.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(())
}
}