Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[improve][tests] Backwards compat tests: added new versions, pulsar upgrade cases, read check from old server #3981

Merged
merged 4 commits into from
Jun 5, 2024

Conversation

dlg99
Copy link
Contributor

@dlg99 dlg99 commented Jun 6, 2023

Descriptions of the changes in this PR:

Expanded backwards-compat tests

Motivation

On pulsar slack @rdhabalia reported BK-related errors during upgrade.

Changes

Backwards compat tests:

  • added new BK versions
  • pulsar upgrade cases
  • read check from old server (new client reads from old server)
  • upgrade tests with CRC32C (for select cases)

@@ -55,13 +55,25 @@ class TestCompatUpgrade {

try {
def ledger0 = currentRunningBK.createLedger(2, 2,
currentRunningCL.digestType("CRC32"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make digest type CRC32C which a default value for bk and other projects like pulsar.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rdhabalia done

@dlg99
Copy link
Contributor Author

dlg99 commented Jun 6, 2023

CI doesn't run these test for the reason unknown:

mvn -B -nsu -DintegrationTests -pl :upgrade test
..
[INFO] Tests run: 0, Failures: 0, Errors: 0, Skipped: 0

@dlg99
Copy link
Contributor Author

dlg99 commented Jun 6, 2023

Adding

    <dependency>
      <groupId>org.junit.vintage</groupId>
      <artifactId>junit-vintage-engine</artifactId>
      <version>${junit5.version}</version>
      <scope>test</scope>
    </dependency>

to the backwards compat modules makes mvn find the test but there are missing dependencies and/or some conflicts in the runtime that I haven't figured out

@hangc0276 hangc0276 added this to the 4.17.0 milestone Jun 13, 2023
Copy link
Member

@StevenLuMT StevenLuMT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work

@StevenLuMT
Copy link
Member

rerun failure checks

@StevenLuMT
Copy link
Member

BookKeeper CI / Backward compatibility tests (pull_request) Failed for:

Error: Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.0.0-M5:test (default-test) on project backward-compat-current-server-old-clients: No tests were executed! (Set -DfailIfNoTests=false to ignore this error.) -> [Help 1]
Error:
Error: To see the full stack trace of the errors, re-run Maven with the -e switch.
Error: Re-run Maven using the -X switch to enable full debug logging.
Error:
Error: For more information about the errors and possible solutions, please read the following articles:
Error: [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
Error: Process completed with exit code 1.

@eolivelli eolivelli modified the milestones: 4.17.0, 4.18.0 Mar 29, 2024
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, good work @dlg99

@dlg99 dlg99 merged commit 9656202 into apache:master Jun 5, 2024
23 checks passed
@dlg99 dlg99 deleted the backwards-compat-4.16 branch June 5, 2024 20:24
Ghatage pushed a commit to sijie/bookkeeper that referenced this pull request Jul 12, 2024
…pgrade cases, read check from old server (apache#3981)

* Backwards compat tests: added new versions, pulsar upgrade cases, read check from old server
* upgrade tests with crc32c
* Fail integration tests if no tests found
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants