ctubbsii commented on a change in pull request #1552: Fix and simplify
TransportCachingIT
URL: https://github.com/apache/accumulo/pull/1552#discussion_r389738446
##########
File path: test/src/main/java/org/apache/accumulo/test/TransportCachingIT.java
##########
@@ -148,7 +114,7 @@ public void testCachedTransport() throws
InterruptedException {
while (fourth == null) {
try {
// Get a non-cached transport
- fourth = pool.getAnyTransport(servers, false).getSecond();
+ fourth = pool.getAnyTransport(servers.subList(0, 1),
false).getSecond();
Review comment:
I think the test is similar between the first/second. Where those were
checking basic caching behavior, this one was specifically testing LIFO
behavior, as mentioned in the comment above: *// ensure the LIFO scheme with a
fourth and fifth entry*
However, there are still some nuances I don't understand. Like this fourth
seems like it is supposed to be for the same server as one of first, second, or
third. Fifth is supposed to make sure that it matches fourth, and not the same
server from first/second/third (LIFO behavior). However, because we have more
than one server, it doesn't seem like there's any guarantee that fourth/fifth
refer to the same server as one of first/second/third, so this test might
occasionally not test LIFO behavior, and basically do the same as first/second
(basic caching) but for a different server than that one chose.
I think some of this can be resolved if we select `servers.subList(0,1)` at
the beginning of the test, and stick with that one the whole time, so we make
sure we're testing with the same server.
----------------------------------------------------------------
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
