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

feat: add test coverage to restxml #1214

Open
2 tasks
Tracked by #710
dayaffe opened this issue Oct 31, 2023 · 2 comments
Open
2 tasks
Tracked by #710

feat: add test coverage to restxml #1214

dayaffe opened this issue Oct 31, 2023 · 2 comments
Labels
feature-request A feature should be added or improved.

Comments

@dayaffe
Copy link
Collaborator

dayaffe commented Oct 31, 2023

Describe the feature

In tests for restxml/xml-enums.smithy we currently tests structures, lists, nested lists, and enum lists, but do not tests sets and maps. The following comment was left in code found here:

//  Todo: sets, maps
//  1.  This will be part of: XmlEnumsInputOutput
//    fooEnumSet: FooEnumSet,
//    fooEnumMap: FooEnumMap,
//
//  2.  Then add this:
//    set FooEnumSet {
//        member: FooEnum,
//    }
//
//    map FooEnumMap {
//        key: String,
//        value: FooEnum,
//    }
//

Use Case

We should support sets and maps in smithy files sent over restxml protocol and therefore should also provide adequate test coverage of these scenarios.

Proposed Solution

Add test coverage and address any gaps in our implementation.

Other Information

Related issue: #710

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change
@dayaffe dayaffe added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Oct 31, 2023
@jbelkins
Copy link
Contributor

Is it possible that Smithy has added these as protocol tests in the restxml suite since the TODO was written?
(Such a test is not Swift-specific and I would expect it to be in the cross-platform protocol test suite if it is needed to ensure an implementation understands the protocol)

@sichanyoo
Copy link
Contributor

This ticket will be part of clean-up: #1217.
Look into whether the files in same directory are covered by protocl tests in Smithy project already: https://github.com/smithy-lang/smithy/tree/main/smithy-aws-protocol-tests/model/restXml

@sichanyoo sichanyoo removed the needs-triage This issue or PR still needs to be triaged. label Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

3 participants