kfaraz commented on code in PR #18235:
URL: https://github.com/apache/druid/pull/18235#discussion_r2202448377
##########
services/src/test/java/org/apache/druid/testing/embedded/EmbeddedClusterApis.java:
##########
@@ -116,6 +116,20 @@ public void waitForTaskToSucceed(String taskId,
EmbeddedOverlord overlord)
verifyTaskHasStatus(taskId, TaskStatus.success(taskId));
}
+ /**
+ * Waits for the given task to fail, with the given errorMsq. If the given
+ * {@link EmbeddedOverlord} is not the leader, this method can only return by
+ * throwing an exception upon timeout.
+ */
+ public void waitForTaskToFail(String taskId, String errorMsg,
EmbeddedOverlord overlord)
Review Comment:
I think `waitForTaskToFail` feels like an odd condition to check anyway.
`waitForTaskToSucceed` is a much more common (and constructive) condition
that is required by many tests.
As suggested by @gianm in other comments that we should make things
composable,
this method should probably just be `waitForTaskToFinish`.
The verification of the actual task status and error message can be done in
the calling test itself using matchers.
If a similar assertion needs to be done in multiple places in the test, I
would still prefer that we add a method `waitForTaskToFail` in that test rather
than here.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
