dbwong commented on a change in pull request #550: PHOENIX-5348: Fix flaky
test: testIndexRebuildTask
URL: https://github.com/apache/phoenix/pull/550#discussion_r307914212
##########
File path:
phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexRebuildTaskIT.java
##########
@@ -188,7 +187,17 @@ public static void waitForTaskState(Connection conn,
PTable.TaskType taskType, P
String taskStatus = null;
- if (rs.next()) {
+ while (rs.next()) {
Review comment:
I will attempt to allude to the type of testing I'm referring to here. I
have not yet spent enough time to understand all the logic but this appears to
be 2 changes. 1, the data layer may return multiple results for our query. 2,
we need to test against the results and filter some of them (Why are we not
filtering in the query itself?). However, since the logic today couples these
concerns testing that our logic handles is not straight forward. We should be
able to write 3 unit tests, one that our logic handles multiple results and two
we look for the expected name and the expected time stamp. While I agree that
your IT tests likely check this behavior the very act of making code unit
testable suggests better boundaries. @gokceni
----------------------------------------------------------------
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
