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<DocumentStatusUpdateResult> 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<DocumentStatusUpdateResult> { 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<DocumentStatusUpdateResult> {
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<NhrDocumentMetadataUpdateResult> { 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: ['<rootDir>/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 blockupdateDocumentStatus (UpdateDocumentStatus)) - Criticality: 8/10
- The implementation at line 366 in
DmpApi.tsdeliberately re-throws any non-BusinessErrorexception (throw e). This is the explicit contract boundary:BusinessError→ structured return, anything else → propagation. There is no test verifying that a plainError(orTypeError,NetworkError, etc.) thrown bydmpService.callactually propagates as a rejection rather than being silently swallowed into a{ error: ... }return. If a future refactor accidentally widens thecatchblock (e.g., by removing theinstanceof BusinessErrorguard), every unrecoverable error would become a silent, structured non-error — a silent regression with no test to catch it. - The analogous test for
getDocumentsContentByUniqueIdon the same path (line 1997) tests that aBusinessErrorreturns[], but it also has no non-BusinessError throw-through test. TheupdateDocumentStatusdmpService path is the more critical case because it changes the return shape entirely. - Suggested test to add: ```ts 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:
```ts
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_apiErrorContextands_apiErrorTypeDescription, then asserts withexpect.objectContaining(...). These two fields are mapped in the implementation (lines 390–391 ofDmpApi.ts) but never verified to round-trip correctly through a test. Thesearchequivalent at lines 1349–1373 does check all six fields with a fulltoEqual. Making theupdateDocumentStatuslegacy error test use the same exhaustive shape (including these two fields) would catch a typo in the field mapping (e.g., ifapiErrorContextwere accidentally mapped toapiErrorType). - 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 assertsapiErrorDescription: '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: ```ts // 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`:
```ts
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.tscovers all fourDmpApiErrormessage-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.tscorrectly 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
dmpServicepath covers the success case (line 2033) and theBusinessErrorstructured-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
DmpApiErrorinline mock inDmpNhrAdapter.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/dmpmodule graph — a good trade-off.