Fixing unstable tests - concern about retryStrategically(..) method

[Devin Bost <de...@gmail.com>]

I'm working on fixing the unstable tests (
https://github.com/apache/pulsar/issues/6137), and I've learned that many
of the unstable tests appear to be related to timing issues or the use of
Thread.sleep.

While investigating the issue, I noticed a commonly used method in the
tests with something odd:

public static boolean retryStrategically(Predicate<Void> predicate,
int retryCount, long intSleepTimeInMillis)
            throws Exception {
        for (int i = 0; i < retryCount; i++) {
            if (predicate.test(null) || i == (retryCount - 1)) {
                return true;
            }
            Thread.sleep(intSleepTimeInMillis + (intSleepTimeInMillis * i));
        }
        return false;
    }

If the predicate evaluates to true, then the method returns true (as would
be expected.)
However, if the predicate evaluates to false with every retry, once the
number of retries is exhausted, then the method just returns *true* (which
seems counter-intuitive because a true state was never reached.) Is this
the desired behavior? (It seems to me like the method should return *false*
if a true state was never reached.)

If not, then quite a few tests would be impacted by this issue and might be
incorrectly passing.

My changes that expose this behavior can be seen in my PR here:
https://github.com/apache/pulsar/pull/6164/files

If this is not the right place to ask this, please let me know.

-
Devin G. Bost