westonpace commented on a change in pull request #11958:
URL: https://github.com/apache/arrow/pull/11958#discussion_r769804708
##########
File path: cpp/src/arrow/util/async_util_test.cc
##########
@@ -158,6 +158,22 @@ TYPED_TEST(TypedTestAsyncTaskGroup, Error) {
ASSERT_FINISHES_AND_RAISES(Invalid, task_group.End());
}
+TYPED_TEST(TypedTestAsyncTaskGroup, ErrorWhileNotEmpty) {
+ TypeParam task_group;
+ Future<> pending_task = Future<>::Make();
+ Future<> will_fail_task = Future<>::Make();
+ Future<> after_fail_task = Future<>::Make();
+ ASSERT_OK(task_group.AddTask([pending_task] { return pending_task; }));
+ ASSERT_OK(task_group.AddTask([will_fail_task] { return will_fail_task; }));
+ ASSERT_OK(task_group.AddTask([after_fail_task] { return after_fail_task; }));
+ Future<> end = task_group.End();
+ AssertNotFinished(end);
+ pending_task.MarkFinished();
+ will_fail_task.MarkFinished(Status::Invalid("XYZ"));
+ after_fail_task.MarkFinished();
Review comment:
Yes, there is a little bit of discrepancy between the serialized task
group and the parallel one here so I had to mark `after_fail_task` finished
(turn it into future). The serialized task group will never "launch" the task
and so we can safely complete without running `after_fail_task`. The parallel
task group launches the task as soon as it is added so even though we stop
accepting new tasks once we hit an error we have to wait for all launched tasks
to complete before finishing the group.
I could split it into two different test cases but this one was enough to
reproduce the issue in the serialized group so I left it this way.
--
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]
