You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2022/04/19 00:05:41 UTC
[GitHub] [geode] DonalEvans commented on a diff in pull request #7602: GEODE-10229: GII image should fill disk region RVV's exceptions.
DonalEvans commented on code in PR #7602:
URL: https://github.com/apache/geode/pull/7602#discussion_r852492638
##########
geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderJUnitTest.java:
##########
@@ -53,6 +53,58 @@ protected InternalDistributedMember member() {
return member;
}
+ @Test
+ public void fillSpecialExceptionForRVHWithBitSet() {
+ fillSpecialExceptionForRVH(true);
+ }
+
+ @Test
+ public void fillSpecialExceptionForRVHWithoutBitSet() {
+ fillSpecialExceptionForRVH(false);
+ }
+
+ private void fillSpecialExceptionForRVH(boolean useBitSet) {
+ RegionVersionHolder vh1 = null;
+ RegionVersionHolder vh2 = null;
+ if (useBitSet) {
+ vh1 = new RegionVersionHolder(member);
+ vh2 = new RegionVersionHolder(member);
+ } else {
+ vh1 = new RegionVersionHolder(0);
+ vh2 = new RegionVersionHolder(0);
Review Comment:
Warnings here can be fixed by using:
```
RegionVersionHolder<InternalDistributedMember> vh1;
RegionVersionHolder<InternalDistributedMember> vh2;
if (useBitSet) {
vh1 = new RegionVersionHolder<>(member);
vh2 = new RegionVersionHolder<>(member);
} else {
vh1 = new RegionVersionHolder<>(0);
vh2 = new RegionVersionHolder<>(0);
}
```
##########
geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderJUnitTest.java:
##########
@@ -53,6 +53,58 @@ protected InternalDistributedMember member() {
return member;
}
+ @Test
+ public void fillSpecialExceptionForRVHWithBitSet() {
+ fillSpecialExceptionForRVH(true);
+ }
+
+ @Test
+ public void fillSpecialExceptionForRVHWithoutBitSet() {
+ fillSpecialExceptionForRVH(false);
+ }
+
+ private void fillSpecialExceptionForRVH(boolean useBitSet) {
+ RegionVersionHolder vh1 = null;
+ RegionVersionHolder vh2 = null;
+ if (useBitSet) {
+ vh1 = new RegionVersionHolder(member);
+ vh2 = new RegionVersionHolder(member);
+ } else {
+ vh1 = new RegionVersionHolder(0);
+ vh2 = new RegionVersionHolder(0);
+ }
+ for (int i = 1; i <= 3; i++) {
+ vh1.recordVersion(i);
+ }
+
+ for (int i = 1; i <= 5; i++) {
+ vh2.recordVersion(i);
+ }
+
+ // create special exception 5(n=6,p=3)
+ vh2.initializeFrom(vh1);
+ System.out.println("vh2=" + vh2);
+ List<RVVException> exceptionList = vh2.getExceptionForTest();
+ assertEquals(1, exceptionList.size());
+ RVVException exception = exceptionList.get(0);
+ assertEquals(3, exception.previousVersion);
+ assertEquals(6, exception.nextVersion);
Review Comment:
These assertions should be using AssertJ rather than JUnit assertions:
```
assertThat(exceptionList).hasSize(1);
RVVException exception = exceptionList.get(0);
assertThat(exception.previousVersion).isEqualTo(3);
assertThat(exception.nextVersion).isEqualTo(6);
```
##########
geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderJUnitTest.java:
##########
@@ -53,6 +53,58 @@ protected InternalDistributedMember member() {
return member;
}
+ @Test
+ public void fillSpecialExceptionForRVHWithBitSet() {
+ fillSpecialExceptionForRVH(true);
+ }
+
+ @Test
+ public void fillSpecialExceptionForRVHWithoutBitSet() {
+ fillSpecialExceptionForRVH(false);
+ }
+
+ private void fillSpecialExceptionForRVH(boolean useBitSet) {
+ RegionVersionHolder vh1 = null;
+ RegionVersionHolder vh2 = null;
+ if (useBitSet) {
+ vh1 = new RegionVersionHolder(member);
+ vh2 = new RegionVersionHolder(member);
+ } else {
+ vh1 = new RegionVersionHolder(0);
+ vh2 = new RegionVersionHolder(0);
+ }
+ for (int i = 1; i <= 3; i++) {
+ vh1.recordVersion(i);
+ }
+
+ for (int i = 1; i <= 5; i++) {
+ vh2.recordVersion(i);
+ }
+
+ // create special exception 5(n=6,p=3)
+ vh2.initializeFrom(vh1);
+ System.out.println("vh2=" + vh2);
+ List<RVVException> exceptionList = vh2.getExceptionForTest();
+ assertEquals(1, exceptionList.size());
+ RVVException exception = exceptionList.get(0);
+ assertEquals(3, exception.previousVersion);
+ assertEquals(6, exception.nextVersion);
+ System.out.println("vh2=" + vh2);
+
+ vh2.recordVersion(5);
+ exceptionList = vh2.getExceptionForTest();
+ assertEquals(1, exceptionList.size());
+ exception = exceptionList.get(0);
+ assertEquals(3, exception.previousVersion);
+ assertEquals(5, exception.nextVersion);
+ System.out.println("vh2=" + vh2);
+
+ vh2.recordVersion(4);
+ exceptionList = vh2.getExceptionForTest();
+ assertEquals(0, exceptionList.size());
Review Comment:
This assertion should be using AssertJ:
```
assertThat(exceptionList).isEmpty();
```
##########
geode-core/src/test/java/org/apache/geode/internal/cache/versions/RegionVersionHolderJUnitTest.java:
##########
@@ -53,6 +53,58 @@ protected InternalDistributedMember member() {
return member;
}
+ @Test
+ public void fillSpecialExceptionForRVHWithBitSet() {
+ fillSpecialExceptionForRVH(true);
+ }
+
+ @Test
+ public void fillSpecialExceptionForRVHWithoutBitSet() {
+ fillSpecialExceptionForRVH(false);
+ }
+
+ private void fillSpecialExceptionForRVH(boolean useBitSet) {
+ RegionVersionHolder vh1 = null;
+ RegionVersionHolder vh2 = null;
+ if (useBitSet) {
+ vh1 = new RegionVersionHolder(member);
+ vh2 = new RegionVersionHolder(member);
+ } else {
+ vh1 = new RegionVersionHolder(0);
+ vh2 = new RegionVersionHolder(0);
+ }
+ for (int i = 1; i <= 3; i++) {
+ vh1.recordVersion(i);
+ }
+
+ for (int i = 1; i <= 5; i++) {
+ vh2.recordVersion(i);
+ }
+
+ // create special exception 5(n=6,p=3)
+ vh2.initializeFrom(vh1);
+ System.out.println("vh2=" + vh2);
+ List<RVVException> exceptionList = vh2.getExceptionForTest();
+ assertEquals(1, exceptionList.size());
+ RVVException exception = exceptionList.get(0);
+ assertEquals(3, exception.previousVersion);
+ assertEquals(6, exception.nextVersion);
+ System.out.println("vh2=" + vh2);
+
+ vh2.recordVersion(5);
+ exceptionList = vh2.getExceptionForTest();
+ assertEquals(1, exceptionList.size());
+ exception = exceptionList.get(0);
+ assertEquals(3, exception.previousVersion);
+ assertEquals(5, exception.nextVersion);
Review Comment:
These assertions should be using AssertJ rather than JUnit assertions:
```
assertThat(exceptionList).hasSize(1);
RVVException exception = exceptionList.get(0);
assertThat(exception.previousVersion).isEqualTo(3);
assertThat(exception.nextVersion).isEqualTo(5);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org