dev: Clean up some unneccesary mocks to speed up gazebo tests #3604
Conversation
Bundle ReportBundle size has no change ✅ |
Bundle ReportBundle size has no change ✅ |
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #3604 +/- ##
=======================================
Coverage 99.00% 99.00%
=======================================
Files 810 810
Lines 14562 14569 +7
Branches 4143 4154 +11
=======================================
+ Hits 14417 14424 +7
Misses 138 138
Partials 7 7 see 14 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3604 +/- ##
=======================================
Coverage 99.00% 99.00%
=======================================
Files 810 810
Lines 14562 14569 +7
Branches 4150 4161 +11
=======================================
+ Hits 14417 14424 +7
Misses 138 138
Partials 7 7 see 14 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3604 +/- ##
=======================================
Coverage 99.00% 99.00%
=======================================
Files 810 810
Lines 14562 14569 +7
Branches 4150 4161 +11
=======================================
+ Hits 14417 14424 +7
Misses 138 138
Partials 7 7 see 14 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3604 +/- ##
=======================================
Coverage 99.00% 99.00%
=======================================
Files 810 810
Lines 14562 14569 +7
Branches 4143 4154 +11
=======================================
+ Hits 14417 14424 +7
Misses 138 138
Partials 7 7 see 14 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
| @@ -6,19 +6,6 @@ import { MemoryRouter, Route } from 'react-router-dom' | |||
|
|
|||
| import ToggleHeader from './ToggleHeader' | |||
|
|
|||
| const mocks = vi.hoisted(() => ({ | |||
There was a problem hiding this comment.
this mock wasn't doing anything
✅ Deploy preview for gazebo ready!Previews expire after 1 month automatically.
|
| @@ -1316,12 +1318,6 @@ describe('DefaultOrgSelector', () => { | |||
| render(<DefaultOrgSelector />, { wrapper: wrapper() }) | |||
| mocks.useIntersection.mockReturnValue({ isIntersecting: true }) | |||
|
|
|||
| const selectOrg = await screen.findByRole('button', { | |||
There was a problem hiding this comment.
yea this is fine. was scratching my head for a bit, but I get it now. This interaction is just trying to mimic real usage, but the test isn't doing that anyway.
There was a problem hiding this comment.
yeah that's what I figured lol. With the mock "mocking" what the button click would have done anyway maybe there was some interaction that was tripping over itself?
| @@ -359,12 +349,12 @@ describe('CommitsTab', () => { | |||
|
|
|||
| describe('when there is a next page', () => { | |||
| it('calls fetchNextPage', async () => { | |||
| const { fetchNextPage, user } = setup({ hasNextPage: true }) | |||
| const { fetchNextPage } = setup({ hasNextPage: true }) | |||
There was a problem hiding this comment.
yep same as the rest lol.
There was a problem hiding this comment.
This is really interesting to me. They all follow the pattern of:
- set mock isintersecting true at the start of a test
- do some unnecessary UI interaction
- assert the isintersecting had the desired result
Not really tracking why this had such a crazy effect on runtimes, but hey if it works, it works 😅
Description
Friday afternoon "chores" had me looking at the Test Analytics tool on Codecov for the Gazebo repo just because I hadn't in a while. I was surprised to see that some tests were taking over 5 seconds to run, with one being an even more abnormal "18 seconds."
Screenshot of initial test analytics page
I used the historical selector in the top to narrow my search to the last 7 days to see if it was just a blip of some weird data or if there was actually something abnormal going on here. To my surprise, even at 7 days I was seeing some similarly high test times and had to do a double check locally to see if I could reproduce those exact tests.
Screenshot of last 7 days data
Locally, I booted up the Commit.jsx test suite to see if I was also seeing some slow test suite run times since I wasn't completely convinced still. Lo and behold, I was seeing "13 seconds" as a pretty routine runtime for that suite. Using cursor, I literally just plugged in, any idea what I can do to make this test suite run faster? Cursor immediately spat back out, "yeah you probably don't need this mock," which after double checking I think is correct? if someone else could double check these aren't just poorly written tests, I think this just works? In any case, the test suite passed and I was able to do a similar fix for the rest of the tests that were
So yes, test analytics works, and hopefully we get a little extra speed from our test suite now as well! Oh and cursor works too I guess.
Screenshots of the test runs before and after
Link to Sample Entry
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.