Added new ERROR event to EndpointTestsTraitValidator when builtin params are inconsistent#2955
Added new ERROR event to EndpointTestsTraitValidator when builtin params are inconsistent#2955sugmanue merged 7 commits intosmithy-lang:mainfrom
Conversation
…sistent params and builtInParams within a testcase
|
This pull request does not contain a staged changelog entry. To create one, use the Make sure that the description is appropriate for a changelog entry and that the proper feature type is used. See |
| // 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())) { |
There was a problem hiding this comment.
this is only a partial/bandaid fix. there is still a bug in extractTestSuiteParameters that double counts and causes testSuiteParams to be larger than it should be.
There was a problem hiding this comment.
Do we have a plan to get the root cause fixed?
There was a problem hiding this comment.
I took a look and it seemed fairly complicated at least to me. I didn't want to accidentally create a regression, so I did this minimal fix for now
SyntaxError: invalid syntax when running ./.changes/new-change --pull-requests "#2955" --type feature --description "Added EndpointTestCasesParamsValidator" |
...java/software/amazon/smithy/rulesengine/aws/validators/EndpointTestCasesParamsValidator.java
Outdated
Show resolved
Hide resolved
...java/software/amazon/smithy/rulesengine/aws/validators/EndpointTestCasesParamsValidator.java
Outdated
Show resolved
Hide resolved
...java/software/amazon/smithy/rulesengine/aws/validators/EndpointTestCasesParamsValidator.java
Outdated
Show resolved
Hide resolved
...java/software/amazon/smithy/rulesengine/aws/validators/EndpointTestCasesParamsValidator.java
Outdated
Show resolved
Hide resolved
| // 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())) { |
There was a problem hiding this comment.
Do we have a plan to get the root cause fixed?
Co-authored-by: Manuel Sugawara <sugmanue@amazon.com>
|
Ok, I look a bit more into this, the problem you’re to avoid is not AWS-specific, so I think it would be more appropriate to just do it in the class that validates the tests, EndpointTestsTraitValidator, there we already do similar validations but not this case, also there is already some of the information you need already computed. |
rchache
changed the title
...java/software/amazon/smithy/rulesengine/aws/validators/EndpointTestCasesParamsValidator.java
Outdated
Show resolved
Hide resolved
...src/main/java/software/amazon/smithy/rulesengine/validators/EndpointTestsTraitValidator.java
Outdated
Show resolved
Hide resolved
...src/main/java/software/amazon/smithy/rulesengine/validators/EndpointTestsTraitValidator.java
Outdated
Show resolved
Hide resolved
|
This pull request does not contain a staged changelog entry. To create one, use the Make sure that the description is appropriate for a changelog entry and that the proper feature type is used. See |
2 participants
Background
Testing
Links
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.