Add tags and appliesTo to protocol tests #696
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kstich
requested changes
Jan 28, 2021
| @@ -265,7 +265,6 @@ structure InputAndOutputWithHeadersIO { | |||
| /// Null and empty headers are not sent over the wire. | |||
| @readonly | |||
| @http(uri: "/NullAndEmptyHeadersClient", method: "GET") | |||
| @tags(["client-only"]) | |||
| operation NullAndEmptyHeadersClient { | |||
There was a problem hiding this comment.
Can these operations be consolidated now that the distinction is on the tests themselves? This would require removing the operation level tags, though, and possibly cause a downstream issue. Maybe we just consolidate this as a followup.
There was a problem hiding this comment.
Since I'm adding tags back for now, this will need to wait. But it's a good idea.
smithy-aws-protocol-tests/model/restXml/document-structs.smithy
Outdated
Show resolved
Hide resolved
| @@ -265,7 +265,6 @@ structure InputAndOutputWithHeadersIO { | |||
| /// Null and empty headers are not sent over the wire. | |||
| @readonly | |||
| @http(uri: "/NullAndEmptyHeadersClient", method: "GET") | |||
| @tags(["client-only"]) | |||
| operation NullAndEmptyHeadersClient { | |||
There was a problem hiding this comment.
Can these operations be consolidated now that the distinction is on the tests themselves? This would require removing the operation level tags, though, and possibly cause a downstream issue. Maybe we just consolidate this as a followup.
This commit adds support for applying tags to individual test cases, allowing them to be grouped much more granularly. It also adds support for appliesTo, which allows test cases to indicate if they should only be implemented by clients or servers. Test cases that do not define an appliesTo member are to be implemented by both client and server implementations. Existing AWS protocol tests are updated to now utilize appliesTo, and the tags are left as-is. These tags will be removed in the future once implementations have migrated away from them.
mtdowling
force-pushed
the
add-tags-to-protocol-tests
branch
from
8b13ea9 to
9034f24
Compare
kstich
approved these changes
Jan 28, 2021
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit adds support for applying tags to individual test cases,
allowing them to be grouped much more granularly. It also adds support
for appliesTo, which allows test cases to indicate if they should only
be implemented by clients or servers. Test cases that do not define an
appliesTo member are to be implemented by both client and server
implementations.
Existing AWS protocol tests are updated to now utilize appliesTo rather
than adding tags to operations.