diff --git a/.changes/next-release/feature-c6ca466b9d6f375fd8c739fce3768c560bcc4dbd.json b/.changes/next-release/feature-c6ca466b9d6f375fd8c739fce3768c560bcc4dbd.json new file mode 100644 index 00000000000..2bfe093b5f5 --- /dev/null +++ b/.changes/next-release/feature-c6ca466b9d6f375fd8c739fce3768c560bcc4dbd.json @@ -0,0 +1,7 @@ +{ + "type": "feature", + "description": "Added new ERROR event to EndpointTestsTraitValidator when builtin params are inconsistent", + "pull_requests": [ + "[#2955](https://github.com/smithy-lang/smithy/pull/2955)" + ] +} diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/validators/EndpointTestsTraitValidator.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/validators/EndpointTestsTraitValidator.java index c2d3f0de3b2..2cf43710bed 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/validators/EndpointTestsTraitValidator.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/validators/EndpointTestsTraitValidator.java @@ -8,9 +8,13 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Map.Entry; +import java.util.Optional; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.knowledge.TopDownIndex; import software.amazon.smithy.model.node.Node; +import software.amazon.smithy.model.node.ObjectNode; +import software.amazon.smithy.model.node.StringNode; import software.amazon.smithy.model.shapes.OperationShape; import software.amazon.smithy.model.shapes.ServiceShape; import software.amazon.smithy.model.shapes.StructureShape; @@ -71,6 +75,7 @@ private void validateEndpointRuleSet( // be used frequently in downstream validation. List builtInParamsWithDefaults = new ArrayList<>(); List builtInParamsWithoutDefaults = new ArrayList<>(); + Map builtInParameters = new HashMap<>(); for (Parameter parameter : parameters) { if (parameter.isBuiltIn()) { @@ -79,6 +84,7 @@ private void validateEndpointRuleSet( } else { builtInParamsWithoutDefaults.add(parameter); } + builtInParameters.put(parameter.getName().getName().getValue(), parameter.getBuiltIn().get()); } } @@ -112,6 +118,11 @@ private void validateEndpointRuleSet( testCase, testOperationInput, events); + validateBuiltInsUseInconsistentValues(serviceShape, + builtInParameters, + testCase, + testOperationInput, + events); StructureShape inputShape = model.expectShape( operationNameMap.get(operationName).getInputShape(), @@ -149,12 +160,8 @@ private void validateConfiguredBuiltInValues( for (Map.Entry builtInParasWithNonDefaultValue : builtInParamsWithNonDefaultValues .entrySet()) { String builtInName = builtInParasWithNonDefaultValue.getKey().getBuiltIn().get(); - // Emit if either the built-in with a non-matching value isn't - // specified or the value set for it doesn't match. - if (!testOperationInput.getBuiltInParams().containsMember(builtInName) - || !testOperationInput.getBuiltInParams() - .expectMember(builtInName) - .equals(builtInParasWithNonDefaultValue.getValue())) { + // Emit if the built-in with a non-matching value isn't specified. + if (!testOperationInput.getBuiltInParams().containsMember(builtInName)) { events.add(error(serviceShape, testOperationInput, String.format("Test case does not supply the `%s` value for the `%s` parameter's " @@ -186,6 +193,38 @@ private void validateBuiltInsWithoutDefaultsHaveValues( } } + private void validateBuiltInsUseInconsistentValues( + ServiceShape serviceShape, + Map builtInParameters, + EndpointTestCase testCase, + EndpointTestOperationInput testOperationInput, + List events + ) { + for (Entry testCaseParam : testCase.getParams().getMembers().entrySet()) { + if (!builtInParameters.containsKey(testCaseParam.getKey().getValue())) { + continue; + } + + ObjectNode operationInputBuiltInParams = testOperationInput.getBuiltInParams(); + String builtInName = builtInParameters.get(testCaseParam.getKey().getValue()); + if (!operationInputBuiltInParams.containsMember(builtInName)) { + continue; + } + + Optional paramValue = operationInputBuiltInParams.getMember(builtInName); + if (!paramValue.isPresent()) { + continue; + } + + if (!paramValue.get().equals(testCaseParam.getValue())) { + events.add(error( + serviceShape, + "Inconsistent testcase parameters: " + testCaseParam.getValue() + + " and " + paramValue.get())); + } + } + } + private void validateOperationInput( Model model, ServiceShape serviceShape, diff --git a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/validators/RuleSetParameterValidator.java b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/validators/RuleSetParameterValidator.java index 55ae194170f..68d1ac95a27 100644 --- a/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/validators/RuleSetParameterValidator.java +++ b/smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/validators/RuleSetParameterValidator.java @@ -271,7 +271,7 @@ private void validateTestsParameters( // All required params from a ruleset must be present in all test cases. if (parameter.isRequired() && !parameter.getDefault().isPresent() - && (!testSuiteHasParam || testSuiteParams.get(name).size() != trait.getTestCases().size())) { + && (!testSuiteHasParam || testSuiteParams.get(name).size() < trait.getTestCases().size())) { errors.add(parameterError(serviceShape, parameter, "TestCase.RequiredMissing", diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/traits/errorfiles/inconsistent-builtin-params.errors b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/traits/errorfiles/inconsistent-builtin-params.errors new file mode 100644 index 00000000000..0e205f04b58 --- /dev/null +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/traits/errorfiles/inconsistent-builtin-params.errors @@ -0,0 +1,3 @@ +[WARNING] example#FizzBuzz: This shape applies a trait that is unstable: smithy.rules#endpointRuleSet | UnstableTrait.smithy.rules#endpointRuleSet +[WARNING] example#FizzBuzz: This shape applies a trait that is unstable: smithy.rules#endpointTests | UnstableTrait.smithy.rules#endpointTests +[ERROR] example#FizzBuzz: Inconsistent testcase parameters: https://another.example.com and https://custom.example.com | EndpointTestsTrait diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/traits/errorfiles/inconsistent-builtin-params.smithy b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/traits/errorfiles/inconsistent-builtin-params.smithy new file mode 100644 index 00000000000..d1cb85f6e9c --- /dev/null +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/traits/errorfiles/inconsistent-builtin-params.smithy @@ -0,0 +1,82 @@ +$version: "1.0" + +namespace example + +use smithy.rules#endpointRuleSet +use smithy.rules#endpointTests + +@endpointRuleSet({ + "version": "1.3", + parameters: { + endpoint: { + type: "string" + builtIn: "SDK::Endpoint" + documentation: "docs" + } + }, + "rules": [ + { + "conditions": [], + "documentation": "base rule", + "endpoint": { + "url": "https://fizzbuzz.amazonaws.com", + "headers": {} + }, + "type": "endpoint" + } + ] +}) +@endpointTests( + version: "1.3", + testCases: [ + { + "documentation": "Inconsistent", + "params": { + endpoint: "https://another.example.com" + }, + "expect": { + "endpoint": { + "url": "https://fizzbuzz.amazonaws.com" + } + }, + "operationInputs": [ + { + "operationName": "ListShards", + "builtInParams": { + "SDK::Endpoint": "https://custom.example.com", + } + } + ] + } + { + "documentation": "Consistent", + "params": { + endpoint: "https://another.example.com" + }, + "expect": { + "endpoint": { + "url": "https://fizzbuzz.amazonaws.com" + } + }, + "operationInputs": [ + { + "operationName": "ListShards", + "builtInParams": { + "SDK::Endpoint": "https://another.example.com", + } + } + ] + } + ] +) +service FizzBuzz { + operations: [ + ListShards + ] +} + +operation ListShards { + input: Struct +} + +structure Struct {} diff --git a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/traits/errorfiles/non-default-builtin-values.errors b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/traits/errorfiles/non-default-builtin-values.errors index 9428e400a6b..88eea9cdeb7 100644 --- a/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/traits/errorfiles/non-default-builtin-values.errors +++ b/smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/traits/errorfiles/non-default-builtin-values.errors @@ -1,4 +1,4 @@ [WARNING] smithy.example#ExampleService: This shape applies a trait that is unstable: smithy.rules#endpointRuleSet | UnstableTrait [WARNING] smithy.example#ExampleService: This shape applies a trait that is unstable: smithy.rules#endpointTests | UnstableTrait -[ERROR] smithy.example#ExampleService: Test case does not supply the `"https://another.example.com"` value for the `endpoint` parameter's `SDK::Endpoint` built-in. | EndpointTestsTrait +[ERROR] smithy.example#ExampleService: Inconsistent testcase parameters: https://another.example.com and https://custom.example.com | EndpointTestsTrait [ERROR] smithy.example#ExampleService: Test case does not supply the `"https://some.example.com"` value for the `endpoint` parameter's `SDK::Endpoint` built-in. | EndpointTestsTrait