determined-ai / determined

Determined is an open-source machine learning platform that simplifies distributed training, hyperparameter tuning, experiment tracking, and resource management. Works with PyTorch and TensorFlow.
https://determined.ai
Apache License 2.0
3k stars 350 forks source link

chore: tests for Searches component (ET-198) #9576

Closed johnkim-det closed 3 months ago

johnkim-det commented 3 months ago

Ticket

ET-198

Description

Add tests to Searches component

Test Plan

No additional testing required, automated tests should pass.

Checklist

netlify[bot] commented 3 months ago

Deploy Preview for determined-ui ready!

Name Link
Latest commit 7f0ada42fc9a78b48ce6d3510d051c2aa219f9ce
Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/66845b809ab1910008877d2e
Deploy Preview https://deploy-preview-9576--determined-ui.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 47.43%. Comparing base (84072f6) to head (7f0ada4). Report is 4 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #9576 +/- ## ========================================== - Coverage 51.36% 47.43% -3.93% ========================================== Files 1252 929 -323 Lines 152174 123056 -29118 Branches 3024 3130 +106 ========================================== - Hits 78158 58369 -19789 + Misses 73858 64534 -9324 + Partials 158 153 -5 ``` | [Flag](https://app.codecov.io/gh/determined-ai/determined/pull/9576/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai) | Coverage Ξ” | | |---|---|---| | [harness](https://app.codecov.io/gh/determined-ai/determined/pull/9576/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai) | `?` | | | [web](https://app.codecov.io/gh/determined-ai/determined/pull/9576/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai) | `49.98% <100.00%> (+1.99%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/determined-ai/determined/pull/9576?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai) | Coverage Ξ” | | |---|---|---| | [webui/react/src/components/ColumnPickerMenu.tsx](https://app.codecov.io/gh/determined-ai/determined/pull/9576?src=pr&el=tree&filepath=webui%2Freact%2Fsrc%2Fcomponents%2FColumnPickerMenu.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai#diff-d2VidWkvcmVhY3Qvc3JjL2NvbXBvbmVudHMvQ29sdW1uUGlja2VyTWVudS50c3g=) | `95.87% <100.00%> (ΓΈ)` | | | [webui/react/src/components/Searches/Searches.tsx](https://app.codecov.io/gh/determined-ai/determined/pull/9576?src=pr&el=tree&filepath=webui%2Freact%2Fsrc%2Fcomponents%2FSearches%2FSearches.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai#diff-d2VidWkvcmVhY3Qvc3JjL2NvbXBvbmVudHMvU2VhcmNoZXMvU2VhcmNoZXMudHN4) | `56.49% <100.00%> (+56.49%)` | :arrow_up: | ... and [334 files with indirect coverage changes](https://app.codecov.io/gh/determined-ai/determined/pull/9576/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai)
johnkim-det commented 3 months ago

Saw that some tests were failing in CI so I've reverted the dependency update, which still makes the act warnings appear.

I do think these warnings are not unique to this PR and are appearing throughout our test suite.

It looks like this is something that's an issue in @testing-library/react which was fixed in version 14 (https://github.com/testing-library/react-testing-library/releases/tag/v14.0.0). However, upgrading to that version breaks other tests. My thinking here is that we should just create a separate task for updating this library version: https://hpe-aiatscale.atlassian.net/browse/ET-608

johnkim-det commented 3 months ago

I think code cov report is saying there's full coverage, the 47.43% is project-level coverage?

Also I think a couple of the waitFors are necessary but removing the unnecessary ones and the unnecessary Providers.

keita-determined commented 3 months ago

I think code cov report is saying there's full coverage, the 47.43% is project-level coverage?

oh yes i always get confused

Also I think a couple of the waitFors are necessary but removing the unnecessary ones and the unnecessary Providers.

i tested them without waitFor and it worked well on my local though. not sure