search

canonical / juju-dashboard

View the real-time status of your Juju or JAAS environment.
https://juju.is
GNU Lesser General Public License v3.0
23 stars 23 forks source link

WD-11590 - refactor: remove nested components in CodeSnippetBlock #1766

Closed vladimir-cucu closed 3 months ago

vladimir-cucu commented 3 months ago

Done

Details

webteam-app commented 3 months ago

Demo

Jenkins

demos.haus

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 99.24242% with 2 lines in your changes missing coverage. Please review.

Project coverage is 96.94%. Comparing base (16a7c52) to head (ef6b678). Report is 3 commits behind head on main.

Files Patch % Lines
...es/AdvancedSearch/CodeSnippetBlock/Value/Value.tsx 98.68% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1766 +/- ## ========================================== + Coverage 96.93% 96.94% +0.01% ========================================== Files 411 418 +7 Lines 23067 23160 +93 Branches 2922 2934 +12 ========================================== + Hits 22360 22453 +93 Misses 707 707 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

huwshimi commented 3 months ago

@vladimir-cucu (not sure if this PR is ready for a review yet, if not you can ignore this comment). It looks like most of the logic in Label is only tested by the parent component. I think either the tests should be moved over, or if you think some of them are necessary then it'd be worth making sure the logic is also tested in the Label so that if the parent changes we don't lose those tests.

vladimir-cucu commented 3 months ago

@vladimir-cucu (not sure if this PR is ready for a review yet, if not you can ignore this comment). It looks like most of the logic in Label is only tested by the parent component. I think either the tests should be moved over, or if you think some of them are necessary then it'd be worth making sure the logic is also tested in the Label so that if the parent changes we don't lose those tests.

I thought of leaving the tests as they are in the parent component and add additional tests for Label. The PR is not yet ready for review, as most of the tests are missing. Sorry for not specifying that.