yifan-c commented on a change in pull request #470: CASSANDRA-15630 fix
testSerializeError
URL: https://github.com/apache/cassandra/pull/470#discussion_r398665708
##########
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:
It is probably better to use `spinAssertEquals` explicitly. If you strongly
prefer to use `testAndFailCheck`, I am cool with it.
The spin equality assertion was used because
- the original code that Benedict wrote did it explicitly using a for loop.
The assertion is likely to require multiple retries.
- the parameter type `FailCheck` for `doCheck()` suggests it can accept any
`FailCheck` impl. We cannot assume the input check is always spin assertion
based (, although it is only used in `ConnectionTest` as of now). If a custom
non-spin assertion is passed, the check can fail.
- the error message, i.e. "scheduled count values don't match", suggests the
purpose is to assert values are equal. So `spinAssertEquals` does the job.
----------------------------------------------------------------
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]
