[GitHub] [pulsar] codelipenghui commented on a diff in pull request #19124: [fix][broker][PIP-195] fix cursor skip read

[GitBox <gi...@apache.org>]

codelipenghui commented on code in PR #19124:
URL: https://github.com/apache/pulsar/pull/19124#discussion_r1061102602


##########
managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedCursorTest.java:
##########
@@ -4275,7 +4275,14 @@ public void testReadEntriesWithFilterOut() throws ManagedLedgerException, Interr
         cursor.asyncReadEntriesWithSkipOrWait(sendNumber, new ReadEntriesCallback() {
             @Override
             public void readEntriesComplete(List<Entry> entries, Object ctx) {
-                completableFuture.complete(entries.size());
+                try {
+                    entries.forEach(entry -> {
+                        assertNotEquals(entry.getEntryId() % 2, 0);

Review Comment:
   It should be `assertEquals(entry.getEntryId() % 2, 0);`
   
   And due to this one is executed by the SafeRun, so the test will not get failed event if the assert fails.



##########
managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedCursorTest.java:
##########
@@ -4295,7 +4302,14 @@ public void readEntriesFailed(ManagedLedgerException exception, Object ctx) {
         cursor.asyncReadEntriesWithSkipOrWait(sendNumber, new ReadEntriesCallback() {
             @Override
             public void readEntriesComplete(List<Entry> entries, Object ctx) {
-                completableFuture2.complete(entries.size());
+                try {
+                    entries.forEach(entry -> {
+                        assertEquals(entry.getEntryId() % 2, 0);

Review Comment:
   Same as the above comment. You might need to consider using a different way to assert the result.



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org