Bug 2032537: show count of the improvements and regressions in the subtests per platform #1029
Conversation
✅ Deploy Preview for mozilla-perfcompare ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1029 +/- ##
==========================================
+ Coverage 96.22% 96.32% +0.10%
==========================================
Files 112 113 +1
Lines 3175 3262 +87
Branches 715 739 +24
==========================================
+ Hits 3055 3142 +87
Misses 118 118
Partials 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@mgaudet I'm glad you brought up #3 because I wasn't too sure if it was per platform or all the platforms together. I can update it to go by platform and see how it looks. Hmm, I'll look over #2. |
gmierz
left a comment
There was a problem hiding this comment.
Great change! Some things I noticed as I was testing:
(1) It looks like the spacing between the rows on the result page are larger. Is this expected?
(2) Looking at the network tab, when I am loading the page with MWU on the results page, I see the subtest results being fetched twice: once when we're fetching the main results, and then a second time once the main results are obtained. Seems like a bug in the implementation somewhere and will need to be fixed.
(3) Sometimes it's unclear which row the tags that show the regressions/improvements are associated with (is it the row above it or below it). Is there something we can do to make it clearer that it's the row below them?
I think that if we're starting to fetch the subtest results in the background, we should get a bug filed to use those results in the subtest page too instead of doing another request for the same data when we navigate to the subtests.
|
@gmierz Thanks for your feedback! 1.) and 3.) The extra spacing is because of the regressions and improvement pills' placements. I've moved them inside the revision row so it's clearer which pills belongs to which row and to preserve the original spacing of the rows.
2.) I'm not seeing the subtests call for the main results fetch? We shouldn't be fetching the subtests when the main results are being fetched. The only subtests calls I see are for the rows with unique subtests that I'm fetching to count the improvements and regressions. Since that's a lot of unique calls, I could batch them into one request? Also could you please show me exactly where you see the subtests fetch for the main results call, maybe with screenshots?
|
Also improved performance
b3e3cf3 to
a8ed45a
Compare
…gger two fetches when switching between test versions
gmierz
left a comment
There was a problem hiding this comment.
r+ great work! The fix you made also seemed to make the caching work when switching the test versions too. :)


Solution for Bug 2032537
JIRA
Updated:
In this PR, I created a custom hook called
useSubtestRegressionCountto find, for a given test block, every row that has subtests; fetch those subtests from the treeherderAPI; and return the counts of regressions and improvements for each platform. Intreeherder.ts, added memoized fetches for the subtests to improve performance and avoid unnecessary refetching.RevisionRowsupplies the hook with the props to display the number of regressions and improvements for the subtests present in each row of the block.Note: need to add test coverage next. So in progress.
Deploy link