[No QA] Clean up unsafe type assertions in tests - #97510
Conversation
|
@ikevin127 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Reviewer Checklist
|
| ...(name === TEST_CASES.hasSyncErrors.name | ||
| ? { | ||
| quickbooksOnline: { | ||
| quickbooksOnline: createMock<NonNullable<NonNullable<Policy['connections']>[typeof CONST.POLICY.CONNECTIONS.NAME.QBO]>>({ |
There was a problem hiding this comment.
🟡 DRY: useWorkspacesTabIndicatorStatusTest.ts - Use the exported Connections map instead of indexing into Policy
The connection fixtures repeat this ~130-char extraction 3+ times:
createMock<NonNullable<NonNullable<Policy['connections']>[typeof CONST.POLICY.CONNECTIONS.NAME.QBO]>>({ ... })Connections is exported from @src/types/onyx/Policy (export list at Policy.ts:2883), so this can read from the map type directly and drop one NonNullable layer:
import type {Connections} from '@src/types/onyx/Policy';
createMock<NonNullable<Connections[typeof CONST.POLICY.CONNECTIONS.NAME.QBO]>>({ ... })Same type, less indexing gymnastics, and it leans on the exported type the way the rest of the series does. Low severity, but since it repeats it's worth tidying ✅
| route={createMock<PlatformStackScreenProps<MoneyRequestNavigatorParamList, typeof SCREENS.MONEY_REQUEST.STEP_SCAN>['route']>({ | ||
| key: 'StepScan', | ||
| name: SCREENS.MONEY_REQUEST.STEP_SCAN, | ||
| params: { | ||
| action: CONST.IOU.ACTION.CREATE, | ||
| iouType: CONST.IOU.TYPE.SUBMIT, | ||
| reportID: REPORT_ID, | ||
| transactionID: TRANSACTION_ID_1, | ||
| backTo: ROUTES.MONEY_REQUEST_STEP_CONFIRMATION.route, | ||
| pageIndex: 0, | ||
| }, | ||
| })} |
There was a problem hiding this comment.
🟡 CONSISTENCY: IOURequestStepScanTest.tsx: Use ScanRoute for both renders
The second render uses the exported ScanRoute, but the first still spells out createMock<PlatformStackScreenProps<MoneyRequestNavigatorParamList, typeof SCREENS.MONEY_REQUEST.STEP_SCAN>['route']>.
ScanRoute is PlatformStackRouteProp<..., STEP_SCAN | CREATE> (IOURequestStepScan/types.ts:27), so it already covers the STEP_SCAN case, both route props can be createMock<ScanRoute>(...).
Right now the same prop is typed two different ways in one file, which reads like an oversight.
(The navigation props can stay as-is)
ikevin127
left a comment
There was a problem hiding this comment.
🟢 LGTM - Two DRY / CONSISTENCY non-blocking comments ☝️
Solid cleanup, and the highest-stakes part (the shared tests/utils/TestHelper.ts) is done well. I ran all 14 executable suites against the checked-out head (a41c5193254, the PR's exact head): everything passes.
Two low-severity nits below; nothing blocks ✅
Explanation of Change
Removed
@typescript-eslint/no-unsafe-type-assertionviolations from 15 test and test-helper files as part of Expensify/App issue #94739.What changed:
Why this approach is safe:
BottomTabBarTestfailure.CREATEroute shape while replacing a fully unchecked cast.a41c5193254679009ec4e2b4f63a66df8c219d49.Fixed Issues
$ #94739
PROPOSAL: #94739 (comment)
Count change
Current baseline source:
config/eslint/eslint.seatbelt.tsvat pinned base8960aa81fd37e54ae2d1493a3c9230b42751814bBefore:
tests/actions/IOU/RequestMoneyTest.ts: 5 violationstests/navigation/createDynamicRouteTests.ts: 5 violationstests/ui/BottomTabBarTest.tsx: 5 violationstests/ui/IOURequestStepScanTest.tsx: 5 violationstests/ui/MoneyRequestReportFooter.tsx: 5 violationstests/unit/AgentZeroStatusContextTest.ts: 5 violationstests/unit/BaseSelectionListSectionsTest.tsx: 5 violationstests/unit/CIGitLogicTest.ts: 5 violationstests/unit/TransactionNavigationUtilsTest.ts: 6 violationstests/unit/TravelInvoicingUtilsTest.ts: 6 violationstests/unit/useAutoUpdateTimezoneTest.tsx: 6 violationstests/unit/useCurrentReportIDTest.tsx: 6 violationstests/unit/useGetExpensifyCardFromReportActionTest.ts: 6 violationstests/unit/useWorkspacesTabIndicatorStatusTest.ts: 6 violationstests/utils/TestHelper.ts: 6 violationsAfter:
tests/actions/IOU/RequestMoneyTest.ts: 0 violationstests/navigation/createDynamicRouteTests.ts: 0 violationstests/ui/BottomTabBarTest.tsx: 0 violationstests/ui/IOURequestStepScanTest.tsx: 0 violationstests/ui/MoneyRequestReportFooter.tsx: 0 violationstests/unit/AgentZeroStatusContextTest.ts: 0 violationstests/unit/BaseSelectionListSectionsTest.tsx: 0 violationstests/unit/CIGitLogicTest.ts: 0 violationstests/unit/TransactionNavigationUtilsTest.ts: 0 violationstests/unit/TravelInvoicingUtilsTest.ts: 0 violationstests/unit/useAutoUpdateTimezoneTest.tsx: 0 violationstests/unit/useCurrentReportIDTest.tsx: 0 violationstests/unit/useGetExpensifyCardFromReportActionTest.ts: 0 violationstests/unit/useWorkspacesTabIndicatorStatusTest.ts: 0 violationstests/utils/TestHelper.ts: 0 violationsNet reduction:
@typescript-eslint/no-unsafe-type-assertionviolations across 15 files.TSV evidence:
main.Tests
git diff --checkand verify the tracked Seatbelt TSV remains byte-identical.30613219884passed typecheck, lint, format, React Compiler, and Jest for commita41c5193254679009ec4e2b4f63a66df8c219d49.BottomTabBarTestfailure was reproduced identically on untouched pinned base8960aa81fd37e54ae2d1493a3c9230b42751814b; exact-head Fork CI Jest passed.Failure-scenario coverage is supplied by the existing focused Jest scenarios. Browser/device console checks and manual platform execution are not applicable to this test-only change.
Offline tests
Not applicable because this is a test-only type-safety cleanup with no production network, storage, or offline behavior changes.
QA Steps
Not applicable for staging or production QA because production behavior and user-interface code are unchanged. Validation is fully automated by focused verification, independent review, and exact-head Fork CI.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Not applicable on Android Native, Android mWeb Chrome, iOS Native, iOS mWeb Safari, or MacOS Chrome/Safari because this PR changes only test implementation and has no visual output.
Changed files (15)
tests/actions/IOU/RequestMoneyTest.tstests/navigation/createDynamicRouteTests.tstests/ui/BottomTabBarTest.tsxtests/ui/IOURequestStepScanTest.tsxtests/ui/MoneyRequestReportFooter.tsxtests/unit/AgentZeroStatusContextTest.tstests/unit/BaseSelectionListSectionsTest.tsxtests/unit/CIGitLogicTest.tstests/unit/TransactionNavigationUtilsTest.tstests/unit/TravelInvoicingUtilsTest.tstests/unit/useAutoUpdateTimezoneTest.tsxtests/unit/useCurrentReportIDTest.tsxtests/unit/useGetExpensifyCardFromReportActionTest.tstests/unit/useWorkspacesTabIndicatorStatusTest.tstests/utils/TestHelper.ts