From 5401e0d364ee2c68e0026b3d47e5f7649dd91ebe Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Fri, 8 Sep 2023 09:09:45 -0700 Subject: [PATCH] Make order of expected/actual consistent in protocol tests (#2976) Debugging tests is easier when the order of arguments in assertions is consistent across the board. With consistent order, you don't need to look at the line of code to know which value is the correct value. Most of the rest of the project uses expected/actual order, so this PR makes the protocol tests also use that order. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- .../aws-smithy-client/src/test_connection.rs | 2 +- .../aws-smithy-protocol-test/src/lib.rs | 20 +++++++------- .../src/urlencoded.rs | 20 +++++++------- .../aws-smithy-protocol-test/src/xml.rs | 26 ++++++++++--------- 4 files changed, 35 insertions(+), 33 deletions(-) diff --git a/rust-runtime/aws-smithy-client/src/test_connection.rs b/rust-runtime/aws-smithy-client/src/test_connection.rs index 1c90e7606f..b0600b369a 100644 --- a/rust-runtime/aws-smithy-client/src/test_connection.rs +++ b/rust-runtime/aws-smithy-client/src/test_connection.rs @@ -163,7 +163,7 @@ impl ValidateRequest { }; match (actual_str, expected_str) { (Ok(actual), Ok(expected)) => assert_ok(validate_body(actual, expected, media_type)), - _ => assert_eq!(actual.body().bytes(), expected.body().bytes()), + _ => assert_eq!(expected.body().bytes(), actual.body().bytes()), }; } } diff --git a/rust-runtime/aws-smithy-protocol-test/src/lib.rs b/rust-runtime/aws-smithy-protocol-test/src/lib.rs index d110272cea..111a8afa2f 100644 --- a/rust-runtime/aws-smithy-protocol-test/src/lib.rs +++ b/rust-runtime/aws-smithy-protocol-test/src/lib.rs @@ -307,8 +307,8 @@ pub fn validate_body>( ) -> Result<(), ProtocolTestFailure> { let body_str = std::str::from_utf8(actual_body.as_ref()); match (media_type, body_str) { - (MediaType::Json, Ok(actual_body)) => try_json_eq(actual_body, expected_body), - (MediaType::Xml, Ok(actual_body)) => try_xml_equivalent(actual_body, expected_body), + (MediaType::Json, Ok(actual_body)) => try_json_eq(expected_body, actual_body), + (MediaType::Xml, Ok(actual_body)) => try_xml_equivalent(expected_body, actual_body), (MediaType::Json, Err(_)) => Err(ProtocolTestFailure::InvalidBodyFormat { expected: "json".to_owned(), found: "input was not valid UTF-8".to_owned(), @@ -318,7 +318,7 @@ pub fn validate_body>( found: "input was not valid UTF-8".to_owned(), }), (MediaType::UrlEncodedForm, Ok(actual_body)) => { - try_url_encoded_form_equivalent(actual_body, expected_body) + try_url_encoded_form_equivalent(expected_body, actual_body) } (MediaType::UrlEncodedForm, Err(_)) => Err(ProtocolTestFailure::InvalidBodyFormat { expected: "x-www-form-urlencoded".to_owned(), @@ -327,7 +327,7 @@ pub fn validate_body>( (MediaType::Other(media_type), Ok(actual_body)) => { if actual_body != expected_body { Err(ProtocolTestFailure::BodyDidNotMatch { - comparison: pretty_comparison(actual_body, expected_body), + comparison: pretty_comparison(expected_body, actual_body), hint: format!("media type: {}", media_type), }) } else { @@ -358,25 +358,25 @@ impl Debug for PrettyString { } } -fn pretty_comparison(left: &str, right: &str) -> PrettyString { +fn pretty_comparison(expected: &str, actual: &str) -> PrettyString { PrettyString(format!( "{}", - Comparison::new(&PrettyStr(left), &PrettyStr(right)) + Comparison::new(&PrettyStr(expected), &PrettyStr(actual)) )) } -fn try_json_eq(actual: &str, expected: &str) -> Result<(), ProtocolTestFailure> { +fn try_json_eq(expected: &str, actual: &str) -> Result<(), ProtocolTestFailure> { + let expected_json: serde_json::Value = + serde_json::from_str(expected).expect("expected value must be valid JSON"); let actual_json: serde_json::Value = serde_json::from_str(actual).map_err(|e| ProtocolTestFailure::InvalidBodyFormat { expected: "json".to_owned(), found: e.to_string() + actual, })?; - let expected_json: serde_json::Value = - serde_json::from_str(expected).expect("expected value must be valid JSON"); match assert_json_eq_no_panic(&actual_json, &expected_json) { Ok(()) => Ok(()), Err(message) => Err(ProtocolTestFailure::BodyDidNotMatch { - comparison: pretty_comparison(actual, expected), + comparison: pretty_comparison(expected, actual), hint: message, }), } diff --git a/rust-runtime/aws-smithy-protocol-test/src/urlencoded.rs b/rust-runtime/aws-smithy-protocol-test/src/urlencoded.rs index 83c64e777b..e3c66bef43 100644 --- a/rust-runtime/aws-smithy-protocol-test/src/urlencoded.rs +++ b/rust-runtime/aws-smithy-protocol-test/src/urlencoded.rs @@ -41,16 +41,16 @@ fn rewrite_url_encoded_body(input: &str) -> String { } pub(crate) fn try_url_encoded_form_equivalent( - actual: &str, expected: &str, + actual: &str, ) -> Result<(), ProtocolTestFailure> { - let actual = rewrite_url_encoded_body(actual); let expected = rewrite_url_encoded_body(expected); + let actual = rewrite_url_encoded_body(actual); if actual == expected { Ok(()) } else { Err(ProtocolTestFailure::BodyDidNotMatch { - comparison: pretty_comparison(&actual, &expected), + comparison: pretty_comparison(&expected, &actual), hint: "".into(), }) } @@ -71,30 +71,30 @@ mod tests { ); assert!(try_url_encoded_form_equivalent( - "Action=Something&Version=test&Property=foo", "Action=Something&Version=test&Property=bar", + "Action=Something&Version=test&Property=foo", ) .is_err()); assert!(try_url_encoded_form_equivalent( - "Action=Something&Version=test&WrongProperty=foo", "Action=Something&Version=test&Property=foo", + "Action=Something&Version=test&WrongProperty=foo", ) .is_err()); assert_eq!( Ok(()), try_url_encoded_form_equivalent( - "Action=Something&Version=test\ - &SomeMap.1.key=foo\ - &SomeMap.1.value=Foo\ - &SomeMap.2.key=bar\ - &SomeMap.2.value=Bar", "Action=Something&Version=test\ &SomeMap.1.key=bar\ &SomeMap.1.value=Bar\ &SomeMap.2.key=foo\ &SomeMap.2.value=Foo", + "Action=Something&Version=test\ + &SomeMap.1.key=foo\ + &SomeMap.1.value=Foo\ + &SomeMap.2.key=bar\ + &SomeMap.2.value=Bar", ) ); } diff --git a/rust-runtime/aws-smithy-protocol-test/src/xml.rs b/rust-runtime/aws-smithy-protocol-test/src/xml.rs index a27420e36b..054bf4b7c1 100644 --- a/rust-runtime/aws-smithy-protocol-test/src/xml.rs +++ b/rust-runtime/aws-smithy-protocol-test/src/xml.rs @@ -11,23 +11,25 @@ use std::fmt::Write; /// /// This will normalize documents and attempts to determine if it is OK to sort members or not by /// using a heuristic to determine if the tag represents a list (which should not be reordered) -pub(crate) fn try_xml_equivalent(actual: &str, expected: &str) -> Result<(), ProtocolTestFailure> { - if actual == expected { +pub(crate) fn try_xml_equivalent(expected: &str, actual: &str) -> Result<(), ProtocolTestFailure> { + if expected == actual { return Ok(()); } - let norm_1 = normalize_xml(actual).map_err(|e| ProtocolTestFailure::InvalidBodyFormat { - expected: "actual document to be valid XML".to_string(), - found: format!("{}\n{}", e, actual), - })?; - let norm_2 = normalize_xml(expected).map_err(|e| ProtocolTestFailure::InvalidBodyFormat { - expected: "expected document to be valid XML".to_string(), - found: format!("{}", e), - })?; - if norm_1 == norm_2 { + let norm_expected = + normalize_xml(expected).map_err(|e| ProtocolTestFailure::InvalidBodyFormat { + expected: "expected document to be valid XML".to_string(), + found: format!("{}", e), + })?; + let norm_actual = + normalize_xml(actual).map_err(|e| ProtocolTestFailure::InvalidBodyFormat { + expected: "actual document to be valid XML".to_string(), + found: format!("{}\n{}", e, actual), + })?; + if norm_expected == norm_actual { Ok(()) } else { Err(ProtocolTestFailure::BodyDidNotMatch { - comparison: pretty_comparison(&norm_1, &norm_2), + comparison: pretty_comparison(&norm_expected, &norm_actual), hint: "".to_string(), }) }