Conversation
shkr7
force-pushed
the
fix-env-it
branch
2 times, most recently
from
8684eec to
7f50e0b
Compare
| @When("I call the batch summary API with expected status of {int} with total {int} txns") | ||
| public void callBatchSummaryAPI(int expectedStatus, int totalTxns) { | ||
| await().atMost(awaitMost, SECONDS).pollDelay(pollDelay, SECONDS).pollInterval(pollInterval, SECONDS).untilAsserted(() -> { | ||
| await().atMost(120, SECONDS).pollDelay(pollDelay, SECONDS).pollInterval(pollInterval, SECONDS).untilAsserted(() -> { |
|
|
||
| awaitly: | ||
| maxWaitTime: 25 | ||
| maxWaitTime: 120 |
There was a problem hiding this comment.
I am accepting this as the awaitly time out is a max wait time and this doesn't affect tests that complete early.
fynmanoj
left a comment
fynmanoj
left a comment
There was a problem hiding this comment.
Sorry for approving it earlier. some of these issues did not catch my eye.
please review Awaitly usage.
| @Then("I check for result file URL validity") | ||
| public void iCheckForResultFileURLValidity() { | ||
| assertThat(isValidURL(scenarioScopeState.batchAndSubBatchSummaryResponse.getFile())).isTrue(); | ||
| await().atMost(awaitMost, SECONDS).pollDelay(pollDelay, SECONDS).pollInterval(pollInterval, SECONDS).untilAsserted(() -> { |
There was a problem hiding this comment.
Awaitly may not be needed here as, this block is not making any external call.
Are we waiting for any parallel process to fill data into this DTO? i dont think we have such a n architecture
| logger.error("Error parsing the batch summary response", e); | ||
| } | ||
| assertThat(scenarioScopeState.batchAndSubBatchSummaryResponse).isNotNull(); | ||
| await().atMost(awaitMost, SECONDS).pollDelay(pollDelay, SECONDS).pollInterval(pollInterval, SECONDS).untilAsserted(() -> { |
There was a problem hiding this comment.
here again, Awaitly is not required here, there are no external request in this block
| iCanCreateAnRedeemVoucherRequestDTOForVoucherRedemption(); | ||
| iCallTheRedeemVoucherAPIWithExpectedStatusOf(200); | ||
| assertUnsuccessfulRedemption(); | ||
| await().atMost(awaitMost, SECONDS).pollDelay(pollDelay, SECONDS).pollInterval(pollInterval, SECONDS).untilAsserted(() -> { |
There was a problem hiding this comment.
here as well, Await doesn't make much sense as we are making a post request. repeating this step has no value.
2 participants
PHEE-682 Ephemeral Flaky TCs Fix
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
Followed the PR title naming convention mentioned above.
Acknowledge that we will not merge PRs that are not passing the checks ("green") - it is your (author's) responsibility to get a proposed PR to pass all the checks, not primarily the project's maintainers.
The PR title should include a JIRA ticket
Design-related bullet points or design document links related to this PR added in the description above.
Updated corresponding Postman Collection or API documentation for the changes in this PR.
Create/update unit or integration tests for verifying the changes made.
Add required Swagger annotation and update API documentation with details of any API changes if applicable
Followed the naming conventions as given in https://docs.google.com/document/d/1Q4vaMSzrTxxh9TS0RILuNkSkYCxotuYk1Xe0CMIkkCU/edit?usp=sharing
Followed coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
FYI our guidelines for code reviews same as https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.