From 87f1822d2e9b403f5cc98daa52ff0bf1cbe058bc Mon Sep 17 00:00:00 2001 From: leodziki Date: Fri, 15 Nov 2024 04:20:03 -0800 Subject: [PATCH] Update coverage for config and error package Coverage update for serde_until completed. Bazel test, cargo test, pre-commit passed. --- nativelink-config/BUILD.bazel | 1 + nativelink-config/Cargo.toml | 1 + .../tests/deserialization_test.rs | 216 +++++++++++++++++- 3 files changed, 207 insertions(+), 11 deletions(-) diff --git a/nativelink-config/BUILD.bazel b/nativelink-config/BUILD.bazel index 3d36b2998..82eae2c38 100644 --- a/nativelink-config/BUILD.bazel +++ b/nativelink-config/BUILD.bazel @@ -41,6 +41,7 @@ rust_test_suite( "@crates//:pretty_assertions", "@crates//:serde", "@crates//:serde_json5", + "@crates//:serde_test", ], ) diff --git a/nativelink-config/Cargo.toml b/nativelink-config/Cargo.toml index c4881854b..193299f63 100644 --- a/nativelink-config/Cargo.toml +++ b/nativelink-config/Cargo.toml @@ -12,3 +12,4 @@ shellexpand = { version = "3.1.0", default-features = false, features = ["base-0 [dev-dependencies] pretty_assertions = { version = "1.4.1", features = ["std"] } +serde_test = "1.0" diff --git a/nativelink-config/tests/deserialization_test.rs b/nativelink-config/tests/deserialization_test.rs index d408b3996..90f8f1dfb 100644 --- a/nativelink-config/tests/deserialization_test.rs +++ b/nativelink-config/tests/deserialization_test.rs @@ -14,7 +14,9 @@ use nativelink_config::serde_utils::{ convert_data_size_with_shellexpand, convert_duration_with_shellexpand, - convert_optional_numeric_with_shellexpand, convert_optional_string_with_shellexpand, + convert_numeric_with_shellexpand, convert_optional_numeric_with_shellexpand, + convert_optional_string_with_shellexpand, convert_string_with_shellexpand, + convert_vec_string_with_shellexpand, }; use serde::Deserialize; @@ -26,11 +28,11 @@ struct DurationEntity { #[derive(Deserialize, Debug)] struct DataSizeEntity { - #[serde(default, deserialize_with = "convert_data_size_with_shellexpand")] - data_size: usize, + #[serde(deserialize_with = "convert_data_size_with_shellexpand")] + data_size: u128, } -#[derive(Deserialize, Debug)] +#[derive(Deserialize, Debug, PartialEq)] struct OptionalNumericEntity { #[serde( default, @@ -44,6 +46,22 @@ struct OptionalStringEntity { #[serde(default, deserialize_with = "convert_optional_string_with_shellexpand")] value: Option, } +#[derive(Deserialize, Debug)] +struct NumericEntity { + #[serde(deserialize_with = "convert_numeric_with_shellexpand")] + value: i64, +} +#[derive(Deserialize, Debug)] +struct StringEntity { + #[serde(deserialize_with = "convert_string_with_shellexpand")] + value: String, +} + +#[derive(Deserialize, Debug)] +struct VecStringEntity { + #[serde(deserialize_with = "convert_vec_string_with_shellexpand")] + values: Vec, +} mod duration_tests { use super::*; @@ -222,7 +240,7 @@ mod optional_values_tests { let examples = [ (r#"{"value": null}"#, None), (r#"{"value": 42}"#, Some(42)), - (r"{}", None), // Missing field + (r"{}", None), ]; for (input, expected) in examples { @@ -332,33 +350,27 @@ mod shellexpand_tests { std::env::set_var("TEST_VAR", "test_value"); std::env::set_var("EMPTY_VAR", ""); - // Test duration with environment variable let duration_result = serde_json5::from_str::(r#"{"duration": "${TEST_DURATION}"}"#).unwrap(); assert_eq!(duration_result.duration, 300); - // Test data size with environment variable let size_result = serde_json5::from_str::(r#"{"data_size": "${TEST_SIZE}"}"#).unwrap(); assert_eq!(size_result.data_size, 1_000_000_000); - // Test optional numeric with environment variable let numeric_result = serde_json5::from_str::(r#"{"value": "${TEST_NUMBER}"}"#) .unwrap(); assert_eq!(numeric_result.value, Some(42)); - // Test optional string with environment variable let string_result = serde_json5::from_str::(r#"{"value": "${TEST_VAR}"}"#).unwrap(); assert_eq!(string_result.value, Some("test_value".to_string())); - // Test optional string with empty environment variable let empty_string_result = serde_json5::from_str::(r#"{"value": "${EMPTY_VAR}"}"#).unwrap(); assert_eq!(empty_string_result.value, Some(String::new())); - // Test undefined environment variable let undefined_result = serde_json5::from_str::(r#"{"value": "${UNDEFINED_VAR}"}"#); assert!(undefined_result @@ -367,3 +379,185 @@ mod shellexpand_tests { .contains("environment variable not found")); } } +#[cfg(test)] +mod convert_numeric_with_shellexpand_tests { + use std::env; + + use serde_test::{assert_de_tokens_error, Token}; + + use super::*; + + #[test] + fn test_numeric_parsing() { + let json = r#"{"value": 42}"#; + let deserialized: NumericEntity = serde_json5::from_str(json).unwrap(); + assert_eq!(deserialized.value, 42); + + let json_str = r#"{"value": "42"}"#; + let deserialized_str: NumericEntity = serde_json5::from_str(json_str).unwrap(); + assert_eq!(deserialized_str.value, 42); + } + + #[test] + fn test_numeric_parsing_with_env_var() { + env::set_var("TEST_NUMERIC", "42"); + let json_env = r#"{"value": "${TEST_NUMERIC}"}"#; + let deserialized_env: NumericEntity = serde_json5::from_str(json_env).unwrap(); + assert_eq!(deserialized_env.value, 42); + } + + #[test] + fn test_numeric_parsing_with_large_values() { + let large_value = "9223372036854775807"; // i64::MAX + let json = format!(r#"{{"value": "{large_value}"}}"#); + let deserialized: NumericEntity = serde_json5::from_str(&json).unwrap(); + assert_eq!(deserialized.value, 9_223_372_036_854_775_807); + } + + #[test] + fn test_numeric_invalid_value_error() { + let invalid_json = r#"{"value": "not_a_number"}"#; + let result = serde_json5::from_str::(invalid_json); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("invalid digit found in string")); + } + + #[test] + fn test_numeric_invalid_env_var_error() { + let invalid_json = r#"{"value": "${UNDEFINED_ENV_VAR}"}"#; + let result = serde_json5::from_str::(invalid_json); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("environment variable not found")); + } + + #[test] + fn test_expectation_error_message() { + let invalid_json = r#"{"value": true}"#; + let result = serde_json5::from_str::(invalid_json); + assert!(result.is_err()); + let error_message = result.unwrap_err().to_string(); + assert!(error_message.contains("an integer or a plain number string")); + } + + #[test] + fn test_visit_u64_within_i64_range() { + let json = r#"{"value": 9223372036854775807}"#; + let deserialized: NumericEntity = serde_json5::from_str(json).unwrap(); + assert_eq!(deserialized.value, 9_223_372_036_854_775_807); + } + + #[test] + fn test_visit_u64_exceeds_i64_range() { + assert_de_tokens_error::( + &[ + Token::Map { len: Some(1) }, + Token::Str("value"), + Token::U64(9_223_372_036_854_775_808), + Token::MapEnd, + ], + "out of range integral type conversion attempted", + ); + } +} +#[cfg(test)] +mod shellexpand_string_tests { + use std::env; + + use super::*; + + #[test] + fn test_convert_string_with_shellexpand() { + env::set_var("TEST_STRING", "expanded_value"); + + let json = r#"{"value": "${TEST_STRING}"}"#; + + let deserialized: StringEntity = serde_json5::from_str(json).unwrap(); + + assert_eq!(deserialized.value, "expanded_value"); + } + + #[test] + fn test_convert_vec_string_with_shellexpand() { + env::set_var("TEST_VAR1", "value1"); + env::set_var("TEST_VAR2", "value2"); + + let json = r#"{"values": ["${TEST_VAR1}", "static_value", "${TEST_VAR2}"]}"#; + let deserialized: VecStringEntity = serde_json5::from_str(json).unwrap(); + + assert_eq!( + deserialized.values, + vec!["value1", "static_value", "value2"] + ); + } +} +#[cfg(test)] +mod convert_optional_numeric_with_shellexpand_tests { + + use serde_test::{assert_de_tokens_error, Token}; + + use super::*; + + #[test] + fn test_visit_unit_returns_none() { + serde_test::assert_de_tokens( + &OptionalNumericEntity { value: None }, + &[ + Token::Map { len: Some(1) }, + Token::Str("value"), + Token::Unit, + Token::MapEnd, + ], + ); + } + + #[test] + fn test_visit_u64_within_i64_range() { + let json = r#"{"value": 9223372036854775807}"#; + let deserialized: OptionalNumericEntity = serde_json5::from_str(json).unwrap(); + assert_eq!(deserialized.value, Some(9_223_372_036_854_775_807)); + } + + #[test] + fn test_visit_u64_exceeds_i64_range() { + assert_de_tokens_error::( + &[ + Token::Map { len: Some(1) }, + Token::Str("value"), + Token::U64(9_223_372_036_854_775_808), + Token::MapEnd, + ], + "out of range integral type conversion attempted", + ); + } + + #[test] + fn test_visit_some_valid_value() { + let json = r#"{"value": "42"}"#; + let deserialized: OptionalNumericEntity = serde_json5::from_str(json).unwrap(); + assert_eq!(deserialized.value, Some(42)); + } + + #[test] + fn test_visit_str_empty_string_error() { + let json = r#"{"value": ""}"#; + let result = serde_json5::from_str::(json); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("empty string is not a valid number")); + } + + #[test] + fn test_visit_str_whitespace_only() { + let json = r#"{"value": " "}"#; + let deserialized: OptionalNumericEntity = serde_json5::from_str(json).unwrap(); + assert_eq!(deserialized.value, None); + } +}