Unverified Commit 5401e0d3 authored by John DiSanti's avatar John DiSanti Committed by GitHub
Browse files

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._
parent 95e8c30b

rust-runtime/aws-smithy-client/src/test_connection.rs

+1 −1
Original line number Diff line number Diff line
@@ -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()),

        };

    }

}

rust-runtime/aws-smithy-protocol-test/src/lib.rs

+10 −10
Original line number Diff line number Diff line
@@ -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(),
@@ -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(),
@@ -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 {
@@ -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,

        }),

    }

rust-runtime/aws-smithy-protocol-test/src/urlencoded.rs

+10 −10
Original line number Diff line number Diff line
@@ -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",

            )

        );

    }

rust-runtime/aws-smithy-protocol-test/src/xml.rs

+14 −12
Original line number Diff line number Diff line
@@ -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 {

    let norm_expected =

        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_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(),

        })

    }