DonalEvans commented on a change in pull request #6976:
URL: https://github.com/apache/geode/pull/6976#discussion_r726625190
##########
File path:
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionSingleHopDUnitTest.java
##########
@@ -952,16 +915,16 @@ public void testClientMetadataForPersistentPrs() throws
Exception {
});
}
- LATCH.get().await(getTimeout().toMillis(), MILLISECONDS);
+ assertThat(LATCH.get().await(getTimeout().toMillis(),
MILLISECONDS)).isTrue();
Review comment:
I may be misunderstanding something here, but am I right in thinking
that this change won't prevent the test from failing, just make it fail at a
different point than described in GEODE-9617? If the failure described in
GEODE-9617 is due to this await silently timing out, then asserting here that
it doesn't time out will just move the failure to here.
Also, GEODE-9617 seems to describe two distinct failures; one in this test,
and one in `testMetadataServiceCallAccuracy_FromGetOp()`. Since this PR doesn't
modify `testMetadataServiceCallAccuracy_FromGetOp()` beyond minor refactoring,
is it accurate to say that any flakiness in that test won't be affected by it?
Finally, could the title of this PR be changed to meet the criteria
described on the wiki:
https://cwiki.apache.org/confluence/display/GEODE/Commit+Message+Format
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
