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

HDDS-9792. Add tests for Pipelines page #7859

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

devabhishekpal
Copy link
Contributor

What changes were proposed in this pull request?

HDDS-9792. Add tests for Pipelines page

Please describe your PR in detail:

  • This PR adds unit tests for the new Pipelines page, and the Pipelines Table component
  • It also updates the imports from absolute import to use symbolic/alias import i.e from @/__tests__ to the alias @tests

What is the link to the Apache JIRA

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

How was this patch tested?

Patch was tested manually
Screenshot 2025-02-12 at 14 09 48

@devabhishekpal devabhishekpal marked this pull request as ready for review February 12, 2025 09:51
@adoroszlai
Copy link
Contributor

Thanks @devabhishekpal for the patch. Are these tests run anywhere in CI?

@devabhishekpal
Copy link
Contributor Author

Hi @adoroszlai. So currently these tests are not run anywhere.
The overall plan is that we will be enabling these in the CI once the tests for all the pages are added, and it will be run for any JS file changes in recon module instead of the integration, acceptance tests.

For now we are running the tests locally by executing pnpm test to ensure nothing is breaking between changes

@ArafatKhan2198
Copy link
Contributor

Did a quick review on your patch @devabhishekpal
Are we adding tests for pagination? I recall there was some issue with pagination—we might need to try adding it to verify.

@ArafatKhan2198
Copy link
Contributor

Another question :-
For sorting, we are currently only implementing it for PipelineID. Should we also add it for Replicator Type and Status columns?

@devabhishekpal
Copy link
Contributor Author

Thanks for the inputs @ArafatKhan2198.
I addressed the comments in the latest commit. Could you help take a look at it again?

Copy link
Contributor

@ArafatKhan2198 ArafatKhan2198 left a comment

Choose a reason for hiding this comment

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

Thanks for updating the patch @devabhishekpal
LGTM +1

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.

3 participants