You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "rpuch (via GitHub)" <gi...@apache.org> on 2023/05/05 14:32:00 UTC

[GitHub] [ignite-3] rpuch opened a new pull request, #2034: IGNITE-19432 Tests in PartitionReplicaListenerTest are not independent

rpuch opened a new pull request, #2034:
URL: https://github.com/apache/ignite-3/pull/2034

   https://issues.apache.org/jira/browse/IGNITE-19432


-- 
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@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] tkalkirill merged pull request #2034: IGNITE-19432 Tests in PartitionReplicaListenerTest are not independent

Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill merged PR #2034:
URL: https://github.com/apache/ignite-3/pull/2034


-- 
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@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #2034: IGNITE-19432 Tests in PartitionReplicaListenerTest are not independent

Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #2034:
URL: https://github.com/apache/ignite-3/pull/2034#discussion_r1187041683


##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/replication/PartitionReplicaListenerTest.java:
##########
@@ -125,39 +125,45 @@
 import org.apache.ignite.network.NetworkAddress;
 import org.apache.ignite.network.TopologyService;
 import org.apache.ignite.tx.TransactionException;
-import org.junit.jupiter.api.BeforeAll;
+import org.jetbrains.annotations.Nullable;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.junitpioneer.jupiter.cartesian.CartesianTest;
+import org.junitpioneer.jupiter.cartesian.CartesianTest.Values;
 import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
 
-/** There are tests for partition replica listener. */
+/** Tests for partition replica listener. */
 @ExtendWith(ConfigurationExtension.class)
+@ExtendWith(MockitoExtension.class)
 public class PartitionReplicaListenerTest extends IgniteAbstractTest {
     /** Partition id. */
     private static final int partId = 0;
 
     /** Table id. */
-    private static final UUID tblId = UUID.randomUUID();
+    private final UUID tblId = UUID.randomUUID();
 
-    private static final Map<UUID, Set<RowId>> pendingRows = new ConcurrentHashMap<>();
+    private final Map<UUID, Set<RowId>> pendingRows = new ConcurrentHashMap<>();
 
     /** The storage stores partition data. */
-    private static final TestMvPartitionStorage testMvPartitionStorage = new TestMvPartitionStorage(partId);
+    private final TestMvPartitionStorage testMvPartitionStorage = new TestMvPartitionStorage(partId);
 
-    private static final LockManager lockManager = new HeapLockManager();
+    private final LockManager lockManager = new HeapLockManager();
 
-    private static final Function<PartitionCommand, CompletableFuture<?>> DEFAULT_MOCK_RAFT_FUTURE_CLOSURE = cmd -> {
+    private final Function<PartitionCommand, CompletableFuture<?>> defaultMockRaftFutureClosure = cmd -> {
         if (cmd instanceof TxCleanupCommand) {
             Set<RowId> rows = pendingRows.remove(cmd.txId());
 
+            HybridTimestamp commitTimestamp = Objects.requireNonNull(((TxCleanupCommand) cmd).commitTimestamp());

Review Comment:
   Personally, I prefer `assertNotNull` instead of `requireNonNull` in tests, at your discretion.



##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/replication/PartitionReplicaListenerTest.java:
##########
@@ -1217,53 +1215,57 @@ private void cleanup(UUID txId) {
         txState = TxState.COMMITED;
     }
 
-    private static BinaryTuplePrefix toIndexBound(int val) {
+    private BinaryTuplePrefix toIndexBound(int val) {
         ByteBuffer tuple = new BinaryTuplePrefixBuilder(1, 1).appendInt(val).build();
 
         return new BinaryTuplePrefix(sortedIndexBinarySchema, tuple);
     }
 
-    private static BinaryTuple toIndexKey(int val) {
+    private BinaryTuple toIndexKey(int val) {
         ByteBuffer tuple = new BinaryTupleBuilder(1, true).appendInt(val).build();
 
         return new BinaryTuple(sortedIndexBinarySchema, tuple);
     }
 
-    private static BinaryRow nextBinaryKey() {
+    private BinaryRow nextBinaryKey() {
         try {
-            int nextInt = (int) System.nanoTime();
+            int nextInt = monotonicInt();
 
             return kvMarshaller.marshal(new TestKey(nextInt, "key " + nextInt));

Review Comment:
   ```suggestion
               return kvMarshaller.marshal(new TestKey(nextInt, "key " + monotonicInt()));
   ```



##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/replication/PartitionReplicaListenerTest.java:
##########
@@ -1217,53 +1215,57 @@ private void cleanup(UUID txId) {
         txState = TxState.COMMITED;
     }
 
-    private static BinaryTuplePrefix toIndexBound(int val) {
+    private BinaryTuplePrefix toIndexBound(int val) {
         ByteBuffer tuple = new BinaryTuplePrefixBuilder(1, 1).appendInt(val).build();
 
         return new BinaryTuplePrefix(sortedIndexBinarySchema, tuple);
     }
 
-    private static BinaryTuple toIndexKey(int val) {
+    private BinaryTuple toIndexKey(int val) {
         ByteBuffer tuple = new BinaryTupleBuilder(1, true).appendInt(val).build();
 
         return new BinaryTuple(sortedIndexBinarySchema, tuple);
     }
 
-    private static BinaryRow nextBinaryKey() {
+    private BinaryRow nextBinaryKey() {
         try {
-            int nextInt = (int) System.nanoTime();
+            int nextInt = monotonicInt();
 
             return kvMarshaller.marshal(new TestKey(nextInt, "key " + nextInt));
         } catch (MarshallerException e) {
             throw new IgniteException(e);
         }
     }
 
-    protected static BinaryRow binaryRow(int i) {
+    private static int monotonicInt() {
+        return nextMonotonicInt.getAndIncrement();
+    }
+
+    protected BinaryRow binaryRow(int i) {
         try {
             return kvMarshaller.marshal(new TestKey(i, "k" + i), new TestValue(i, "v" + i));
         } catch (MarshallerException e) {
-            throw new IgniteException(e);
+            throw new RuntimeException(e);
         }
     }
 
-    private static BinaryRow binaryRow(TestKey key, TestValue value) {
+    private BinaryRow binaryRow(TestKey key, TestValue value) {
         try {
             return kvMarshaller.marshal(key, value);
         } catch (MarshallerException e) {
-            throw new IgniteException(e);
+            throw new RuntimeException(e);
         }
     }
 
-    private static TestKey key(BinaryRow binaryRow) {
+    private TestKey key(BinaryRow binaryRow) {
         try {
             return kvMarshaller.unmarshalKey(new Row(schemaDescriptor, binaryRow));
         } catch (MarshallerException e) {
-            throw new IgniteException(e);
+            throw new RuntimeException(e);

Review Comment:
   ```suggestion
               throw new AssertionError(e);
   ```



##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/replication/PartitionReplicaListenerTest.java:
##########
@@ -1217,53 +1215,57 @@ private void cleanup(UUID txId) {
         txState = TxState.COMMITED;
     }
 
-    private static BinaryTuplePrefix toIndexBound(int val) {
+    private BinaryTuplePrefix toIndexBound(int val) {
         ByteBuffer tuple = new BinaryTuplePrefixBuilder(1, 1).appendInt(val).build();
 
         return new BinaryTuplePrefix(sortedIndexBinarySchema, tuple);
     }
 
-    private static BinaryTuple toIndexKey(int val) {
+    private BinaryTuple toIndexKey(int val) {
         ByteBuffer tuple = new BinaryTupleBuilder(1, true).appendInt(val).build();
 
         return new BinaryTuple(sortedIndexBinarySchema, tuple);
     }
 
-    private static BinaryRow nextBinaryKey() {
+    private BinaryRow nextBinaryKey() {
         try {
-            int nextInt = (int) System.nanoTime();
+            int nextInt = monotonicInt();
 
             return kvMarshaller.marshal(new TestKey(nextInt, "key " + nextInt));
         } catch (MarshallerException e) {
             throw new IgniteException(e);
         }
     }
 
-    protected static BinaryRow binaryRow(int i) {
+    private static int monotonicInt() {
+        return nextMonotonicInt.getAndIncrement();
+    }
+
+    protected BinaryRow binaryRow(int i) {
         try {
             return kvMarshaller.marshal(new TestKey(i, "k" + i), new TestValue(i, "v" + i));
         } catch (MarshallerException e) {
-            throw new IgniteException(e);
+            throw new RuntimeException(e);

Review Comment:
   ```suggestion
               throw new AssertionError(e);
   ```



##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/replication/PartitionReplicaListenerTest.java:
##########
@@ -1217,53 +1215,57 @@ private void cleanup(UUID txId) {
         txState = TxState.COMMITED;
     }
 
-    private static BinaryTuplePrefix toIndexBound(int val) {
+    private BinaryTuplePrefix toIndexBound(int val) {
         ByteBuffer tuple = new BinaryTuplePrefixBuilder(1, 1).appendInt(val).build();
 
         return new BinaryTuplePrefix(sortedIndexBinarySchema, tuple);
     }
 
-    private static BinaryTuple toIndexKey(int val) {
+    private BinaryTuple toIndexKey(int val) {
         ByteBuffer tuple = new BinaryTupleBuilder(1, true).appendInt(val).build();
 
         return new BinaryTuple(sortedIndexBinarySchema, tuple);
     }
 
-    private static BinaryRow nextBinaryKey() {
+    private BinaryRow nextBinaryKey() {
         try {
-            int nextInt = (int) System.nanoTime();
+            int nextInt = monotonicInt();
 
             return kvMarshaller.marshal(new TestKey(nextInt, "key " + nextInt));
         } catch (MarshallerException e) {
             throw new IgniteException(e);
         }
     }
 
-    protected static BinaryRow binaryRow(int i) {
+    private static int monotonicInt() {
+        return nextMonotonicInt.getAndIncrement();
+    }
+
+    protected BinaryRow binaryRow(int i) {
         try {
             return kvMarshaller.marshal(new TestKey(i, "k" + i), new TestValue(i, "v" + i));
         } catch (MarshallerException e) {
-            throw new IgniteException(e);
+            throw new RuntimeException(e);
         }
     }
 
-    private static BinaryRow binaryRow(TestKey key, TestValue value) {
+    private BinaryRow binaryRow(TestKey key, TestValue value) {
         try {
             return kvMarshaller.marshal(key, value);
         } catch (MarshallerException e) {
-            throw new IgniteException(e);
+            throw new RuntimeException(e);

Review Comment:
   ```suggestion
               throw new AssertionError(e);
   ```



-- 
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@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org