Skip to content

Commit

Permalink
Make order of expected/actual consistent in protocol tests (#2976)
Browse files Browse the repository at this point in the history
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._
  • Loading branch information
jdisanti authored Sep 8, 2023
1 parent 95e8c30 commit 5401e0d
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 33 deletions.
2 changes: 1 addition & 1 deletion rust-runtime/aws-smithy-client/src/test_connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
};
}
}
Expand Down
20 changes: 10 additions & 10 deletions rust-runtime/aws-smithy-protocol-test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,8 @@ pub fn validate_body<T: AsRef<[u8]>>(
) -> 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(),
Expand All @@ -318,7 +318,7 @@ pub fn validate_body<T: AsRef<[u8]>>(
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(),
Expand All @@ -327,7 +327,7 @@ pub fn validate_body<T: AsRef<[u8]>>(
(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 {
Expand Down Expand Up @@ -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,
}),
}
Expand Down
20 changes: 10 additions & 10 deletions rust-runtime/aws-smithy-protocol-test/src/urlencoded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
})
}
Expand All @@ -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",
)
);
}
Expand Down
26 changes: 14 additions & 12 deletions rust-runtime/aws-smithy-protocol-test/src/xml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
})
}
Expand Down

0 comments on commit 5401e0d

Please sign in to comment.