HDDS-9790. Add tests for Overview page

[devabhishekpal]

What changes were proposed in this pull request?

HDDS-9790. Add tests for Overview page

Please describe your PR in detail:
-Currently we don't have any tests for Recon UI. This increases manual effort for validating UI fixes/changes.

• This PR will add the initial setup required for adding tests using ReactTestingLibrary and Jest, along with some unit test cases for the Recon Overview page and the AutoReloadPanel component for Recon.
• After we get good coverage of tests for basic scenarios, these can be run as a part of the CI checks to reduce manual effort in verifying changes, as well as providing an initial filter to catch bugs/errors early on.
• It uses RTL library to provide test functionality and Vitest as a test runner.
• Along with that it will use msw to intercept API calls to endpoints and return mock responses to ensure a single static source of truth for testing.

Reference for quick overview on the frameworks:

ReactTestingLibrary

React Testing Library is a lightweight testing library built on top of the DOM Testing Library for React components.
It focuses on the functionality of the component in contrast to the actual implementation of the component. This ensures that no matter how we implement our components and no matter the changes, we can ensure a consistent functionality.
It can work with any test runner, but we are using Jest as the preferred runner in this case.

Vitest

Vitest is a testing framework which also acts as a runner for our tests, allowing us to implement various testing methods and provide a way to interact with the virtual DOM to verify and assert conditions. It also has many built in features like mocking functions, generating coverage etc. allowing us to focus on the testing, rather than the implementation.

MSW

Mock Service Worker is a API mocking library that allows us to intercept API calls, and provide mocks for the same. It is independent of the client, framework and environment - this means we can run it anywhere independently.

How are we setting up our tests?

The main configuration for our tests start with configuring Vitest, this will be added under the vite.config.ts file's test section.
Under this test section we are calling the vitest.setup.ts file to run initially to import missing Window functions as jsdom doesn't provide them as a part of the virtual DOM and also we are importing the jest-dom vitest types which allows us to use jest assertions like toBeVisible(), toHaveTextContent() etc in our tests.

Reference Documentations for reading:

Vitest: https://v1.vitest.dev/
React Testing Library: https://testing-library.com/docs/react-testing-library/intro/
MSW: https://mswjs.io/docs/getting-started

How to run the tests

In order to run the tests we follow the below steps:

• Make sure pnpm version is 8.15.7 (this is the latest we are using in Recon)
• Run pnpm install in the project root (this will install all the test dependencies)
• Run pnpm test (this will call the vitest command and run the tests)

How to add new tests

We can add any other tests to the __tests__ folder inside the src directory.
Follow the naming convention of <component-name>.test.tsx

Any requests or API calls can be mocked using MSW and can be called to start listening in the beforeAll() method for the tests. This will allow it to intercept any API calls and return a mock data for the same.
Make sure to stop the listener in the cleanup function i.e. afterAll() in this case

What is the link to the Apache JIRA

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

How was this patch tested?

This patch was tested manually

[ArafatKhan2198]

@ivandika3 Could you please take a look at this.

[ivandika3]

Thanks @devabhishekpal for the patch and @ArafatKhan2198 for the review request. This is a good step to enable automated testing on UI and something I personally look forward to. Once UI unit tests is done, we can consider to enable UI e2e testing (using things like puppeteer / cypress / playwright).

I'm still trying to read the relevant documentations regarding the unit test frameworks since I'm not familiar with UI unit tests before, so I might need some more time.

@ArafatKhan2198 @dombizita @smitajoshi12 @devmadhuu Meanwhile, I think it's good to start familiarizing ourselves with the UI unit tests (and UI in general) so that we can review the related tests meaningful.

[ArafatKhan2198]

Thanks @devabhishekpal for the patch and @ArafatKhan2198 for the review request. This is a good step to enable automated testing on UI and something I personally look forward to. Once UI unit tests is done, we can consider to enable UI e2e testing (using things like puppeteer / cypress / playwright).

I'm still trying to read the relevant documentations regarding the unit test frameworks since I'm not familiar with UI unit tests before, so I might need some more time.

@ArafatKhan2198 @dombizita @smitajoshi12 @devmadhuu Meanwhile, I think it's good to start familiarizing ourselves with the UI unit tests (and UI in general) so that we can review the related tests meaningful.

That's a great point @ivandika3. It is absolutely necessary to go through them.

[devmadhuu]

Thanks @devabhishekpal for taking this initiative. Thanks @ivandika3 for patching me. Let me get some idea about this patch and test library.

[devmadhuu]

@devabhishekpal , few basic doubts:

1. As I understand these UI tests and the framework for Recon pages are unit tests which mocks the API calls and mainly we are testing 2 assertions with each card on page, one is if card is visible and another if mocked intercepted data is matching with data on card, so can we also expand the tests on this page for negative scenarios like:
   a. If API fails to provide data , what the page behaviour should be.

2. For UI tests, we also need to mainly deal with how the look & feel including color, width, height dimensions etc. E.g. if we have multiple components on a single page, how they will together be laid out and not overlapping or any other UI related issue like tooltip overlapping etc. This layout testing is very important part of our UI test because we have observed in past that with some PR change, it has broken layout settings or impacted on some other component. We should prioritise this.

3. Does this test framework will cover pnpm build failure before actually being tested in CI. Currently we have seen that pnpm version and compatibility issues arise out in CI once changes being pushed.

4. Are we still pushing static pnpm lock file ? If yes, t hen can we prioritise this to be built dynamically as part of CI build. This we discussed with @smitajoshi12 few months back.

5. Basic smoke tests for UI should be given priority first based on config flags , whether that is broken due to any UI PR change or not like if some nav item to be visible or not etc.

[devabhishekpal]

Hi @devmadhuu, thanks a lot for asking:

• Yes, thanks for your inputs, we should have the tests validate negative test scenarios as well like failure of API call, missing data etc.
• So regarding the layout, colour, feel etc. this would not be handled by Unit tests. While this is certainly possible via unit tests, it is not recommended. Mostly because:
  • The tests are run in a virtual DOM, and it is usually bad practice to make such test scenarios as virtual DOM is not visible to the user, it is an emulated DOM.
  • Other libraries like Cypress are better equipped for things like this as it has a usable browser where we can check what is being executed (it will show where the click happened, what is the current step being executed etc.) and it is a better job for E2E tests
• This would not be able to detect build issues, as this directly runs on the code and validates kind of similar to how Java Unit tests don't require the build step to happen, but directly validate the code - so build errors are not detected in this scenario. For this I would still recommend contributors to build the project after making UI changes both for build failure detection and also for testing it out via actual cluster operations
• So static lock-file needs to be pushed, as building it dynamically via CI is not quite optimized. Basically what the lockfile contains is metadata regarding the packages and generating it from scratch takes some time. Since we run pnpm install --frozen-lockfile this skips the fetching of package data from npm, re-creating the lockfile and then installing and directly goes to the installation phase which saves us quite some time in the install phase. This is also recommended for other package managers like npm and yarn in order to save build time.
• I think this might be possible, but it is not as intuitive as say for Python or Java where we can specify test group and markers to run specific set of tests.

[devmadhuu]

Hi @devmadhuu, thanks a lot for asking:

• Yes, thanks for your inputs, we should have the tests validate negative test scenarios as well like failure of API call, missing data etc.

• So regarding the layout, colour, feel etc. this would not be handled by Unit tests. While this is certainly possible via unit tests, it is not recommended. Mostly because:

  • The tests are run in a virtual DOM, and it is usually bad practice to make such test scenarios as virtual DOM is not visible to the user, it is an emulated DOM.
  • Other libraries like Cypress are better equipped for things like this as it has a usable browser where we can check what is being executed (it will show where the click happened, what is the current step being executed etc.) and it is a better job for E2E tests
    --- As long as we have it in plan, its good, whatever library is suitable for it. But this should be prioritised as in real time, we have faced this in past.

• This would not be able to detect build issues, as this directly runs on the code and validates kind of similar to how Java Unit tests don't require the build step to happen, but directly validate the code - so build errors are not detected in this scenario. For this I would still recommend contributors to build the project after making UI changes both for build failure detection and also for testing it out via actual cluster operations

• So static lock-file needs to be pushed, as building it dynamically via CI is not quite optimized. Basically what the lockfile contains is metadata regarding the packages and generating it from scratch takes some time. Since we run pnpm install --frozen-lockfile this skips the fetching of package data from npm, re-creating the lockfile and then installing and directly goes to the installation phase which saves us quite some time in the install phase. This is also recommended for other package managers like npm and yarn in order to save build time.

• I think this might be possible, but it is not as intuitive as say for Python or Java where we can specify test group and markers to run specific set of tests.
  --- IMO, smoke tests should be given priority because this also we have faced in past that due to introduction of some flag, something got broken and it has nothing do with backend.

[ivandika3]

@devabhishekpal Thanks for kickstarting the UI unit test initiative. Overall looks good. Left a few comments.

[devabhishekpal]

Hello, it took me some time to get back to introducing these tests. Sorry for the delay.

@ivandika3 Thanks for pointing out the issues.
Currently we are undertaking an effort to revamp the Ozone Recon UI. As a part of this the latest commit migrates the tests to run on the new V2 UI.
I also addressed the other comments you gave.

@devmadhuu I have parameterized the tests to run on both positive as well as negative scenario.

Please do take another look and let me know your inputs.
I think we could improve the test description? @ivandika3 @devmadhuu

Thanks a lot for taking the time to review this PR.

[ivandika3]

Thanks for the update. LGTM overall, left few more comments.

[devabhishekpal]

Thanks for the review @ivandika3

Addressed the issues you pointed out.
Could you take a look at the current format of the test description?
#6983 (comment)

[ivandika3]

@devabhishekpal Thanks for the patch. LGTM +1.

[ivandika3]

@devabhishekpal Thanks for the patch, @devmadhuu @ArafatKhan2198 for the reviews.