Skip to content

Conversation

@gesterzhou
Copy link
Contributor

…arlier than provider

For all changes:

  • Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

  • Has your PR been rebased against the latest commit within the target branch (typically develop)?

  • Is your initial contribution a single, squashed commit?

  • Does gradlew build run cleanly?

  • Have you written or updated unit tests to verify your changes?

  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?

count = getDeltaGIICount(P);
assertEquals(0, count);
final int countConst = getDeltaGIICount(P);
GeodeAwaitility.await().untilAsserted(() -> assertEquals(0, countConst));
Copy link
Contributor

Choose a reason for hiding this comment

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

Once countConst is assigned by getDeltaGIICount(P), its value will not change. If its value is not 0, no matter how long you wait, the assert in the next line still fails.
I was thinking about GeodeAwaitility.await().untilAsserted(() -> assertEquals(0, getDeltaGIICount(P)));.
Please correct me if I am wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to this comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. I fixed.

@gesterzhou gesterzhou requested a review from jchen21 October 13, 2021 18:16
Copy link
Contributor

@kirklund kirklund left a comment

Choose a reason for hiding this comment

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

Approved! You might want to consider changing the assertion to use org.assertj.core.api.Assertions.assertThat but I think it's ready to merge now.

@gesterzhou gesterzhou merged commit 00f92b6 into develop Oct 13, 2021
@gesterzhou gesterzhou deleted the feature/GEODE-9677 branch October 13, 2021 21:05
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.

4 participants