[No QA] Reduce unsafe type assertions in SearchUIUtils tests - #95904
Conversation
53f2732 to
b602e07
Compare
Target rule: @typescript-eslint/no-unsafe-type-assertion Target files: tests/unit/Search/SearchUIUtilsTest.ts Verification: completed Generated TSV restored: yes
b602e07 to
30bfd90
Compare
|
@blimpich The large diff is intentional: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30bfd90d92
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -716,7 +731,8 @@ const searchResultsGroupByCategory: OnyxTypes.SearchResults = { | |||
| }, | |||
| }; | |||
|
|
|||
| const reportActionListItems = [ | |||
| const reportActionListItems = createMock<ReportActionListItemType[]>([ | |||
| // @ts-expect-error -- these fixtures intentionally include the minimal report-action fields consumed by the search helpers. | |||
There was a problem hiding this comment.
In this createMock<ReportActionListItemType[]> call the fixtures are accepted as partial mocks, so omitting unused report-action fields does not produce an error on the following object literal. That makes this directive (and the same directives before the later report-action fixtures) report TS2578 as an unused @ts-expect-error, which blocks typecheck/CI before the tests can run; delete the suppressions or move to a construct that actually needs one.
Useful? React with 👍 / 👎.
|
🚀 Deployed to staging by https://github.com/blimpich in version: 9.4.34-0 🚀
|
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.4.34-14 🚀
|
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.4.34-14 🚀
Bundle Size Analysis (Sentry): |
3 participants
Explanation of Change
This PR addresses #94739 by removing the remaining @typescript-eslint/no-unsafe-type-assertion warnings from tests/unit/Search/SearchUIUtilsTest.ts. The cleanup keeps the existing SearchUIUtils test coverage intact while making fixture and result types explicit instead of forcing them with unsafe assertions.
What changed:
Why this approach is safe:
Reviewer focus:
Safety checks:
Fixed Issues
$ #94739
PROPOSAL:
Use checked TypeScript patterns in the SearchUIUtils unit tests: type fixtures at declaration time, narrow mixed SearchUIUtils results with existing type guards, and rely on jest.mocked/satisfies where those preserve intent without bypassing type checking.
Count change
Current baseline source:
Before:
After:
Net reduction:
TSV evidence:
Exact verification commands:
Tests
Run:
Verify the command succeeds and produces the expected evidence.
Run:
Verify the command succeeds and produces the expected evidence.
Run:
Verify the command succeeds and produces the expected evidence.
Verify the target count decreased from 136 to 0, and total warnings decreased from 5300 to 5164.
Offline tests
Not applicable: this cleanup changes only a TypeScript test file and has no network-dependent runtime behavior.
QA Steps
[No QA]
Not applicable: this is a test-only cleanup with no staging UI or production workflow change.
PR Author Checklist
For checklist items that are not applicable to this cleanup, checked means the item was considered and determined not applicable.
### Fixed Issuessection aboveTestssectionOffline testssection — Not applicable to this test-only cleanup.QA stepssection — Not applicable to this test-only cleanup.Avatar, I verified the components usingAvatarare working as expected) — Not applicable to this test-only cleanup.StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)) — Not applicable to this test-only cleanup.npm run compress-svg) — Not applicable to this test-only cleanup.Avataris modified, I verified thatAvataris working as expected in all cases) — Not applicable to this test-only cleanup.Designlabel and/or tagged@Expensify/designso the design team can review the changes. — Not applicable to this test-only cleanup.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps. — Not applicable to this test-only cleanup.Screenshots/Videos
Not applicable: no UI or visual behavior changed.