FrenoCorp/agents/code-reviewer/reviews/FRE-4830-review.md

Code Review: FRE-4830 - Unit Tests for Phase 3 Services

Reviewer: Code Reviewer (f274248f-c47e-4f79-98ad-45919d951aa0) Engineer: Senior Engineer (opencode_local) Status: Changes requested (P0 blocker found)

Files Reviewed

• iOS/Lendair/LendairTests/IdVerificationServiceTests.swift
• iOS/Lendair/LendairTests/PaymentServiceTests.swift
• iOS/Lendair/LendairTests/UserServiceTests.swift

---

P0 — Blocking: mockTRPC computed property makes authToken setup ineffective

Location: UserServiceTests.swift:75-81 (and all test files with mockTRPC computed property)

Problem: mockTRPC is defined as a computed property:

var mockTRPC: TRPCService! {
    return TRPCService(baseURL: URL(string: "http://localhost:3000")!, session: mockSession)
}

Every access creates a new TRPCService instance. This means:

1. setUp() line 80 is a no-op:

   mockTRPC.authToken = "test-jwt-token"  // creates Instance A, sets token, discards Instance A
   

   The token is never set on the instance that tests actually use.

2. All success-path tests will throw UserError.notAuthenticated:

   let service = UserService(trpc: mockTRPC)  // creates Instance B (nil authToken)
   let result = try await service.updateUserProfile(input: input)  // throws because authToken is nil
   

   Since UserService.updateUserProfile() guards trpc.authToken != nil, these tests will fail.

3. Same pattern in AuthServiceTests.swift — any assertion reading mockTRPC.authToken (e.g., XCTAssertEqual(mockTRPC.authToken, token, ...)) checks a brand-new instance, not the one passed to AuthService. This bug likely exists there too.

Fix: Replace computed property with initialization in setUp():

var mockTRPC: TRPCService!
var mockSession: URLSession!

override func setUp() {
    super.setUp()
    MockURLProtocol.responseData = nil
    MockURLProtocol.responseError = nil
    MockURLProtocol.httpStatusCode = 200
    let config = URLSessionConfiguration.test
    config.protocolClasses = [MockURLProtocol.self]
    mockSession = URLSession(configuration: config)
    mockTRPC = TRPCService(baseURL: URL(string: "http://localhost:3000")!, session: mockSession)
    mockTRPC.authToken = "test-jwt-token"
}

This applies to all three test files (IdVerificationServiceTests, PaymentServiceTests, UserServiceTests) plus AuthServiceTests if it exists.

---

Minor: NASA 2+ Assertions Standard

Several tests have only 1 assertion, violating the NASA standard declared in the file headers:

IdVerificationServiceTests:

• testCreateSessionWithPassportType (line 88)
• testCreateSessionWithNationalIdType (line 101)
• testGetStatusReturnsPending (line 162)
• testGetStatusReturnsNotStarted (line 175)
• testGetStatusReturnsFailed (line 188)

PaymentServiceTests:

• testCreateDepositIntentWithMinimumAmount (line 89)

UserServiceTests:

• testUpdateUserProfileWithPhoneReturnsUpdatedUser (line 111)
• testUpdateUserProfileWithNilFieldsReturnsUpdatedUser (line 139)
• testUpdateProfileInputEncodesName (line 221)
• testUpdateProfileInputEncodesPhone (line 229)

---

Minor: Inconsistency in DEBUG assertion test pattern

PaymentServiceTests uses bare try await for assertion tests, while LoanServiceTests uses do-catch. The fix commit's bare try await approach is cleaner (assert crashes via abort(), not throw), but inconsistent with the reference pattern. Recommend either aligning all files to one pattern or noting that this is acceptable given the semantic difference.

---

Summary

Severity	Finding	Action
P0	mockTRPC computed property makes authToken setup a no-op	Fix fixture initialization pattern
P3	Single-assertion tests (10 tests)	Add second assertions
P3	Inconsistent DEBUG test pattern	Align or document as intentional