Project: workspace-doctolib · Branch: HEAD · Mode: · Model: claude-sonnet-4-6
Stats: 1 user messages, 13 tool calls, tools used: Bash, Read.
Conversation
Turn 1 — User
Analyze test coverage for the treatment-plan repo changes at /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2223_fix-column-widths-truncation (branch: fhir-2223_fix-column-widths-truncation, PR #3086).
Our changes (last 2 commits, HEAD~2..HEAD) touch exactly:
- packages/tpf-national-health-records-ui/src/types.ts — added width?: string to ColumnConfig
- packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTable.tsx — added fixedLayout prop on Table
- packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTableHead/NhrDocumentListTableHead.tsx — added initialWidth={col.width} on TableHeadCell
- packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTableBody/NhrDocumentListTableBody.tsx — added
{content}
wrapper
- packages/tpf-national-health-records-ui/src/utils/reactNodeHelpers.ts — new getTextContent helper
- Plus the corresponding test files.
Please read the source and test files, then run git diff HEAD~2..HEAD to see exact changes. Report: are all new behaviors tested? Are there any edge cases missing? Focus on the new files and the changed test files only.
Turn 1 — Assistant
I'll analyze the test coverage for these changes. Let me read all the relevant files in parallel.
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2223_fix-column-widths-truncation && git diff HEAD~2..HEAD
Tools used:
- Read: /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2223_fix-column-widths-truncation/packages/tpf-national-health-records-ui/src/utils/reactNodeHelpers.ts
Tools used:
- Read: /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2223_fix-column-widths-truncation/packages/tpf-national-health-records-ui/src/types.ts
Tool results:
→ result (ok): diff --git a/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTable.test.tsx b/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTable.test.tsx
index 4e7eb52c1..8cfd9e422 100644
--- a/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTable.test.tsx
+++ b/packages/tpf-national-health-records-ui/src/components/Nhr
…(truncated, 11689 more chars)
Tool results:
→ result (ok): 1 import { isValidElement } from 'react'
2 import type { ReactNode } from 'react'
3
4 export function getTextContent(node: ReactNode): string {
5 if (typeof node === 'string' || typeof node === 'number') return String(node)
6 if (Array.isArray(node)) return node.map(getTextContent).join('')
7 if (isValidElement(node)) return getTextContent((node.props as { children?: ReactNode }).children)
8 return ''
9 }
10
Tool results:
→ result (ok): 1 import type { DesignSystemIconComponent } from '@doctolib/design-system-icons'
2 import type { NhrDocument, NhrResultStatus } from '@doctolib/tpf-national-health-records'
3 import type { ReactNode } from 'react'
4
5 export type NhrPatientInfo = {
6 fullName: string
7 gender: 'male' | 'female' | 'other'
8 birthDate: string
9 age: number
10 }
11
12 export type ColumnConfig = {
13 key: string
14 header: string
15 sortable: boolean
16 / Only required when sortable: true. Return
…(truncated, 3831 more chars)
Now let me read all the test files to understand full coverage:
Tools used:
- Read: /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2223_fix-column-widths-truncation/packages/tpf-national-health-records-ui/src/utils/reactNodeHelpers.test.ts
Tool results:
→ result (ok): 1 import { createElement } from 'react'
2 import { describe, expect, it } from 'vitest'
3
4 import { getTextContent } from './reactNodeHelpers'
5
6 describe('getTextContent', () => {
7 it('should return a string value unchanged', () => {
8 expect(getTextContent('hello')).toBe('hello')
9 })
10
11 it('should convert a number to string', () => {
12 expect(getTextContent(42)).toBe('42')
13 })
14
15 it('should extract text from a React element with a string child', () => {
16
…(truncated, 868 more chars)
Tools used:
- Read: /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2223_fix-column-widths-truncation/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTableBody/NhrDocumentListTableBody.test.tsx
Tool results:
→ result (ok): 1 import type { DesignSystemIconComponent } from '@doctolib/design-system-icons'
2 import { setup } from '@doctolib/test-helpers'
3 import { screen, waitFor } from '@testing-library/react'
4 import { createElement } from 'react'
5 import { describe, expect, it, vi } from 'vitest'
6
7 import { createMoreActionsAction } from '../../../../business/factories/actions/createMoreActionsAction'
8 import { mockColumns, mockRows } from '../../../../test-helpers'
9 import { nhrDocumentFixtureBuilder } fro
…(truncated, 6924 more chars)
Tools used:
- Read: /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2223_fix-column-widths-truncation/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTableHead/NhrDocumentListTableHead.test.tsx
Tool results:
→ result (ok): 1 import { setup } from '@doctolib/test-helpers'
2 import { screen } from '@testing-library/react'
3 import { describe, expect, it } from 'vitest'
4
5 import { mockColumns } from '../../../../test-helpers'
6 import type { ColumnConfig } from '../../../../types'
7 import { NhrDocumentListTableHead } from './NhrDocumentListTableHead'
8
9 describe('NhrDocumentListTableHead', () => {
10 it('should render all column header labels from the column config', () => {
11 setup(
12
13
…(truncated, 1858 more chars)
Tools used:
- Read: /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2223_fix-column-widths-truncation/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTable.test.tsx
Tool results:
→ result (ok): 1 import { setup } from '@doctolib/test-helpers'
2 import { screen } from '@testing-library/react'
3 import { describe, expect, it } from 'vitest'
4
5 import { mockColumns, mockRows } from '../../../test-helpers'
6 import { NhrDocumentListTable } from './NhrDocumentListTable'
7
8 const defaultPaginationProps = {
9 currentPage: 1,
10 total: 0,
11 onPageChange: () => {},
12 }
13
14 describe('NhrDocumentListTable', () => {
15 it('should render a table', () => {
16 setup( doc.classification?.codes[0]?.code ?? '-',
9 },
10 {
11 key: 'title',
12 header: 'Document name',
13 sortable: true,
14 sortAccessor: (doc) => doc.title,
15 render: (doc) => doc.title,
16 },
17 {
18 key: 'procedureDate',
19 header: 'Procedure
…(truncated, 894 more chars)
I now have all the information I need. Here is the full coverage analysis.
---
Summary
The PR introduces five behaviors: a width? property on ColumnConfig, fixedLayout on the , initialWidth={col.width} on , a truncate div wrapper with a title attribute in table body cells, and the getTextContent utility. Every new behavior has at least one test. Coverage quality is good and aligned with the project's DAMP conventions. There are a small number of edge cases worth noting, none of them critical.
---
Critical Gaps
None. No untested path could cause a silent production failure or data loss.
---
Important Improvements
1. getTextContent — mixed array of React elements and primitives (Rating: 5)
The current array test uses ['foo', ' ', 'bar'] — an array of strings only. The implementation joins array items with '' (no separator), which means getTextContent(['foo', 'bar']) returns 'foobar', not 'foo bar'. The test passes a space as a separate element, so it accidentally confirms the no-separator behavior without making it explicit.
More importantly, there is no test for a mixed array such as [createElement('span', null, 'First'), ' ', createElement('em', null, 'Last')], which is a realistic value for a cell that renders an inline icon badge next to text. If this pattern appears in a real column's render() function, the tooltip would concatenate without any separator between the element-derived text and the surrounding strings.
A test covering this input would lock in the existing concatenation behavior (or reveal it is wrong):
it('should extract text from a mixed array of elements and strings', () => {
expect(getTextContent([createElement('span', null, 'Hello'), ' ', 'world'])).toBe('Hello world')
})
File: /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2223_fix-column-widths-truncation/packages/tpf-national-health-records-ui/src/utils/reactNodeHelpers.test.ts
---
2. getTextContent — React element with multiple children (Rating: 5)
The nested-element test uses a single child at each level (createElement('div', null, createElement('span', null, 'nested'))). A realistic render function can produce createElement('div', null, 'Hello', ' ', 'World') — a React element whose children prop is itself an array, not a single node. The implementation handles this correctly because Array.isArray catches it on the recursive call, but there is no test asserting this. Without a test, a future simplification of the recursion branch could silently drop sibling text nodes.
it('should join multiple string children of a single React element', () => {
expect(getTextContent(createElement('p', null, 'Hello', ' ', 'World'))).toBe('Hello World')
})
File: /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2223_fix-column-widths-truncation/packages/tpf-national-health-records-ui/src/utils/reactNodeHelpers.test.ts
---
3. NhrDocumentListTableBody — truncate CSS class is applied to the wrapper div (Rating: 4)
The body test verifies that a title attribute is present on the correct element (getByTitle(…)), which implicitly proves the wrapper div exists. However, no test asserts that the truncate class is actually on that element. If someone removes className='truncate' but keeps title, the tooltip test would still pass and the visual truncation would silently regress.
Given the project's testing rules explicitly allow CSS class assertions when testing styling behavior (with a justification comment), a brief assertion would be appropriate here:
it('should apply the truncate class to the cell content wrapper', () => {
setup(
)
const wrapper = screen.getByTitle('SOPK test results')
expect(wrapper).toHaveClass('truncate')
})
File: /Users/USER/workspace/doctolib/worktrees/treatment-plan/fhir-2223_fix-column-widths-truncation/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListTable/NhrDocumentListTableBody/NhrDocumentListTableBody.test.tsx
---
4. nhrDocumentListColumns — none of the production column configs define width (Rating: 4)
The mockColumns used throughout all component tests are exported directly from nhrDocumentListColumns.tsx, and none of those columns have the width property set. This means NhrDocumentListTable.test.tsx and the pre-existing NhrDocumentListTableHead.test.tsx tests never exercise the initialWidth prop in the fully integrated table component. The NhrDocumentListTableHead tests do cover this via a custom columnWithWidth fixture, so the behavior is tested in isolation. But if the actual production column config is eventually given widths, there is no integration-level regression guard in NhrDocumentListTable.test.tsx. This is low-risk since the isolated tests are sufficient, but worth noting.
---
Test Quality Issues
CSS selector usage in NhrDocumentListTable.test.tsx and NhrDocumentListTableBody.test.tsx
Both files use document.querySelector('.dl-table-layout-fixed') and document.querySelector('.dl-table-body-skeleton-loader'). The project's testing rules allow CSS selector assertions when specifically testing that a CSS class is applied, provided a justification comment is present. The fixedLayout test already carries such a comment. The skeleton loader assertion in the body test does not, but this is a pre-existing pattern not introduced by this PR, so it is out of scope.
The new fixedLayout test (should use fixed table layout) queries for .dl-table-layout-fixed — an Oxygen internal class name. This is testing the implementation detail of how Oxygen applies fixedLayout, not the contract of the prop itself. If Oxygen renames this class in a future design-system update, the test will fail without any actual behavior regression. A comment acknowledging this coupling is already present in the test, which is the minimum acceptable mitigation. The approach is acceptable, but the team should be aware the test is fragile to upstream design-system changes.
---
Positive Observations
- getTextContent has thorough unit coverage of all ReactNode type branches: string, number, null, undefined, boolean, single React element, nested React element, and array. Every branch of the four-line function is exercised.
- The two NhrDocumentListTableHead width tests correctly use a positive case (with width) and a negative case (without width), avoiding over-assertion by checking style.width === '' rather than asserting absence of a DOM attribute.
- The NhrDocumentListTableBody React-element tooltip test uses createElement instead of JSX, keeping the test file as a .test.ts (not .tsx). This is consistent with the existing test file's type — but this test was actually added to the .test.tsx body test file, which is correct.
- The tests follow DAMP naming (should wrap cell content…, should derive the tooltip title from…) and are not tightly coupled to internal implementation details — they query by observable DOM properties (title, style.width, role).
- No snapshot tests are introduced. All assertions are explicit and behavior-focused.