[No QA] Remove unsafe type assertions from ten test files - #97285
Open
KJ21-ENG wants to merge 1 commit into
Open
[No QA] Remove unsafe type assertions from ten test files#97285KJ21-ENG wants to merge 1 commit into
KJ21-ENG wants to merge 1 commit into
Conversation
KJ21-ENG
changed the title
KJ21-ENG
force-pushed
the
seatbelt/94739-tests-20260728-3
branch
from
July 30, 2026 18:00
3ec4b84 to
7844373
Compare
KJ21-ENG
marked this pull request as ready for review
July 31, 2026 14:27
|
@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] |
Contributor
Reviewer Checklist
|
ikevin127
reviewed
Aug 1, 2026
|
|
||
| it('correctly handles reportNextStep parameter when moving transactions to unreported report', async () => { | ||
| const mockAPIWrite = jest.spyOn(require('@libs/API'), 'write').mockImplementation(() => Promise.resolve()); | ||
| // eslint-disable-next-line rulesdir/no-multiple-api-calls -- Spy on API.write to inspect the generated failure data. |
Contributor
There was a problem hiding this comment.
🟡 tests/unit/TransactionTest.ts: The rulesdir/no-multiple-api-calls suppression is applied inconsistently across the jest.spyOn(API, 'write') sites.
The first converted site (correctly handles reportNextStep parameter when moving transactions between reports) has no disable comment, while the later identical spies all got // eslint-disable-next-line rulesdir/no-multiple-api-calls. If lint is green, one of the two forms is unnecessary.
Please make them consistent so a future reader doesn't assume the first site is special.
ikevin127
reviewed
Aug 1, 2026
Comment on lines
+40
to
+51
| expect(typeof session.authToken).toBe('string'); | ||
| expect(typeof session.encryptedAuthToken).toBe('string'); | ||
| expect(typeof preservedUserSession.authToken).toBe('string'); | ||
| expect(typeof preservedUserSession.encryptedAuthToken).toBe('string'); | ||
| if ( | ||
| typeof session.authToken !== 'string' || | ||
| typeof session.encryptedAuthToken !== 'string' || | ||
| typeof preservedUserSession.authToken !== 'string' || | ||
| typeof preservedUserSession.encryptedAuthToken !== 'string' | ||
| ) { | ||
| throw new Error('Expected masked session tokens'); | ||
| } |
Contributor
There was a problem hiding this comment.
🟢 tests/unit/ExportOnyxStateTest.ts: The expect(typeof x).toBe('string') immediately followed by if (typeof x !== 'string') { throw ... } pair is repeated ~10 times.
The if/throw is needed for TS narrowing (expect doesn't narrow), but the two together are verbose. A tiny typed assertion helper collapses each pair to one line and reads better:
function assertString(value: unknown): asserts value is string {
expect(typeof value).toBe('string');
if (typeof value !== 'string') {
throw new Error('Expected a string');
}
}
// usage
assertString(session.authToken);
expect(session.authToken).toHaveLength('sensitive-auth-token'.length);Not blocking, just noise reduction if you touch this file again.
ikevin127
reviewed
Aug 1, 2026
| (useCardFeeds as jest.Mock).mockReturnValue([{}, {status: 'loaded'}]); | ||
| mockedUseCardFeeds.mockReturnValue([{}, loadedResultMetadata, undefined, emptyCardFeedStatuses, mockWorkspaceAccountID]); | ||
| const {result, rerender} = renderHook(() => useIsBlockedToAddFeed(mockPolicyID)); | ||
| expect(result.current.isBlockedToAddNewFeeds).toBe(false); |
Contributor
There was a problem hiding this comment.
🟢 tests/unit/hooks/useIsBlockedToAddFeed.test.ts: Fixtures moved from literal keys ('plaid.ins_19', 'csv#123456') to computed getCardFeedWithDomainID(FEED_BANK_NAME.MOCK_BANK / CSV_CLASSIC, mockWorkspaceAccountID), and several feed fields were dropped (shouldApplyCashbackToBill, the Expensify settlement block, uploadLayoutSettings: [] → {}).
This is behavior-preserving and more realistic, but the assertions (isBlockedToAddNewFeeds toBe(true)) only stay meaningful if MOCK_BANK still classifies as a real, non-CSV / non-Expensify feed.
Worth a quick sanity confirm since the whole point of those cases is "a real feed exists."
Contributor
|
@KJ21-ENG (3) comments above to be addressed before approval 🙌 |
This was referenced Aug 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
This issue 94739 cleanup removes unsafe type assertions from ten test files. It replaces unsafe assertions with production-linked types and typed mocks, while preserving the intentional malformed persisted-data fixture through a localized TypeScript suppression. No production source files were changed.
Fixed Issues
$ #94739
PROPOSAL: #94739 (comment)
Count change
Current baseline source:
config/eslint/eslint.seatbelt.tsvBefore:
tests/actions/SessionTest.ts: 12 violationstests/ui/NetSuiteCustomListSelectorPageTest.tsx: 12 violationstests/unit/DebugUtilsTest.ts: 11 violationstests/unit/ExportOnyxStateTest.ts: 12 violationstests/unit/IOUUtilsTest.ts: 11 violationstests/unit/RequestConflictUtilsTest.ts: 11 violationstests/unit/TransactionTest.ts: 12 violationstests/unit/hooks/useGettingStartedItems.test.ts: 11 violationstests/unit/hooks/useIsBlockedToAddFeed.test.ts: 11 violationstests/unit/pages/home/GettingStartedSection/GettingStartedSectionTest.tsx: 11 violationsAfter:
Net reduction:
TSV evidence:
Tests
Deterministic cleanup verification reported zero findings for all ten authorized targets. Focused formatter and individually scoped lint checks passed for all ten targets. The executable focused Jest command covering all ten targets passed through the verifier-owned worktree resolver. Verifier-owned focused TypeScript was unavailable only because it reported an unrelated
src/components/AddressSearch/index.tsxdiagnostic; no authorized-path diagnostic was reported. Fresh exact-head Fork CI run30568612660passed typecheck, lint, format, React Compiler, and Jest for rebuilt head7844373e49573745ca3ea092cb996734ad0c1aae. No manual platform testing was performed.Offline tests
N/A — this is a test-only type-safety cleanup; no application runtime or network behavior changed.
QA Steps
N/A — this test-only cleanup requires no staging or manual QA testing; the title uses the
[No QA]prefix.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 — test-only cleanup; no UI changes or platform screenshots were required.
Changed files (10)
tests/actions/SessionTest.tstests/ui/NetSuiteCustomListSelectorPageTest.tsxtests/unit/DebugUtilsTest.tstests/unit/ExportOnyxStateTest.tstests/unit/IOUUtilsTest.tstests/unit/RequestConflictUtilsTest.tstests/unit/TransactionTest.tstests/unit/hooks/useGettingStartedItems.test.tstests/unit/hooks/useIsBlockedToAddFeed.test.tstests/unit/pages/home/GettingStartedSection/GettingStartedSectionTest.tsx