search

firefox-devtools / profiler

Firefox Profiler — Web app for Firefox performance analysis
https://profiler.firefox.com
Mozilla Public License 2.0
1.19k stars 387 forks source link

Add menu item to hide all tracks by type #5106

Closed varpstar closed 4 days ago

varpstar commented 4 weeks ago

fixes https://github.com/firefox-devtools/profiler/issues/2768

I am not sure if I need to add some additional tests, since I used hideProvidedTracks, which seems to be ready for production already

codecov[bot] commented 3 weeks ago

Codecov Report

Attention: Patch coverage is 87.69231% with 8 lines in your changes missing coverage. Please review.

Project coverage is 88.56%. Comparing base (d9dd96d) to head (4dda893). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/components/timeline/TrackContextMenu.js 85.00% 6 Missing :warning:
src/profile-logic/tracks.js 90.90% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #5106 +/- ## ========================================== - Coverage 88.56% 88.56% -0.01% ========================================== Files 307 307 Lines 27837 27898 +61 Branches 7538 7555 +17 ========================================== + Hits 24655 24708 +53 - Misses 2965 2973 +8 Partials 217 217 ``` | [Flag](https://app.codecov.io/gh/firefox-devtools/profiler/pull/5106/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=firefox-devtools) | Coverage Δ | | |---|---|---| | [](https://app.codecov.io/gh/firefox-devtools/profiler/pull/5106/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=firefox-devtools) | `88.56% <87.69%> (-0.01%)` | :arrow_down: | 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=firefox-devtools#carryforward-flags-in-the-pull-request-comment) to find out more.

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

varpstar commented 3 weeks ago

@canova Should I cover all those lines with tests from Codecov Report?

julienw commented 2 weeks ago

To answer your question, yes it would be good to have some specific tests covering this feature. We don't need a lot of them and covering all the lines, but at least the main ones.

For the tests, you can have a look at the existing related ones in src/test/components/TrackContextMenu.test.js. I'm looking for a specific test that would click on this option and check the result, maybe once for a global track and once for a local track.

varpstar commented 2 weeks ago

I found it difficult to decide where to put the type name converter. There are some converters in the tests folder. But I think it would be better to place it somewhere outside of trackContextMenu because it can be used elsewhere as well