Add smoke test traits #2005
Add smoke test traits #2005
Conversation
This commit adds a new package containing the smithy model for smoke tests and their implementation. The model uses the `smithy.test` namespace, which it shares with other test trait models like protocol tests.
|
|
||
| /// A single smoke test case definition. | ||
| @private | ||
| structure SmokeTestCase { |
There was a problem hiding this comment.
I feel like we should share some test case definitions, maybe as mixins. A lot of this is exactly the same as protocol testsvand endpoint tests. It might make writing generators easier too if we have shared interfaces.
There was a problem hiding this comment.
Yea, there's a lot of crossover, but I wanted to just focus on smoke tests in this PR. If you think we should try to get it all in one though I can update it.
| } | ||
|
|
||
| /// A single smoke test case definition. | ||
| @private |
There was a problem hiding this comment.
What is and isn't private isn't consistent
There was a problem hiding this comment.
Made Tag private. Is that all you meant?
| public static Expectation failure(FailureExpectation failure) { | ||
| return new Expectation(failure); | ||
| } | ||
|
|
||
| public boolean isSuccess() { | ||
| return failure == null; | ||
| } | ||
|
|
||
| public boolean isFailure() { | ||
| return failure != null; | ||
| } | ||
|
|
||
| public Optional<FailureExpectation> getFailure() { | ||
| return Optional.ofNullable(failure); | ||
| } |
There was a problem hiding this comment.
public methods need docs
| if (!vendorParamsOptional.isPresent()) { | ||
| events.add(warning(shape, trait, | ||
| "Smoke test case defined a `vendorParamsShape` but no `vendorParams`")); |
There was a problem hiding this comment.
We should put a warning for the case where they have vendor params but no shape too
| events.add(warning(shape, trait, | ||
| "Smoke test case defined a `vendorParamsShape` but no `vendorParams`")); | ||
| } else { | ||
| Shape vendorParamsShape = model.expectShape(vendorParamsShapeOptional.get()); |
There was a problem hiding this comment.
You shouldn't use expectShape here since if the shape is missing then we lose out on validating all the other test cases. The idRef will already put an ERROR event in if the shape isn't present, so you don't need to duplicate that here, but you should tolerate the shape not being available.
(The http request test validator also has this unfortunate bit)
There was a problem hiding this comment.
Good point. Although I think we need to update the protocol test validator then:
| Map<String, List<Shape>> testCaseIdsToOperations = new HashMap<>(); | ||
| Set<Shape> shapes = model.getShapesWithTrait(SmokeTestsTrait.class); | ||
| for (Shape shape : shapes) { | ||
| SmokeTestsTrait trait = shape.expectTrait(SmokeTestsTrait.class); | ||
| for (SmokeTestCase testCase : trait.getTestCases()) { | ||
| testCaseIdsToOperations.computeIfAbsent(testCase.getId(), id -> new ArrayList<>()).add(shape); | ||
| } | ||
| } |
There was a problem hiding this comment.
You should be checking that no two cases within the scope of a service share ids (or within the trait if there's no service), but this will check every trait loaded.
There was a problem hiding this comment.
Fixed and updated test cases.
smithy-smoke-test-traits/src/main/resources/META-INF/smithy/smithy.test.smithy
Show resolved
Hide resolved
smithy-smoke-test-traits/src/main/resources/META-INF/smithy/smithy.test.smithy
Outdated
Show resolved
Hide resolved
smithy-smoke-test-traits/src/main/resources/META-INF/smithy/smithy.test.smithy
Show resolved
Hide resolved
| union Expectation { | ||
| /// Indicates that the call is expeted to not throw an error. No other | ||
| /// assertions are made about the response. | ||
| success: Unit |
There was a problem hiding this comment.
I guess if we ever wanted to do assertions on the output, we could add a new Expectation variant
| @@ -0,0 +1,67 @@ | |||
| /* | |||
| * Copyright 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. | |||
There was a problem hiding this comment.
I think we remove the date now, and maybe use the short form of license headers
There was a problem hiding this comment.
Removed the date, I'm not sure what the short form of license headers is though. Do you have an example?
...esources/software/amazon/smithy/smoketests/traits/errorfiles/vendor-params-validation.errors
Outdated
Show resolved
Hide resolved
...esources/software/amazon/smithy/smoketests/traits/errorfiles/vendor-params-validation.errors
Outdated
Show resolved
Hide resolved
This commit adds a new package containing the smithy model for smoke tests and their implementation. The model uses the
smithy.testnamespace, which it shares with other test trait models like protocol tests.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.