Skip to content

Conversation

@peterrsongg
Copy link
Contributor

Background

This change is in response to a bug found in C# while generating S3, where we weren't correctly writing xmlAttributes to the correct XmlElement. This was never caught in our generator since S3 had it correctly done by hand. This caused us to ship a bug which we quickly fixed.

In languages like C# where writing an xmlAttribute happens like so:


     //Write an element (this one is the root).
     writer.WriteStartElement("bookstore");

     //Write the namespace declaration.
     writer.WriteAttributeString("xmlns", "bk", null, "urn:samples");

     writer.WriteStartElement("book");

the ordering of the WriteAttributeString call matters, and so this test adds a test case where the @xmlAttribute is defined in the middle of the member list.

Testing

  • How did you test these changes? (skipping a lot of the output to just show successful build)
C:\Dev\Repos\smithy-fork [main ≡]> ./gradlew :smithy-aws-protocol-tests:build
Picked up JAVA_TOOL_OPTIONS: -Dlog4j2.formatMsgNoLookups=true
Starting a Gradle Daemon (subsequent builds will be faster)

> Task :smithy-aws-protocol-tests:smithyJarValidate
Running smithy validate
SUCCESS: Validated 3247 shapes (NOTE: 4)


[Incubating] Problems report is available at: file:///C:/Dev/Repos/smithy-fork/build/reports/problems/problems-report.html

Deprecated Gradle features were used in this build, making it incompatible with Gradle 10.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

For more on this, please refer to https://docs.gradle.org/9.1.0/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.

BUILD SUCCESSFUL in 3m 10s
52 actionable tasks: 40 executed, 12 up-to-date

Links

  • Links to additional context, if necessary
  • Issue #, if applicable (see here for a list of keywords to use for linking issues)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@peterrsongg peterrsongg requested a review from a team as a code owner December 1, 2025 21:29
@github-actions
Copy link
Contributor

github-actions bot commented Dec 1, 2025

This pull request does not contain a staged changelog entry. To create one, use the ./.changes/new-change command. For example:

./.changes/new-change --pull-requests "#2870" --type feature --description "Add xmlAttributesInMiddle protocol test"

Make sure that the description is appropriate for a changelog entry and that the proper feature type is used. See ./.changes/README or run ./.changes/new-change -h for more information.

sugmanue
sugmanue previously approved these changes Dec 2, 2025
@sugmanue
Copy link
Contributor

sugmanue commented Dec 2, 2025

Can you please create a change log for this PR? See this comment about it.

@sugmanue sugmanue self-requested a review December 2, 2025 21:38
@sugmanue sugmanue dismissed their stale review December 2, 2025 21:39

The change log is missing from this change.

@peterrsongg
Copy link
Contributor Author

Can you please create a change log for this PR? See this comment about it.

done.

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