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.

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

Upgrade Smithy to 1.45 #3470

Merged
merged 6 commits into from
Mar 8, 2024
Merged

Upgrade Smithy to 1.45 #3470

merged 6 commits into from
Mar 8, 2024

Conversation

jdisanti
Copy link
Collaborator

@jdisanti jdisanti commented Mar 6, 2024

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@jdisanti jdisanti marked this pull request as ready for review March 6, 2024 23:03
@jdisanti jdisanti requested review from a team as code owners March 6, 2024 23:03
Copy link

github-actions bot commented Mar 6, 2024

A new generated diff is ready to view.

A new doc preview is ready to view.

@jdisanti
Copy link
Collaborator Author

jdisanti commented Mar 6, 2024

There's something weird going on with the RestJsonZeroAndFalseQueryValues (which was supposed to be fixed in smithy-lang/smithy#2132). I noticed I forgot to remove it from the deny list in this PR, and when removing it, the test fails to pass with:

failures:

---- operation::server_all_query_string_types_test::rest_json_zero_and_false_query_values_request stdout ----
thread 'operation::server_all_query_string_types_test::rest_json_zero_and_false_query_values_request' panicked at rest_json/rust-server-codegen/src/operation.rs:1229:25:
assertion failed: `(left == right)`: Unexpected value for `query_params_map_of_string_list`

Diff < left / right > :
 Some(
     {
<        "Boolean": [
>        "queryBoolean": [
             "false",
         ],
<        "Integer": [
>        "queryInteger": [
             "0",
         ],
     },
 )


note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    operation::server_all_query_string_types_test::rest_json_zero_and_false_query_values_request

test result: FAILED. 724 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.12s

I think I fixed the test incorrectly, but now I'm puzzled why the fixed test in smithy-rs didn't break. I guess it must not be running for some reason.

@jdisanti
Copy link
Collaborator Author

jdisanti commented Mar 6, 2024

It appears the codegen-server-test subproject doesn't regenerate any code when changing the rest-json-extras model. That's unfortunate.

Copy link

github-actions bot commented Mar 7, 2024

A new generated diff is ready to view.

A new doc preview is ready to view.

Copy link

github-actions bot commented Mar 8, 2024

A new generated diff is ready to view.

A new doc preview is ready to view.

Copy link
Contributor

@ysaito1001 ysaito1001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

smithy-lang/smithy#2167 seems to have been merged so it may not be too long before tests with TODO can be removed. The changes LGTM.

@jdisanti jdisanti enabled auto-merge March 8, 2024 19:33
@jdisanti jdisanti disabled auto-merge March 8, 2024 19:33
Copy link

github-actions bot commented Mar 8, 2024

A new generated diff is ready to view.

A new doc preview is ready to view.

@jdisanti jdisanti merged commit a413b9b into main Mar 8, 2024
41 checks passed
@jdisanti jdisanti deleted the jdisanti-upgrade-smithy branch March 8, 2024 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants