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

 ##########
 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:
   If all those failing tests have proven us somthing is that the spin would be 
needed everywhere. The `doCheck` method with the FailCheck parameter is used in 
the burn tests to allow the implementor to plug his own notification mechanism 
for failure. That would not work with the current implementation that use the 
spin assert.
   My approach would be to let the user of that method handle the spinning if 
needed but I agree it is not a perfect solution.
   Otherwise we need to implements a spining that will work for `doCheck()`and 
`doCheck(FailCheck)`.
   

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to