You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/06/09 05:36:27 UTC

[GitHub] [pulsar] codelipenghui opened a new pull request, #15995: Fix failed tests for branch-2.10

codelipenghui opened a new pull request, #15995:
URL: https://github.com/apache/pulsar/pull/15995

   After cherry-picked #15627, the test `org.apache.bookkeeper.mledger.impl.ManagedLedgerTest`
   will fail for branch-2.10. Details to see https://github.com/apache/pulsar/pull/15627#issuecomment-1149962309


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


[GitHub] [pulsar] codelipenghui commented on pull request #15995: Fix failed tests for branch-2.10

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on PR #15995:
URL: https://github.com/apache/pulsar/pull/15995#issuecomment-1151775907

   I have created an issue to track the failed test https://github.com/apache/pulsar/issues/16000
   I will change to byte[] schema for this PR to unblock the 2.10.1 release.
   It is only a test-related issue.


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


[GitHub] [pulsar] github-actions[bot] commented on pull request #15995: Fix failed tests for branch-2.10

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15995:
URL: https://github.com/apache/pulsar/pull/15995#issuecomment-1150690555

   @codelipenghui:Thanks for your contribution. For this PR, do we need to update docs?
   (The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)


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


[GitHub] [pulsar] gaoran10 commented on a diff in pull request #15995: Fix failed tests for branch-2.10

Posted by GitBox <gi...@apache.org>.
gaoran10 commented on code in PR #15995:
URL: https://github.com/apache/pulsar/pull/15995#discussion_r894157874


##########
managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerTest.java:
##########
@@ -2293,8 +2293,8 @@ public void testGetNumberOfEntriesInStorage() throws Exception {
         stateUpdater.set(managedLedger, ManagedLedgerImpl.State.LedgerOpened);
         managedLedger.rollCurrentLedgerIfFull();
         Awaitility.await().untilAsserted(() -> {
-            assertEquals(managedLedger.getLedgersInfo().size(), 2);
-            assertEquals(managedLedger.getState(), ManagedLedgerImpl.State.ClosedLedger);
+            assertEquals(managedLedger.getLedgersInfo().size(), 3);

Review Comment:
   It seems that this changed the test logic.



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


[GitHub] [pulsar] codelipenghui commented on a diff in pull request #15995: Fix failed tests for branch-2.10

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on code in PR #15995:
URL: https://github.com/apache/pulsar/pull/15995#discussion_r894261046


##########
managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerTest.java:
##########
@@ -2293,8 +2293,8 @@ public void testGetNumberOfEntriesInStorage() throws Exception {
         stateUpdater.set(managedLedger, ManagedLedgerImpl.State.LedgerOpened);
         managedLedger.rollCurrentLedgerIfFull();
         Awaitility.await().untilAsserted(() -> {
-            assertEquals(managedLedger.getLedgersInfo().size(), 2);
-            assertEquals(managedLedger.getState(), ManagedLedgerImpl.State.ClosedLedger);
+            assertEquals(managedLedger.getLedgersInfo().size(), 3);

Review Comment:
   Yes, please see the description of this PR.



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


[GitHub] [pulsar] codelipenghui commented on pull request #15995: Fix failed tests for branch-2.10

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on PR #15995:
URL: https://github.com/apache/pulsar/pull/15995#issuecomment-1150832018

   /pulsarbot run-failure-checks


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


[GitHub] [pulsar] gaoran10 commented on a diff in pull request #15995: Fix failed tests for branch-2.10

Posted by GitBox <gi...@apache.org>.
gaoran10 commented on code in PR #15995:
URL: https://github.com/apache/pulsar/pull/15995#discussion_r894157874


##########
managed-ledger/src/test/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerTest.java:
##########
@@ -2293,8 +2293,8 @@ public void testGetNumberOfEntriesInStorage() throws Exception {
         stateUpdater.set(managedLedger, ManagedLedgerImpl.State.LedgerOpened);
         managedLedger.rollCurrentLedgerIfFull();
         Awaitility.await().untilAsserted(() -> {
-            assertEquals(managedLedger.getLedgersInfo().size(), 2);
-            assertEquals(managedLedger.getState(), ManagedLedgerImpl.State.ClosedLedger);
+            assertEquals(managedLedger.getLedgersInfo().size(), 3);

Review Comment:
   It seems that this changed the intention of the test.



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


[GitHub] [pulsar] codelipenghui merged pull request #15995: Fix failed tests for branch-2.10

Posted by GitBox <gi...@apache.org>.
codelipenghui merged PR #15995:
URL: https://github.com/apache/pulsar/pull/15995


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


[GitHub] [pulsar] codelipenghui commented on pull request #15995: Fix failed tests for branch-2.10

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on PR #15995:
URL: https://github.com/apache/pulsar/pull/15995#issuecomment-1152336144

   > @codelipenghui can you please add in the description the reason why we need to set -DtestReuseFork=false ?
   
   Not 100% sure what's going on for now, but looks like It's related to the concurrency issue in the class initialization. Just found some related information. 
   
   https://maven.apache.org/surefire/maven-surefire-plugin/examples/fork-options-and-parallel-execution.html#parallel-test-execution
   
   https://github.com/testcontainers/testcontainers-java/issues/2939#issuecomment-649810918


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


[GitHub] [pulsar] codelipenghui closed pull request #15995: Fix failed tests for branch-2.10

Posted by GitBox <gi...@apache.org>.
codelipenghui closed pull request #15995: Fix failed tests for branch-2.10
URL: https://github.com/apache/pulsar/pull/15995


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


[GitHub] [pulsar] github-actions[bot] commented on pull request #15995: Fix failed tests for branch-2.10

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15995:
URL: https://github.com/apache/pulsar/pull/15995#issuecomment-1150690692

   @codelipenghui:Thanks for providing doc info!


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