gortiz commented on code in PR #11205:
URL: https://github.com/apache/pinot/pull/11205#discussion_r1277784575
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/operator/MailboxSendOperatorTest.java:
##########
@@ -160,20 +151,23 @@ public void shouldSendDataBlock()
// When:
block = mailboxSendOperator.nextBlock();
// Then:
- assertTrue(block.isNoOpBlock(), "expected No-op block to propagate next
b/c remaining capacity is 0.");
+ assertTrue(block.isErrorBlock(), "expected error block to propagate next
b/c remaining capacity is 0.");
// When:
block = mailboxSendOperator.nextBlock();
// Then:
assertSame(block, eosBlock, "expected EOS block to propagate next even if
capacity is now 0.");
ArgumentCaptor<TransferableBlock> captor =
ArgumentCaptor.forClass(TransferableBlock.class);
- verify(_exchange, times(3)).offerBlock(captor.capture(), anyLong());
+ verify(_exchange, Mockito.times(4)).offerBlock(captor.capture(),
anyLong());
Review Comment:
I've removed several of these mockito checks. The test should not check how
many times an internal method is called. Tests should check the external
contract, not the internal one.
I had tons of test failling for that reason (because even we return the
same, in the new implementation we do an extra call to read the EOS). In this
case I decided to keep it because the test is using mockito to read the actual
results of the operator. I don't think that is a good idea, but it would take
some time to implement the same logic using _better practices_.
--
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]
