Skip to content

Commit 5bd4acf

Browse files
authored
Fix smoke test missing params (#2795)
If a smoke test case didn't define input `params`, the validation that normally checks to make sure `params` matches the operation input would not run. This meant that you could omit `params` even if the operation input has required members. I fixed this by manually checking for required members when `params` aren't defined (I couldn't use NodeValidationVisitor because it needs an actual node to check, plus the error message will be better this way). It emits an error, same as NodeValidationVisitor would. Also removed a branch in the smoke test input param validation logic, which would check for cases where `params` were provided for an operation with no input. This code was unreachable because the method being used to get the operation's input shape, `OperationIndex::expectInputShape`, always returns a value, and we were checking for null. There wasn't a test case for this, so I'm not sure why I wrote it that way to begin with.
1 parent 4a2ac32 commit 5bd4acf

File tree

3 files changed

+40
-9
lines changed

3 files changed

+40
-9
lines changed

smithy-smoke-test-traits/src/main/java/software/amazon/smithy/smoketests/traits/SmokeTestCaseValidator.java

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,19 @@
66

77
import java.util.ArrayList;
88
import java.util.List;
9+
import java.util.Map;
910
import java.util.Optional;
1011
import software.amazon.smithy.model.Model;
1112
import software.amazon.smithy.model.knowledge.OperationIndex;
12-
import software.amazon.smithy.model.node.Node;
1313
import software.amazon.smithy.model.node.ObjectNode;
14+
import software.amazon.smithy.model.shapes.MemberShape;
1415
import software.amazon.smithy.model.shapes.Shape;
1516
import software.amazon.smithy.model.shapes.ShapeId;
1617
import software.amazon.smithy.model.shapes.StructureShape;
1718
import software.amazon.smithy.model.validation.AbstractValidator;
1819
import software.amazon.smithy.model.validation.NodeValidationVisitor;
1920
import software.amazon.smithy.model.validation.ValidationEvent;
21+
import software.amazon.smithy.model.validation.ValidationUtils;
2022
import software.amazon.smithy.model.validation.node.TimestampValidationStrategy;
2123

2224
/**
@@ -72,21 +74,29 @@ public List<ValidationEvent> validate(Model model) {
7274

7375
// Validate input params
7476
StructureShape input = operationIndex.expectInputShape(shape);
75-
if (input != null && testCase.getParams().isPresent()) {
77+
if (testCase.getParams().isPresent()) {
7678
NodeValidationVisitor paramsValidator = createVisitor(testCase.getParams().get(),
7779
model,
7880
shape,
7981
testCase.getId(),
8082
".params");
8183
events.addAll(input.accept(paramsValidator));
82-
} else if (testCase.getParams().isPresent()) {
83-
events.add(error(shape,
84-
trait,
85-
String.format(
86-
"Smoke test parameters provided for operation with no input: `%s`",
87-
Node.printJson(testCase.getParams().get())
84+
} else {
85+
List<String> requiredMemberNames = new ArrayList<>();
86+
for (Map.Entry<String, MemberShape> entry : input.getAllMembers().entrySet()) {
87+
if (entry.getValue().isRequired()) {
88+
requiredMemberNames.add(entry.getKey());
89+
}
90+
}
8891

89-
)));
92+
if (!requiredMemberNames.isEmpty()) {
93+
events.add(error(shape,
94+
trait,
95+
String.format(
96+
"Smoke test case with ID `%s` does not define `params`, but the target operation has required input members: %s",
97+
testCase.getId(),
98+
ValidationUtils.tickedList(requiredMemberNames))));
99+
}
90100
}
91101
}
92102
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
[ERROR] smithy.example#Op: Smoke test case with ID `MissingParams` does not define `params`, but the target operation has required input members: `requiredInputMember` | SmokeTestCase
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
$version: "2.0"
2+
3+
namespace smithy.example
4+
5+
use smithy.test#smokeTests
6+
7+
@smokeTests([
8+
{
9+
id: "MissingParams"
10+
expect: {
11+
failure: {}
12+
}
13+
}
14+
])
15+
operation Op {
16+
input := {
17+
@required
18+
requiredInputMember: String
19+
}
20+
}

0 commit comments

Comments
 (0)