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 only. 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
