[No QA] Remove unsafe type assertions from test files - #97322
Conversation
8365118 to
e93b8f7
Compare
|
@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
|
ikevin127
left a comment
There was a problem hiding this comment.
🟢 LGTM
Test-only cleanup that removes 212 @typescript-eslint/no-unsafe-type-assertion violations across 10 test files.
I verified the supporting helpers (createMock, typeGuards, TestHelper.getRequiredWriteCall / getRequiredOnyxUpdate(s) / createGlobalFetchMock / assertFormDataMatchesObject) already exist in main and are used correctly, so this is genuinely a no-runtime-change PR.
Overall: Solid and safe to merge, no blocking issues ✅
| expect(result).toBe('{report:unknown}'); | ||
| }); | ||
|
|
||
| // FormulaContext.policy is OnyxEntry<Policy> (Policy | undefined), so missing-policy fixtures use undefined; null is outside the typed boundary. |
There was a problem hiding this comment.
Unnecessary comment
| // FormulaContext.policy is OnyxEntry<Policy> (Policy | undefined), so missing-policy fixtures use undefined; null is outside the typed boundary. |
There was a problem hiding this comment.
Fixed. removed the unnecessary comment.
| @@ -1,3 +1,5 @@ | |||
| import type ReportNameUtils = require('@libs/ReportNameUtils'); | |||
|
|
|||
There was a problem hiding this comment.
Why is there a newline here?
There was a problem hiding this comment.
Fixed. removed the unnecessary newline between the imports.
|
|
||
| const buildArgs = (overridePolicies?: OnyxCollection<Policy>, overrideReports?: OnyxCollection<Report>, transactionsUpdate?: OnyxCollection<Transaction> | null) => | ||
| [ | ||
| // Onyx-derived dependency values use undefined for absent entries; null is outside the typed transactions boundary. |
There was a problem hiding this comment.
Unnecessary comment
| // Onyx-derived dependency values use undefined for absent entries; null is outside the typed transactions boundary. |
There was a problem hiding this comment.
Fixed. removed the unnecessary comment.
blimpich
left a comment
There was a problem hiding this comment.
Very minor changes requested, but otherwise looks good 👍
|
Merge conflicts 😢 |
|
@blimpich Could you please take a look at this when you get a chance? Thanks! |
|
🚀 Deployed to staging by https://github.com/blimpich in version: 9.4.53-0 🚀
|
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.4.53-10 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
This test-only cleanup removes 212 unsafe type assertion violations from the ten requested test files while preserving test behavior and production runtime behavior. It uses production-linked types, typed mocks and fixtures, and narrow exceptions only for intentional malformed runtime fixtures. No application UI, production logic, CSS, assets, messaging, generic components, Storybook stories, or deeplink behavior changed.
Fixed Issues
$ #94739
PROPOSAL: #94739 (comment)
Count change
Current baseline source:
config/eslint/eslint.seatbelt.tsvat the exact run base/worktree, SHA-256960dda33f77a4eaa80d8760fc635a7ac156edb0e2638cf80acf89c0933ad2bd3.Before:
tests/actions/DomainTest.ts: 20 violations.tests/actions/IOUTest/SplitTest.ts: 20 violations.tests/ui/UnreadIndicatorsTest.tsx: 20 violations.tests/unit/FormulaTest.ts: 21 violations.tests/unit/MiddlewareTest.ts: 20 violations.tests/unit/ReportNameUtilsTest.ts: 23 violations.tests/unit/WorkspacesSettingsUtilsTest.ts: 23 violations.tests/unit/reportAttributesTest.ts: 24 violations.tests/unit/resolveChatTargetTest.ts: 20 violations.tests/unit/useTransactionViolationsTest.ts: 21 violations.After:
tests/actions/DomainTest.ts: 0 violations.tests/actions/IOUTest/SplitTest.ts: 0 violations.tests/ui/UnreadIndicatorsTest.tsx: 0 violations.tests/unit/FormulaTest.ts: 0 violations.tests/unit/MiddlewareTest.ts: 0 violations.tests/unit/ReportNameUtilsTest.ts: 0 violations.tests/unit/WorkspacesSettingsUtilsTest.ts: 0 violations.tests/unit/reportAttributesTest.ts: 0 violations.tests/unit/resolveChatTargetTest.ts: 0 violations.tests/unit/useTransactionViolationsTest.ts: 0 violations.Net reduction:
@typescript-eslint/no-unsafe-type-assertionviolations.TSV evidence:
960dda33f77a4eaa80d8760fc635a7ac156edb0e2638cf80acf89c0933ad2bd3.Tests
git diff --checkalso passed.src/App.tsxdiagnostic.Manual JS-console verification: N/A — this is test-only cleanup with no application runtime change.
Offline tests
N/A — this is test-only type cleanup; no network, offline, or synchronization behavior changed.
QA Steps
N/A — this is test-only cleanup with no staging or production behavior change. The title starts with
[No QA].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
N/A — no UI changes, so screenshots and videos are not applicable.
Changed files (10)
tests/actions/DomainTest.tstests/actions/IOUTest/SplitTest.tstests/ui/UnreadIndicatorsTest.tsxtests/unit/FormulaTest.tstests/unit/MiddlewareTest.tstests/unit/ReportNameUtilsTest.tstests/unit/WorkspacesSettingsUtilsTest.tstests/unit/reportAttributesTest.tstests/unit/resolveChatTargetTest.tstests/unit/useTransactionViolationsTest.ts