westonpace commented on a change in pull request #8680:
URL: https://github.com/apache/arrow/pull/8680#discussion_r546020261
##########
File path: cpp/src/arrow/util/future_test.cc
##########
@@ -122,15 +122,21 @@ void AssertFinished(const Future<T>& fut) {
// Assert the future is successful *now*
template <typename T>
void AssertSuccessful(const Future<T>& fut) {
- ASSERT_EQ(fut.state(), FutureState::SUCCESS);
- ASSERT_OK(fut.status());
+ ASSERT_TRUE(fut.Wait(0.1));
Review comment:
I should clarify, I took out the `Wait(0.1)` since the check beforehand
that it was completed was good enough. So now if it is in the pending state it
will fail. This change was intended to make debugging easier.
`AssertSuccessful` calls `fut.state()` which (if the future is pending) will
wait on the pending future. This means a bug in the test or code which
resulted in a pending future being passed to `AssertSuccessful` would deadlock
and this was making the tests difficult to debug.
Now we simply check for pending first as a special case and fail if it is
pending before we check the state.
----------------------------------------------------------------
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]
