AHeise commented on a change in pull request #16940:
URL: https://github.com/apache/flink/pull/16940#discussion_r695190021
##########
File path:
flink-test-utils-parent/flink-connector-testing/src/main/java/org/apache/flink/connectors/test/common/utils/TestDataMatchers.java
##########
@@ -151,11 +156,15 @@ protected boolean matchesSafely(Iterator<T>
resultIterator, Description descript
final T record = resultIterator.next();
if (!matchThenNext(record)) {
mismatchDescription = String.format("Unexpected record
'%s'", record);
+ logCurrentProgress();
return false;
}
}
if (!hasReachedEnd()) {
- mismatchDescription = "Result data is less than test data";
+ mismatchDescription =
+ "Number of records consumed from external system is
less than "
+ + "records written to external system";
Review comment:
This message apparently was also not very helpful. Could we add how many
matched and which record is the first that didn't?
##########
File path:
flink-test-utils-parent/flink-connector-testing/src/main/java/org/apache/flink/connectors/test/common/utils/TestDataMatchers.java
##########
@@ -151,11 +156,15 @@ protected boolean matchesSafely(Iterator<T>
resultIterator, Description descript
final T record = resultIterator.next();
if (!matchThenNext(record)) {
mismatchDescription = String.format("Unexpected record
'%s'", record);
Review comment:
Apparently this mismatch description is not very helpful. In the very
least, I'd add offset and the expected record.
##########
File path:
flink-test-utils-parent/flink-connector-testing/src/main/java/org/apache/flink/connectors/test/common/utils/TestDataMatchers.java
##########
@@ -191,55 +204,32 @@ private boolean matchThenNext(T record) {
* @return True if all pointers have reached the end.
*/
private boolean hasReachedEnd() {
- for (IteratorWithCurrent<T> testDataIterator : testDataIterators) {
- if (testDataIterator.hasNext()) {
+ for (int listIndex = 0; listIndex < testRecordsLists.size();
listIndex++) {
+ if (testDataOffsets[listIndex] <
testRecordsLists.get(listIndex).size()) {
return false;
}
}
return true;
}
- }
-
- /**
- * An iterator wrapper which can access the element that the iterator is
currently pointing to.
- *
- * @param <E> The type of elements returned by this iterator.
- */
- static class IteratorWithCurrent<E> implements Iterator<E> {
-
- private final Iterator<E> originalIterator;
- private E current;
-
- public IteratorWithCurrent(Iterator<E> originalIterator) {
- this.originalIterator = originalIterator;
- try {
- current = originalIterator.next();
- } catch (NoSuchElementException e) {
- current = null;
- }
- }
- @Override
- public boolean hasNext() {
- return current != null;
- }
-
- @Override
- public E next() {
- if (current == null) {
- throw new NoSuchElementException();
- }
- E previous = current;
- if (originalIterator.hasNext()) {
- current = originalIterator.next();
- } else {
- current = null;
+ private void logCurrentProgress() {
Review comment:
Why is this not added to the `mismatchDescription`?
--
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]
