ref: Move useReposBackfilled off Repo Deprecated #2862
Conversation
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 #2862 +/- ##
=======================================
Coverage 98.48% 98.48%
=======================================
Files 879 879
Lines 12951 12957 +6
Branches 3468 3410 -58
=======================================
+ Hits 12755 12761 +6
Misses 192 192
Partials 4 4
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 #2862 +/- ##
=======================================
Coverage 98.48% 98.48%
=======================================
Files 879 879
Lines 12951 12957 +6
Branches 3407 3470 +63
=======================================
+ Hits 12755 12761 +6
Misses 192 192
Partials 4 4
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. Additional details and impacted files@@ Coverage Diff @@
## main #2862 +/- ##
=====================================
Coverage 98.49 98.49
=====================================
Files 879 879
Lines 12951 12957 +6
Branches 3473 3409 -64
=====================================
+ Hits 12755 12761 +6
Misses 192 192
Partials 4 4
Continue to review full report in Codecov by Sentry.
|
Bundle ReportChanges will increase total bundle size by 907 bytes ⬆️
|
Bundle ReportChanges will increase total bundle size by 907 bytes ⬆️
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #2862 +/- ##
=======================================
Coverage 98.48% 98.48%
=======================================
Files 879 879
Lines 12951 12957 +6
Branches 3473 3470 -3
=======================================
+ Hits 12755 12761 +6
Misses 192 192
Partials 4 4
Continue to review full report in Codecov by Sentry.
|
✅ Deploy preview for gazebo ready!Previews expire after 1 month automatically.
|
src/services/repo/hooks.spec.tsx
Outdated
| afterAll(() => server.close()) | ||
|
|
||
| console.error = () => {} |
There was a problem hiding this comment.
Can you mock this console in a setup, but also reset it after the test is finished, just to ensure that the console is back and running normally.
There was a problem hiding this comment.
Yep yep! Fixed in 5ef5686
| } | ||
|
|
||
| export function useRepoBackfilled() { | ||
| const { provider, owner, repo } = useParams<URLParams>() |
There was a problem hiding this comment.
m: We've shifted away from grabbing these in the service hooks directly and instead have moved to favouring them being passed in as args, do you mind making that quick update here?
There was a problem hiding this comment.
Definitely agree with you here, looking at this reference though:
gazebo/src/pages/RepoPage/FlagsTab/BackfillBanners/hooks.ts
Lines 3 to 4 in 9f3692f
There was a problem hiding this comment.
Can you make a ticket for this?
There was a problem hiding this comment.
Doing a quick search in the codebase, we have 54 hooks to update. I can go ahead and create an epic for that; though I don't really see this getting prioritized
| @@ -212,6 +221,7 @@ describe('useUpdateRepo', () => { | |||
| wrapper: wrapper(), | |||
| }) | |||
|
|
|||
| // @ts-expect-error | |||
There was a problem hiding this comment.
Interested as to what's going on here? is it just that the current hook that returns the mutation hasn't been uploaded to TS yet?
| }) | ||
| it('can return unsuccessful parse error', async () => { | ||
| setup({ isUnsuccessfulParseError: true }) | ||
| const { result } = renderHook(() => useRepoBackfilled(), { | ||
| wrapper: wrapper(), | ||
| }) | ||
|
|
||
| await waitFor(() => expect(result.current.isError).toBeTruthy()) | ||
| await waitFor(() => | ||
| expect(result.current.error).toEqual( | ||
| expect.objectContaining({ | ||
| status: 404, | ||
| }) | ||
| ) | ||
| ) | ||
| }) | ||
| it('can return not found error', async () => { | ||
| setup({ isNotFoundError: true }) | ||
| const { result } = renderHook(() => useRepoBackfilled(), { | ||
| wrapper: wrapper(), | ||
| }) | ||
|
|
||
| await waitFor(() => expect(result.current.isError).toBeTruthy()) | ||
| await waitFor(() => | ||
| expect(result.current.error).toEqual( | ||
| expect.objectContaining({ | ||
| status: 404, | ||
| }) | ||
| ) | ||
| ) | ||
| }) | ||
| it('can return owner not activated error', async () => { |
There was a problem hiding this comment.
Just me being a little picky, moving forward if you're working on tests do you mind adding in an empty line in between these test blocks, I feel like it makes things a lot cleaning when you're working around the tests and "mini-mizing" them in your editor.
There was a problem hiding this comment.
For sure, I think stylistic things like this could be brought up in our weekly meeting as well if we want to make team norms around them!
Description
Similar to other conversions in codecov/engineering-team#355, converts useReposBackfilled off RepoDeprecated, with some other small TS conversions and test fixes
Screenshots
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.