Skip to content

HDDS-12715. add integration tests for debug-replicas-verify-checksums tool #8209

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ptlrs
Copy link
Contributor

@ptlrs ptlrs commented Apr 1, 2025

Please describe your PR in detail:

  1. Implemented an Extension class which creates the miniozonecluster that can be reused
  2. Implemented an integration test for the ozone debug replicas verify checksums command
  3. Refactored TestOzoneDebugShell to use the Extension class

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-12715

How was this patch tested?

CI: https://github.com/ptlrs/ozone/actions/runs/15119784564

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @ptlrs for working on this.

Moving cluster creation to an extension reduces duplication, but:

  • test execution is still unnecessarily long, because each test class needs to start its own cluster
  • the specific implementation does not allow config to be customized if needed

Please check TestOzoneIntegrationNonHA and its parent classes for a better way to reuse a shared cluster for multiple test classes (HDDS-12183 and several later sub-tasks of HDDS-9000).

@errose28 errose28 added the tools Tools that helps with debugging label Apr 1, 2025
@ptlrs
Copy link
Contributor Author

ptlrs commented Apr 8, 2025

Thanks for the review @adoroszlai.
I agree, the cluster config here is limited to what is set in the extension.

With the custom extension, the cluster is created only once for both the test classes. The cluster is initialized once and stored as a static member. I had confirmed this by logging the creation of MiniOzoneCluster.

Is the goal of the new approach to consolidate all tests that can reuse the same config/cluster?

@ptlrs ptlrs force-pushed the HDDS-12715-common-integration-test-cluster-for-debug-and-repair-tools branch from 42555b8 to 68fa7bb Compare April 16, 2025 20:00
@ptlrs ptlrs requested a review from adoroszlai April 30, 2025 17:13
@ptlrs
Copy link
Contributor Author

ptlrs commented Apr 30, 2025

Hi @adoroszlai, the PR has been updated to use the existing TestOzoneIntegrationNonHA class. Could you please take another look.

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @ptlrs for updating the patch.

I think the PR title also needs to be updated.

@ptlrs ptlrs force-pushed the HDDS-12715-common-integration-test-cluster-for-debug-and-repair-tools branch from 857aae0 to a88c216 Compare May 19, 2025 17:42
@ptlrs ptlrs changed the title HDDS-12715. create a common miniozone cluster for ozone debug tools tests HDDS-12715. add integration tests for debug-replicas-verify-checksums tool May 19, 2025
@ptlrs ptlrs force-pushed the HDDS-12715-common-integration-test-cluster-for-debug-and-repair-tools branch from a88c216 to 0674e6c Compare May 19, 2025 17:49
@ptlrs ptlrs requested a review from adoroszlai May 19, 2025 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Tools that helps with debugging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants