Project: doctolib-doctolib · Branch: .invalid · Mode: · Model: claude-sonnet-4-6 Stats: 1 user messages, 43 tool calls, tools used: Read, Bash. Conversation Turn 1 — User Analyze test coverage for PR #241133 (FHIR-2224) in the doctolib/doctolib monorepo. Context: This PR changes DmpApi.updateDocumentStatus from throwing on failure to returning Promise with an optional error field. Test files to analyze (in worktree /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/): - packages/@doctolib/dmp/src/business/DmpApi.test.ts - packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts Key scenarios to verify coverage for: In DmpApi.test.ts: 1. Legacy path (no dmpService): success returns {} 2. Legacy path: non-OK status returns structured error with all fields from response (apiErrorCode, apiErrorType, apiErrorContext, apiErrorDescription, apiErrorExtendedInformations, apiErrorTypeDescription, status) 3. Legacy path: missing session throws 4. dmpService path: success returns {} 5. dmpService path: BusinessError returns { error: { status: 'ERROR', apiErrorDescription: e.message } } 6. dmpService path: non-BusinessError still throws (exception propagation test) 7. s_status is null/undefined — does response.s_status ?? 'ERROR' get tested? In DmpNhrAdapter.test.ts: 1. Structured error from updateDocumentStatus → error payload with message 2. apiErrorDescription only → message = description 3. Both apiErrorDescription + apiErrorExtendedInformations → combined message 4. Neither field present → fallback 'DMP API error' 5. Thrown exception (not structured error) → catch path still works 6. Partial success (some succeed, some throw) — existing test updated? Missing tests to flag: - Is apiErrorCode, apiErrorType, apiErrorContext, apiErrorTypeDescription being explicitly tested for correct mapping in the legacy path? - Is the non-BusinessError throw-through tested for the dmpService path? - Is s_status null/undefined case tested? Please read the actual test files and report gaps with Conventional Comments format and file:line references. Turn 1 — Assistant I'll analyze the test files for PR #241133. Let me read them both in parallel. Tools used: - Read: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.test.ts Tools used: - Read: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.test.ts Tool results: → result (ok): 1 import { getPatientInsForDMP } from '@doctolib/dmp' 2 import type { DmpApi, DmpDocumentContent, SearchResult as DmpSearchResult } from '@doctolib/dmp' 3 import type { NhrDocumentSearchProps } from '@doctolib/tpf-national-health-records' 4 import type { DocumentCategorizationConcept, DocumentCategorizationFacet } from '@doctolib/documents-ui/types/Document' 5 import { DmpNhrAdapter } from './DmpNhrAdapter' 6 import { findFacetForCDAType } from '../utils/findFacetForCDAType' 7 8 jest.mock('../u …(truncated, 26876 more chars) Tool results: → result (ok): 1 import type { Patient } from 'monolith_DEPRECATED/types/doctor/api/Patient' 2 import { ConsultationDocumentKind } from 'monolith_DEPRECATED/types/doctor/api/ConsultationDocument' 3 import { getLoincCode } from 'monolith_DEPRECATED/models/document/document' 4 import { blobToBase64 } from 'monolith_DEPRECATED/shared/BlobHelper' 5 import { FeatureSwitchMocker } from '@doctolib/variation_management' 6 import { DmpAction, BusinessError } from '@doctolib/tpf-french-teleservices' 7 import { parse as …(truncated, 85045 more chars) Now let me read the actual implementation file to understand the exact code paths being tested. Tools used: - Bash: find /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business -name "DmpApi.ts" 2>/dev/null Tool results: → result (ok): /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.ts Tools used: - Bash: find /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/national-health-records/src/business/adapters -name "DmpNhrAdapter.ts" 2>/dev/null Tool results: → result (ok): /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.ts Tools used: - Bash: grep -n "updateDocumentStatus" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.ts Tool results: → result (ok): 36: updateDocumentStatus as updateDocumentStatusCommand, 346: async updateDocumentStatus(props: DocumentStatusUpdateProps): Promise { 376: const response = await updateDocumentStatusCommand({ Tools used: - Bash: grep -n "updateDocumentStatus\|DocumentStatusUpdateResult\|BusinessError\|s_status\|apiError" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/national-health-record …(truncated, 40 more chars) Tool results: → result (ok): 221: const result = await this.dmpApi.updateDocumentStatus({ Tools used: - Read: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.ts (340–420) Tool results: → result (ok): 340 341 const { sessionId } = cachedSession 342 const response = await getDocumentsContent({ sessionId, ...props }) 343 return response.Documents.map(this.toDocumentContent) 344 } 345 346 async updateDocumentStatus(props: DocumentStatusUpdateProps): Promise { 347 const { ins, documentUniqueId: uniqueId, uuid, healthCareSettings, getTransactionFrames } = props 348 349 if (this.dmpService) { 350 const payload = buildUpdateDocumentStatusP …(truncated, 2336 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/national-health-records/src/business/adapters/DmpNhrAdapter.ts (200–280) Tool results: → result (ok): 200 const typeCode = doc.cdaContent?.code?._code 201 const facet = findFacetForCDAType(typeCode, this.documentCategorizationFacets) 202 return toNhrDocumentContentResult(doc, facet) 203 }) 204 } 205 206 async updateDocumentMetadata(props: NhrDocumentMetadataUpdateProps): Promise { 207 if (!props.patient?.nationalHealthIdentifier) { 208 throw new Error('patient_national_health_identifier_required') 209 } 210 211 const …(truncated, 2378 more chars) Now let me also check the DmpApiError class in the real @doctolib/dmp package to verify the mock inline implementation matches: Tools used: - Bash: grep -rn "DmpApiError\|class DmpApiError" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/ 2>/dev/null | head -20 Tool results: → result (ok): /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/index.ts:83:export { DmpApiError } from './api/DmpApiError' /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/index.ts:84:export type { DmpApiErrorInfo } from './types/DmpApi' /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/types/DmpApi.ts:171:export interface DmpApiErrorInfo { /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-222 …(truncated, 2595 more chars) Tools used: - Bash: grep -n "BusinessError\|'The teleservice returned a business error'\|apiErrorDescription.e\.message\|e\.message.apiError" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@d …(truncated, 34 more chars) Tool results: → result (ok): 12:import { DmpService, DmpAction, BusinessError } from '@doctolib/tpf-french-teleservices' 325: if (e instanceof BusinessError) { 362: if (e instanceof BusinessError) { 363: return { error: { status: 'ERROR', apiErrorDescription: e.message } } 453: if (e instanceof BusinessError) { 670: if (e instanceof BusinessError) { 736: // On error, BusinessError propagates to the outer catch which Tools used: - Read: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/api/DmpApiError.ts Tool results: → result (ok): 1 import type { DmpApiErrorInfo } from '../types/DmpApi' 2 3 export class DmpApiError extends Error { 4 readonly apiErrorCode: string | undefined 5 6 readonly apiErrorType: string | undefined 7 8 readonly apiErrorContext: string | undefined 9 10 readonly apiErrorDescription: string | undefined 11 12 readonly apiErrorExtendedInformations: string | undefined 13 14 readonly apiErrorTypeDescription: string | undefined 15 16 readonly status: string 17 18 constructor(info: DmpA …(truncated, 730 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/types/DmpApi.ts (160–200) Tool results: → result (ok): 160 serviceStopDateTop?: string 161 submissionDateBottom?: string 162 submissionDateTop?: string 163 status?: number 164 visibility?: number 165 disableMetadataSearch?: number 166 referenceIds?: DmpReferenceId[] 167 eventCodes?: DmpEventCode[] 168 getTransactionFrames?: number 169 } 170 171 export interface DmpApiErrorInfo { 172 apiErrorCode?: string 173 apiErrorType?: string 174 apiErrorContext?: string 175 apiErrorDescription?: string 176 apiErrorExtendedInformatio …(truncated, 568 more chars) Now I have a complete picture. Let me do one final check on the BusinessError message in the test vs. the implementation: Tools used: - Bash: grep -n "The teleservice returned a business error\|BusinessError\b" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.ts | head -10 Tool results: → result (ok): 12:import { DmpService, DmpAction, BusinessError } from '@doctolib/tpf-french-teleservices' 325: if (e instanceof BusinessError) { 362: if (e instanceof BusinessError) { 453: if (e instanceof BusinessError) { 670: if (e instanceof BusinessError) { 736: // On error, BusinessError propagates to the outer catch which Tools used: - Bash: grep -n "The teleservice returned a business error" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.ts Tool results: → result (ok): (Bash completed with no output) Tools used: - Bash: grep -n "The teleservice returned a business error" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.test.ts Tool results: → result (ok): 2058: expect.objectContaining({ status: 'ERROR', apiErrorDescription: 'The teleservice returned a business error' }) Tools used: - Bash: grep -n "class BusinessError" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/ -r 2>/dev/null || grep -rn "class BusinessError" /Users/USER/workspace/d …(truncated, 68 more chars) Tool results: → result (ok): (Bash completed with no output) Tools used: - Bash: find /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/node_modules/@doctolib/tpf-french-teleservices -name ".d.ts" 2>/dev/null | head -5 Tool results: → result (ok): (Bash completed with no output) Tools used: - Bash: find /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224 -path "/tpf-french-teleservices" -name ".d.ts" 2>/dev/null | head -5 Tools used: - Bash: grep -rn "new BusinessError\|BusinessError(" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.test.ts | head -5 Tool results: → result (ok): /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.test.ts:1998: mockDmpService.call.mockRejectedValue(new BusinessError('error')) /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.test.ts:2048: mockDmpService.call.mockRejectedValue(new BusinessError('DMP returned error')) Tool results: → result (ok): (Bash completed with no output) Tools used: - Read: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.test.ts (2047–2062) Tool results: → result (ok): 2047 it('should return error result when dmpService throws BusinessError', async () => { 2048 mockDmpService.call.mockRejectedValue(new BusinessError('DMP returned error')) 2049 2050 const dmpApi = createDmpApiWithService() 2051 const result = await dmpApi.updateDocumentStatus({ 2052 ins: '123456789012345678901', 2053 documentUniqueId: 'unique-doc-123', 2054 uuid: 'uuid-doc-456', 2055 healthCareSettings: 'SA07', 2056 …(truncated, 212 more chars) This is a critical finding. The test passes new BusinessError('DMP returned error') but expects apiErrorDescription: 'The teleservice returned a business error' — a hardcoded string that doesn't match e.message. Let me verify what the implementation actually does: Tools used: - Read: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.ts (358–370) Tool results: → result (ok): 358 try { 359 await this.dmpService.call(DmpAction.UpdateDocumentStatus, payload) 360 return {} 361 } catch (e) { 362 if (e instanceof BusinessError) { 363 return { error: { status: 'ERROR', apiErrorDescription: e.message } } 364 } 365 366 throw e 367 } 368 } 369 The implementation uses e.message directly. The test asserts 'The teleservice returned a business error' but throws new BusinessError('DMP returned error'). This means one of two things: either BusinessError ignores its constructor argument and produces a fixed message, or the test assertion will fail. Let me investigate: Tools used: - Bash: find /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224 -name "package.json" -path "/tpf-french-teleservices" 2>/dev/null | head -3 Tool results: → result (ok): (Bash completed with no output) Tools used: - Bash: find /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224 -name "__mocks__" -type d 2>/dev/null | head -5 Tool results: → result (ok): /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/jestUtils/__mocks__ /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/sick-notes-de-fulfillment/src/__mocks__ /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/desktop/tests/__mocks__ /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/ehr_paper_forms/src/__mocks__ /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/sick-notes …(truncated, 14 more chars) Tools used: - Bash: grep -rn "BusinessError\|tpf-french-teleservices" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/jest.config.js 2>/dev/null || find /Users/USER/workspace/ …(truncated, 133 more chars) Tool results: → result (ERROR): Exit code 1 Tools used: - Bash: cat /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/jest.config.js 2>/dev/null || cat /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/ …(truncated, 40 more chars) Tool results: → result (ERROR): Exit code 1 Tools used: - Bash: find /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp -name "jest.config" 2>/dev/null Tool results: → result (ok): /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/jest.config.cjs Tools used: - Read: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/jest.config.cjs Tool results: → result (ok): 1 const base = require('../../../jest.config') 2 const { name } = require('./package.json') 3 4 module.exports = { 5 ...base, 6 displayName: name, 7 rootDir: __dirname, 8 roots: ['/src/'], 9 testEnvironment: '@happy-dom/jest-environment', 10 reporters: [ 11 'default', 12 'jest-junit', 13 [ 14 'jest-sonar', 15 { 16 reportedFilePath: 'absolute', 17 }, 18 ], 19 ], 20 collectCoverageFrom: [ 21 'src//.{ts,tsx,js,jsx}', 22 …(truncated, 85 more chars) Tools used: - Bash: grep -n "tpf-french-teleservices\|moduleNameMapper" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/jest.config.js 2>/dev/null | head -20 Tool results: → result (ok): 6:const moduleNameMapperWithoutDoctolibDeps = require('./jestUtils/moduleNameMapperWithoutDoctolibDeps') 90: moduleNameMapper: { 96: ...moduleNameMapperWithoutDoctolibDeps(compilerOptions, packageJson), Tools used: - Bash: grep -n "tpf-french-teleservices\|BusinessError" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/jestUtils/moduleNameMapperWithoutDoctolibDeps.js 2>/dev/null | head -10 Tool results: → result (ok): (Bash completed with no output) Tools used: - Bash: find /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/jestUtils/__mocks__ -type f 2>/dev/null | head -10 Tool results: → result (ok): /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/jestUtils/__mocks__/svgIconMock.jsx /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/jestUtils/__mocks__/fileMock.js /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/jestUtils/__mocks__/styleMock.js Tools used: - Bash: find /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224 -path "/__mocks__/@doctolib/tpf-french-teleservices" 2>/dev/null | head -5 Tool results: → result (ok): (Bash completed with no output) Tools used: - Bash: find /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224 -name "tpf-french-teleservices" -type d 2>/dev/null | head -5 Tool results: → result (ok): (Bash completed with no output) Tools used: - Bash: find /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages -name "package.json" -path "tpf-french" 2>/dev/null | head -5 Tool results: → result (ok): (Bash completed with no output) Tools used: - Bash: grep -rn "tpf-french-teleservices" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/package.json 2>/dev/null Tool results: → result (ok): /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/package.json:56: "@doctolib/tpf-french-teleservices": "^7.0.1", Tools used: - Bash: find /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224 -path "/node_modules/@doctolib/tpf-french-teleservices/dist" -name ".js" 2>/dev/null | head -3 Tool results: → result (ok): (Bash completed with no output) Tools used: - Bash: find /Users/USER/workspace/doctolib -name "package.json" -path "/tpf-french-teleservices" 2>/dev/null | head -5 Tool results: → result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-french-teleservices/package.json /Users/USER/workspace/doctolib/treatment-plan/.claude/worktrees/stupefied-lewin/packages/tpf-french-teleservices/package.json /Users/USER/workspace/doctolib/worktrees/doctolib/fhir-2189_class-code-label-map/node_modules/@doctolib/tpf-french-teleservices/package.json /Users/USER/workspace/doctolib/treatment-plan/.claude/worktrees/friendly-lalande/packages/tpf-french-teleservices/package.json /Users/USER/wo …(truncated, 136 more chars) Tools used: - Bash: grep -n "BusinessError\|class BusinessError" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-french-teleservices/src/errors.ts 2>/dev/null || find /Users/USER/workspace/doctolib/t …(truncated, 125 more chars) Tool results: → result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-french-teleservices/src/core/call.ts /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-french-teleservices/src/core/auth-errors.ts /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-french-teleservices/src/core/auth-errors.test.ts Tools used: - Bash: grep -n "class BusinessError\|BusinessError" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-french-teleservices/src/core/call.ts | head -15 Tool results: → result (ok): 26: - BusinessError: teleservice executed but returned s_status ERROR (thrown by buildResponse()) Tools used: - Bash: find /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-french-teleservices/src -name ".ts" 2>/dev/null | xargs grep -ln "export class BusinessError" 2>/dev/null Tool results: → result (ok): /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-french-teleservices/src/core/auth-errors.ts Tools used: - Bash: grep -A 10 "export class BusinessError" /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-french-teleservices/src/core/auth-errors.ts Tool results: → result (ok): export class BusinessError extends FrenchTeleserviceError { constructor(serviceName: string, message?: string) { super(message ?? 'The teleservice returned a business error', 'BUSINESS_ERROR', serviceName) this.name = 'BusinessError' } } Now the picture is clear. BusinessError takes (serviceName, message?) — the first argument is serviceName, not the message. When the test calls new BusinessError('DMP returned error'), the message parameter is omitted and defaults to 'The teleservice returned a business error'. So the test assertion is actually correct — it just uses a misleading constructor invocation. But this reveals a subtle test quality issue. Let me now compile the full analysis. Tools used: - Bash: grep -n "s_status.null\|null.s_status\|s_status ?? 'ERROR'\|undefined.s_status" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.ts Tool results: → result (ok): 394: status: response.s_status ?? 'ERROR', 591: status: response.s_status ?? 'ERROR', Tools used: - Bash: grep -n "apiErrorContext\|apiErrorTypeDescription\|apiErrorType\b" /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.test.ts Tool results: → result (ok): 1353: i_apiErrorType: '6000', 1354: s_apiErrorContext: 'DmpConnect-JS2 server', 1357: s_apiErrorTypeDescription: 'Software error', 1366: apiErrorType: '6000', 1367: apiErrorContext: 'DmpConnect-JS2 server', 1370: apiErrorTypeDescription: 'Software error', 1387: apiErrorType: undefined, 1388: apiErrorContext: undefined, 1391: apiErrorTypeDescription: undefined, 1647: i_apiErrorType: 'TECHNICAL', 1663: apiErrorType: 'TE …(truncated, 9 more chars) Tools used: - Read: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.test.ts (1642–1672) Tool results: → result (ok): 1642 it('should return error result when iCanopée returns a non-OK status', async () => { 1643 getCachedDmpSessionMocked.mockResolvedValue({ sessionId: 'session-abc', healthcareSettings: 'SA07' }) 1644 updateDocumentStatusCommandMocked.mockResolvedValue({ 1645 s_status: 'ERROR_42', 1646 i_apiErrorCode: '42', 1647 i_apiErrorType: 'TECHNICAL', 1648 s_apiErrorDescription: 'Something went wrong', 1649 }) 1650 1651 const dmpApi = new DmpApi …(truncated, 678 more chars) Now I have a complete picture of the test coverage. Here is my full analysis: --- Summary The tests for PR #241133 cover the primary behavioral contract of the DmpApi.updateDocumentStatus refactor well. Both the legacy (iCanopée) path and the dmpService path have their happy path and primary error path covered. The DmpNhrAdapter tests are solid for all four message-composition cases. However, there are three noteworthy gaps and one test quality issue. --- Critical Gaps [issue] DmpApi.test.ts — non-BusinessError throw-through is not tested for the dmpService path - File: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.test.ts, line 2010 (describe block updateDocumentStatus (UpdateDocumentStatus)) - Criticality: 8/10 - The implementation at line 366 in DmpApi.ts deliberately re-throws any non-BusinessError exception (throw e). This is the explicit contract boundary: BusinessError → structured return, anything else → propagation. There is no test verifying that a plain Error (or TypeError, NetworkError, etc.) thrown by dmpService.call actually propagates as a rejection rather than being silently swallowed into a { error: ... } return. If a future refactor accidentally widens the catch block (e.g., by removing the instanceof BusinessError guard), every unrecoverable error would become a silent, structured non-error — a silent regression with no test to catch it. - The analogous test for getDocumentsContentByUniqueId on the same path (line 1997) tests that a BusinessError returns [], but it also has no non-BusinessError throw-through test. The updateDocumentStatus dmpService path is the more critical case because it changes the return shape entirely. - Suggested test to add: it('should rethrow non-BusinessError exceptions from dmpService', async () => { mockDmpService.call.mockRejectedValue(new Error('Network failure')) const dmpApi = createDmpApiWithService() await expect( dmpApi.updateDocumentStatus({ ins: '...', documentUniqueId: '...', uuid: '...', healthCareSettings: 'SA07' }) ).rejects.toThrow('Network failure') }) --- [issue] DmpApi.test.ts — s_status null/undefined fallback (?? 'ERROR') is not tested for the legacy path - File: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.test.ts, line 1642 - Criticality: 7/10 - DmpApi.ts line 394 uses response.s_status ?? 'ERROR'. This null-coalescing guard is untested. The existing error test (line 1642) provides a non-null s_status: 'ERROR_42', and the success test (line 1612) provides s_status: STATUS_OK. No test covers the case where s_status is absent/null on a non-OK response. If the guard were removed (or accidentally changed to || 'ERROR', which would also convert empty-string status to 'ERROR'), no test would catch it. - This matters because the analogous search path has an explicit test for it (line 1406: 'should return error with status ERROR when s_status is absent from response'). The updateDocumentStatus legacy path should have the same. - Suggested test: it('should return error.status ERROR when s_status is absent from the response', async () => { getCachedDmpSessionMocked.mockResolvedValue({ sessionId: 'session-abc', healthcareSettings: 'SA07' }) updateDocumentStatusCommandMocked.mockResolvedValue({ s_apiErrorDescription: 'Something failed' } as any) const dmpApi = new DmpApi(mockProps) const result = await dmpApi.updateDocumentStatus({ ... }) expect(result.error?.status).toBe('ERROR') }) --- Important Improvements [suggestion] DmpApi.test.ts — legacy path error result uses expect.objectContaining which leaves apiErrorContext and apiErrorTypeDescription implicitly untested - File: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.test.ts, lines 1659–1666 - Criticality: 5/10 - The test at line 1642 mocks a response that omits s_apiErrorContext and s_apiErrorTypeDescription, then asserts with expect.objectContaining(...). These two fields are mapped in the implementation (lines 390–391 of DmpApi.ts) but never verified to round-trip correctly through a test. The search equivalent at lines 1349–1373 does check all six fields with a full toEqual. Making the updateDocumentStatus legacy error test use the same exhaustive shape (including these two fields) would catch a typo in the field mapping (e.g., if apiErrorContext were accidentally mapped to apiErrorType). - This is a test quality concern rather than a blocking gap since the field mapping code is straightforward. --- [suggestion] DmpApi.test.ts — BusinessError constructor semantics make the test misleading - File: /Users/USER/workspace/doctolib/worktrees/doctolib/FHIR-2224/packages/@doctolib/dmp/src/business/DmpApi.test.ts, line 2048 - Criticality: 4/10 - The test passes new BusinessError('DMP returned error') then asserts apiErrorDescription: 'The teleservice returned a business error'. This is not wrong — BusinessError's constructor signature is (serviceName: string, message?: string), so the first argument is treated as a service name, and the default message 'The teleservice returned a business error' is used. However, a reader unfamiliar with this constructor signature will assume the assertion is wrong (the strings don't match) and may incorrectly "fix" the test or lose trust in it. - The test should be written to make the intent explicit: // BusinessError(serviceName, message?) — first arg is service name, not message mockDmpService.call.mockRejectedValue(new BusinessError('UpdateDocumentStatus')) expect(result.error).toEqual( expect.objectContaining({ status: 'ERROR', apiErrorDescription: 'The teleservice returned a business error' }) ) Or alternatively, supply an explicit message to confirm that the caller's message is actually forwarded through e.message: mockDmpService.call.mockRejectedValue(new BusinessError('UpdateDocumentStatus', 'custom error text')) expect(result.error?.apiErrorDescription).toBe('custom error text') The latter form would also serve as a functional test of the e.message forwarding, making it more valuable. --- Positive Observations - The DmpNhrAdapter.test.ts covers all four DmpApiError message-composition branches precisely: (1) description only (line 578), (2) neither field present → fallback 'DMP API error' (line 593), (3) both fields → combined 'desc: extended' (line 605), and (4) thrown exception (line 565). This is thorough and follows behavioral testing well. - The legacy path in DmpApi.test.ts correctly verifies that a successful response returns {} (line 1612–1625) and that missing session throws (line 1627–1639), both of which are the key behavioral changes in this PR. - The dmpService path covers the success case (line 2033) and the BusinessError structured-return case (line 2047), matching the two explicit branches in the implementation. - The partial-success test in DmpNhrAdapter.test.ts (line 623) is present and correctly validates that one success + one thrown exception yields 'partial success' status, confirming the per-payload Promise.all semantics. - The DmpApiError inline mock in DmpNhrAdapter.test.ts (lines 24–33) faithfully replicates the real class's message-construction logic, making the adapter tests self-contained without pulling in the full @doctolib/dmp module graph — a good trade-off.