Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature(Waiters): Boolean Value support for Error Matchers for Waiters #5084

Merged
merged 3 commits into from
Apr 15, 2024

Conversation

joviegas
Copy link
Contributor

@joviegas joviegas commented Apr 9, 2024

Added support for Waiters specifically for Matchers with Error to accept true/false value not as string but as boolean values such that True value is to match on any error code, or boolean false to test if no errors were encountered as per the SDK Waiter specs.

Motivation and Context

  • The Spec statement is as below
error: A string representing the parsed AWS error code when looking for a specific error code, or boolean true to match on any error code, or boolean false to test if no errors were encountered.
  • Currently BaseWaiterClassSpec only handles for String type values for "true/false"
  • But we need to treat True as Any error expected or False as No Error expected.

Modifications

  • Modified BaseWaiterClassSpec to do error code check if its exist or not when Expected value is True
  • When Expected value is false we just need to make sure any response is received and no exception is true.

Testing

Cases

Next state Expected Any Error ? API return Values Waiter Behavior () Test Case Name
failure True API Success The waiter has exceeded the max retry attempts: 3 errorMatcherWithExpectedTrueFails_withAPISuccess
failure True API Error Waiter fails immediately with message "transitioned the waiter to failure state errorMatcherWithExpectedTrueFails_withAPIError
failure False API Success A waiter acceptor was matched and transitioned the waiter to failure state errorMatcherWithExpectedFalseFails
failure False API Error An exception was thrown and did not match any waiter acceptors errorMatcherWithExpectedFalseAndStateAsFailure_whenAPIErrors
Success False API Success Waiter success successForBooleanWaiters_shouldPassIfErrorIsFalse. (If No Erro)
Success False API Error "An exception was thrown and did not match any waiter acceptors" errorMatcherWithExpectedFalseSuccess_APIFailure
Success True API Success "The waiter has exceeded the max retry attempts:" errorMatcherWithExpectedTrueAndStateAsSuccess_ApiSuccess
Success True API Error Waiter success errorMatcherWithExpectedTrueAndStateAsSuccess_ApiError
retry False API Success The waiter has exceeded the max retry attempts: 3 errorMatcherWithExpectedFalseRetries_exhaustAllRetries
retry False API Success followed by failure Waiter success errorMatcherWithExpectedFalseRetries_passesWhenApiReturnErrors

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

License

  • I confirm that this pull request can be released under the Apache 2 license

@joviegas joviegas requested a review from a team as a code owner April 9, 2024 01:17
…s with Error to accept true/false value not as string but as boolean values such that True value is to match on any error code, or boolean false to test if no errors were encountered as per the SDK Waiter specs.
@joviegas joviegas force-pushed the joviegas/waiters_boolean branch from f4aa5f9 to b494abf Compare April 9, 2024 17:49
waiter.waitUntilErrorMatcherWithExpectedFalseRetries(AllTypesRequest.builder().build());
assertThat(waiterResponse.attemptsExecuted()).isEqualTo(3);
// Empty because the waiter specifically waits for Error case.
assertThat(waiterResponse.matched().response()).isEmpty();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the response empty instead of containing the EmptyModeledException thrown?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is existing case coming from second part of acceptor as below where we say if matcher is error and if we get exception as EmptyModeledException then transistion to success state , since we donot have any valid response from response from server we retrurn empty response

    "ErrorMatcherWithExpectedFalseRetries": {
      "delay": 1,
      "operation": "AllTypes",
      "maxAttempts": 40,
      "acceptors": [
        {
          "matcher" : "error",
          "state" : "retry",
          "expected" : false
        },
        {
          "matcher": "error",
          "expected": "EmptyModeledException",
          "state": "success"
        }
      ]
    
    ```

"type": "feature",
"category": "AWS SDK for Java v2",
"contributor": "",
"description": "Added support for Waiters specifically for Matchers with Error to accept true/false value not as string but as boolean values such that True value is to match on any error code, or boolean false to test if no errors were encountered as per the SDK Waiter specs."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit : whitespace between "is to"

}

/**
* Case with the model just defined that it should Fail when there are no Errors But waitor does not tell what to do if there
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: waitor typo

codeBlock.add(trueForAllResponse());
} else {
codeBlock.add("OnExceptionAcceptor(");
codeBlock.add("error -> errorCode(error) " + (expectedBoolean ? "!=" : "==") + " null");
Copy link
Contributor

@zoewangg zoewangg Apr 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exepectedBoolean is always true in this else block, right?
codeBlock.add("error -> errorCode(error) != null");

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap , this is redundant check , I removed it now.

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@joviegas joviegas merged commit 2a07425 into master Apr 15, 2024
15 of 17 checks passed
@joviegas joviegas deleted the joviegas/waiters_boolean branch May 15, 2024 20:31
akidambisrinivasan pushed a commit to akidambisrinivasan/aws-sdk-java-v2 that referenced this pull request Jun 28, 2024
…rs (aws#5084)

* feature(Waiters): Added support for Waiters specifically for Matchers with Error to accept true/false value not as string but as boolean values such that True value is  to match on any error code, or boolean false to test if no errors were encountered as per the SDK Waiter specs.

* addressed review comments v1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants