[GitHub] [cassandra] blerer commented on a change in pull request #470: CASSANDRA-15630 fix testSerializeError

[GitBox <gi...@apache.org>]

blerer commented on a change in pull request #470: CASSANDRA-15630 fix testSerializeError
URL: https://github.com/apache/cassandra/pull/470#discussion_r398494290
 
 

 ##########
 File path: test/unit/org/apache/cassandra/net/ConnectionUtils.java
 ##########
 @@ -197,45 +197,42 @@ public InboundCountChecker error(long count, long bytes)
 
         public void check()
         {
-            doCheck(Assert::assertEquals);
+            doCheck((message, expected, actual) -> spinAssertEquals(message, expected, actual, 5, TimeUnit.SECONDS));
         }
 
         public void check(FailCheck failCheck)
         {
-            doCheck((message, expect, actual) -> { if (expect != actual) failCheck.accept(message, expect, actual); });
+            doCheck((message, expect, actual) -> { if (!Objects.equals(expect, actual.get())) failCheck.accept(message, expect, actual); });
         }
 
         private void doCheck(FailCheck testAndFailCheck)
         {
             if (checkReceived)
             {
-                testAndFailCheck.accept("received count values don't match", received, connection.receivedCount());
-                testAndFailCheck.accept("received bytes values don't match", receivedBytes, connection.receivedBytes());
+                testAndFailCheck.accept("received count values don't match", received, connection::receivedCount);
+                testAndFailCheck.accept("received bytes values don't match", receivedBytes, connection::receivedBytes);
             }
             if (checkProcessed)
             {
-                testAndFailCheck.accept("processed count values don't match", processed, connection.processedCount());
-                testAndFailCheck.accept("processed bytes values don't match", processedBytes, connection.processedBytes());
+                testAndFailCheck.accept("processed count values don't match", processed, connection::processedCount);
+                testAndFailCheck.accept("processed bytes values don't match", processedBytes, connection::processedBytes);
             }
             if (checkExpired)
             {
-                testAndFailCheck.accept("expired count values don't match", expired, connection.expiredCount());
-                testAndFailCheck.accept("expired bytes values don't match", expiredBytes, connection.expiredBytes());
+                testAndFailCheck.accept("expired count values don't match", expired, connection::expiredCount);
+                testAndFailCheck.accept("expired bytes values don't match", expiredBytes, connection::expiredBytes);
             }
             if (checkError)
             {
-                testAndFailCheck.accept("error count values don't match", error, connection.errorCount());
-                testAndFailCheck.accept("error bytes values don't match", errorBytes, connection.errorBytes());
+                testAndFailCheck.accept("error count values don't match", error, connection::errorCount);
+                testAndFailCheck.accept("error bytes values don't match", errorBytes, connection::errorBytes);
             }
             if (checkScheduled)
             {
                 // scheduled cannot relied upon to not race with completion of the task,
-                // so if it is currently above the value we expect, sleep for a bit
-                if (scheduled < connection.scheduledCount())
-                    for (int i = 0; i < 10 && scheduled < connection.scheduledCount() ; ++i)
-                        Uninterruptibles.sleepUninterruptibly(1L, TimeUnit.MILLISECONDS);
-                testAndFailCheck.accept("scheduled count values don't match", scheduled, connection.scheduledCount());
-                testAndFailCheck.accept("scheduled bytes values don't match", scheduledBytes, connection.scheduledBytes());
+                // so if it is currently above the value we expect, use the spinAssert explicitly.
+                spinAssertEquals("scheduled count values don't match", scheduled, connection::scheduledCount, 5, TimeUnit.SECONDS);
+                spinAssertEquals("scheduled bytes values don't match", scheduledBytes, connection::scheduledBytes, 5, TimeUnit.SECONDS);
 
 Review comment:
   NIT: I would have used `testAndFailCheck` for ensuring that the behavior is the one expected by the `FailCheck` argument.
   In practice, unless I missed something, this code is only called from `ConnectionTest` so there will be no change of behavior.
   
   I can change that on commit if you want to.  

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org