From b3b989c42f15acd56a232e88dee8ac4b95e14914 Mon Sep 17 00:00:00 2001 From: Elone Hoo Date: Wed, 21 Oct 2026 03:33:27 -0700 Subject: [PATCH 1/2] test(solid): audit skipped declarations and restore meaningful coverage --- docs/solid-skipped-test-audit.md | 28 +++++++++++++++++++ .../file-upload/test/file-upload.test.tsx | 6 ++-- .../src/components/menu/test/menu.test.tsx | 7 +++-- .../number-input/test/number-input.test.tsx | 10 +++++-- .../otp-input/test/otp-input.test.tsx | 24 ++++++++++++++-- .../components/popover/test/popover.test.tsx | 4 +-- .../components/select/test/select.test.tsx | 25 ++++++++++++----- .../signature/test/signature.test.tsx | 10 ++++++- 8 files changed, 93 insertions(+), 21 deletions(-) create mode 100644 docs/solid-skipped-test-audit.md diff --git a/docs/solid-skipped-test-audit.md b/docs/solid-skipped-test-audit.md new file mode 100644 index 00000000..1cce1adc --- /dev/null +++ b/docs/solid-skipped-test-audit.md @@ -0,0 +1,28 @@ +# Solid skipped-test audit + +Issue: [#156](https://github.com/destyler/ui/issues/156). Baseline: `879f2b4`. + +The eight skipped declarations were test debt, not eight demonstrated runtime defects. +Parameterized declarations expand to multiple cases. This change does not alter runtime code. + +| Declaration | Existing coverage and comparison | Disposition | +| --- | --- | --- | +| FileUpload Field readonly | Active required, disabled, helper/error and label-focus cases cover supported Field integration. Vue likewise does not assert readonly. `use-file-upload` does not forward Field readonly and core's FileUpload context does not expose it. | Remove the unsupported readonly-on-button expectation. Neither a native button nor a file input supports HTML readonly. This is not equivalent readonly coverage: a read-only upload interaction contract remains unsupported and would need a separately designed feature, not merely an attribute assertion. | +| Menu parts matrix | Active exports and interaction tests already cover opening, groups, nested menus and controlled state. React opens the parent for TriggerItem. | Restore every anatomy part assertion after opening the parent and observing its content, so nested TriggerItem has its parent context. | +| NumberInput fraction digits | Active stepping, blur clamping, formatting setup and ValueText tests do not verify fractional rounding/padding. React/Vue FractionDigits examples use defaultValue. | Replace fixed controlled value with defaultValue; verify maximum rounding after edit/blur and minimum padding after a second edit/blur. A fixed value without parent acceptance correctly stays `1.00`. | +| OtpInput Field required | Active editing, disabled, readonly and Field feedback cases do not verify aggregate native validation; React/Vue do not provide equivalent required coverage. | Assert required on the aggregate hidden text input, not each digit; verify native valueMissing, assembled value, and reactive removal of required. | +| Popover Escape | Active trigger/close-button, controlled and presence tests do not replace keyboard dismissal. Vue already opens before Escape. | Assert visible dialog before sending Escape, then await hidden title. The old test incorrectly expected the just-opened title to be hidden before sending the key. | +| Select item selection | Active close-on-select, callback and multiple-selection tests already cover selection behavior. React/Vue have single-selection text assertions. | Restore single-selection coverage and strengthen it with hidden form value, reopened selected state, and replacing the selection. No claim that selection was wholly untested. | +| Select lazy mount/unmount | Active lazy-mount-only test covers initial absence and first mount, not removal or remount. React/Vue cover removal. | Await actual removal on close, then verify remount on reopening. Move the nested InitialValue test to suite scope so it is registered independently. | +| Signature exports matrix | Active anatomy/rendering and Field tests do not verify the complete public namespace. React/Vue exclude the internal SegmentPath anatomy entry. | Restore public anatomy exports, exclude internal SVG SegmentPath, and include Context, HiddenInput and RootProvider. Segment renders the internal path; no public SegmentPath component is promised. | + +## Verification notes + +Local type checking, scoped lint and whitespace checks pass. Supplemental happy-dom +checks reproduce the original NumberInput and Popover premise failures and pass their +corrected tests, the Select suite, the restored OTP/export cases, and an isolated Menu +parts matrix. This diagnostic is not a replacement for the repository's Chromium suite: +menu's shared browser helper cannot load in the DOM pool, and an unchanged Signature +label-focus test fails there. Local Chromium startup is blocked by the environment's IPC +restriction. The normal browser suite and aggregate package checks must pass in CI before +this change is ready. diff --git a/packages/solid/src/components/file-upload/test/file-upload.test.tsx b/packages/solid/src/components/file-upload/test/file-upload.test.tsx index 6bd898e5..5f1feecd 100644 --- a/packages/solid/src/components/file-upload/test/file-upload.test.tsx +++ b/packages/solid/src/components/file-upload/test/file-upload.test.tsx @@ -54,10 +54,8 @@ describe('file Upload / Field', () => { expect(screen.getByTestId('input')).toBeDisabled() }) - it.skip('should set file upload as readonly', async () => { - render(() => ) - expect(screen.getByRole('button', { name: /select/i })).toHaveAttribute('readonly') - }) + // File inputs and buttons have no native readonly behavior, and the file-upload + // machine has no readOnly option. Field readOnly is not an upload contract. it('should display helper text', async () => { render(() => ) diff --git a/packages/solid/src/components/menu/test/menu.test.tsx b/packages/solid/src/components/menu/test/menu.test.tsx index 0002e37f..06b08399 100644 --- a/packages/solid/src/components/menu/test/menu.test.tsx +++ b/packages/solid/src/components/menu/test/menu.test.tsx @@ -63,10 +63,13 @@ function ComponentUnderTest(props: ComponentUnderTestProps) { } describe('menu / Parts & Exports', () => { - it.skip.each(getParts(menuAnatomy))('should render part! %s', async (part) => { + it.each(getParts(menuAnatomy))('should render part! %s', async (part) => { render(() => ) - expect(document.querySelector(part)).toBeInTheDocument() + // Opening the parent connects the nested menu's TriggerItem to its parent. + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })) + await waitFor(() => expect(screen.getByText('JS Frameworks')).toBeVisible()) + await waitFor(() => expect(document.querySelector(part)).toBeInTheDocument()) }) it.each(getExports(menuAnatomy))('should export %s', async (part) => { diff --git a/packages/solid/src/components/number-input/test/number-input.test.tsx b/packages/solid/src/components/number-input/test/number-input.test.tsx index 741edc3d..409b56b9 100644 --- a/packages/solid/src/components/number-input/test/number-input.test.tsx +++ b/packages/solid/src/components/number-input/test/number-input.test.tsx @@ -112,10 +112,10 @@ describe('numberInput', () => { await waitFor(() => expect(input).toHaveAttribute('readonly')) }) - it.skip('should handle min and max fraction digits', async () => { + it('should handle min and max fraction digits', async () => { render(() => ( )) @@ -130,6 +130,12 @@ describe('numberInput', () => { await waitFor(() => { expect(input).toHaveValue('1.123') }) + + await user.clear(input) + await user.type(input, '2.5') + input.blur() + + await waitFor(() => expect(input).toHaveValue('2.50')) }) }) diff --git a/packages/solid/src/components/otp-input/test/otp-input.test.tsx b/packages/solid/src/components/otp-input/test/otp-input.test.tsx index 2b9a4e1e..5bc6c364 100644 --- a/packages/solid/src/components/otp-input/test/otp-input.test.tsx +++ b/packages/solid/src/components/otp-input/test/otp-input.test.tsx @@ -143,9 +143,27 @@ it('seeds default* via InitialValue example', async () => { }) describe('otpInput / Field', () => { - it.skip('should set input as required', async () => { - render(() => ) - expect(screen.getByRole('textbox', { name: /pin code 1 of 3/i })).toBeRequired() + it('should apply required to the aggregate form input and react to Field changes', async () => { + const [required, setRequired] = createSignal(true) + render(() => ) + + // The hidden text input represents the whole code for native form validation. + // Individual digit inputs deliberately do not carry the required constraint. + const input = document.querySelector('input[aria-hidden="true"]')! + expect(input).toBeRequired() + expect(input.validity.valueMissing).toBe(true) + for (const digit of screen.getAllByRole('textbox')) { + expect(digit).not.toBeRequired() + } + + await user.type(screen.getByLabelText('pin code 1 of 3'), '1') + await user.type(screen.getByLabelText('pin code 2 of 3'), '2') + await user.type(screen.getByLabelText('pin code 3 of 3'), '3') + await waitFor(() => expect(input).toHaveValue('123')) + expect(input.validity.valueMissing).toBe(false) + + setRequired(false) + await waitFor(() => expect(input).not.toBeRequired()) }) it('should set input as disabled', async () => { diff --git a/packages/solid/src/components/popover/test/popover.test.tsx b/packages/solid/src/components/popover/test/popover.test.tsx index a4f05ab1..a3224b06 100644 --- a/packages/solid/src/components/popover/test/popover.test.tsx +++ b/packages/solid/src/components/popover/test/popover.test.tsx @@ -43,11 +43,11 @@ describe('popover', () => { warn.mockRestore() }) - it.skip('should hide the popover when escape is pressed', async () => { + it('should hide the popover when escape is pressed', async () => { render(() => ) await user.click(screen.getByText('click me')) - await waitFor(() => expect(screen.queryByText('title')).not.toBeVisible()) + await waitFor(() => expect(screen.getByRole('dialog')).toBeVisible()) await user.keyboard('[Escape]') await waitFor(() => expect(screen.queryByText('title')).not.toBeVisible()) diff --git a/packages/solid/src/components/select/test/select.test.tsx b/packages/solid/src/components/select/test/select.test.tsx index 8f081f2d..0c2d4540 100644 --- a/packages/solid/src/components/select/test/select.test.tsx +++ b/packages/solid/src/components/select/test/select.test.tsx @@ -17,7 +17,7 @@ describe('select', () => { expectExport(Select, part) }) - it.skip('should handle item selection', async () => { + it('should handle item selection', async () => { render(() => ) const trigger = screen.getByRole('combobox', { name: 'Framework' }) await user.click(trigger) @@ -25,6 +25,14 @@ describe('select', () => { const item = screen.getByText('React', { ignore: 'option' }) await user.click(item) await waitFor(() => expect(trigger).toHaveTextContent('React')) + expect(document.querySelector('select')).toHaveValue('react') + + await user.click(trigger) + const selected = await screen.findByRole('option', { name: 'React' }) + expect(selected).toHaveAttribute('aria-selected', 'true') + await user.click(screen.getByRole('option', { name: 'Vue' })) + await waitFor(() => expect(trigger).toHaveTextContent('Vue')) + expect(document.querySelector('select')).toHaveValue('vue') }) it('should close on select', async () => { @@ -114,7 +122,7 @@ describe('select', () => { expect(screen.getByTestId('positioner')).toBeInTheDocument() }) - it.skip('should be able to lazy mount and unmount its items', async () => { + it('should be able to lazy mount and unmount its items', async () => { render(() => ) expect(screen.queryByTestId('positioner')).not.toBeInTheDocument() @@ -122,12 +130,15 @@ describe('select', () => { expect(screen.getByTestId('positioner')).toBeInTheDocument() await user.click(screen.getByRole('combobox', { name: 'Framework' })) - expect(screen.queryByTestId('positioner')).not.toBeInTheDocument() + await waitFor(() => expect(screen.queryByTestId('positioner')).not.toBeInTheDocument()) - it('seeds default* via InitialValue example', async () => { - render(() => ) - expect(screen.getByRole('combobox', { name: 'Framework' })).toHaveTextContent('Vue') - }) + await user.click(screen.getByRole('combobox', { name: 'Framework' })) + expect(await screen.findByTestId('positioner')).toBeVisible() + }) + + it('seeds default* via InitialValue example', async () => { + render(() => ) + expect(screen.getByRole('combobox', { name: 'Framework' })).toHaveTextContent('Vue') }) }) diff --git a/packages/solid/src/components/signature/test/signature.test.tsx b/packages/solid/src/components/signature/test/signature.test.tsx index 589a1b2d..9bf3849c 100644 --- a/packages/solid/src/components/signature/test/signature.test.tsx +++ b/packages/solid/src/components/signature/test/signature.test.tsx @@ -17,7 +17,15 @@ describe('signature / Parts & Exports', () => { expect(document.querySelector(part)).toBeInTheDocument() }) - it.skip.each(getExports(signatureAnatomy))('should export %s', async (part) => { + // SegmentPath is an internal SVG path rendered by Segment, not a component export. + const componentExports = [ + ...getExports(signatureAnatomy).filter(part => part !== 'SegmentPath'), + 'Context', + 'HiddenInput', + 'RootProvider', + ] + + it.each(componentExports)('should export %s', async (part) => { expectExport(Signature, part) }) From 858d30f7e869236875f7e07d17f6f0a7e9237186 Mon Sep 17 00:00:00 2001 From: Elone Hoo Date: Wed, 21 Oct 2026 03:42:56 -0700 Subject: [PATCH 2/2] test(solid): wait for deferred Popover Escape listener readiness --- docs/solid-skipped-test-audit.md | 7 ++++++- .../components/popover/test/popover.test.tsx | 18 ++++++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/docs/solid-skipped-test-audit.md b/docs/solid-skipped-test-audit.md index 1cce1adc..7d9d33f0 100644 --- a/docs/solid-skipped-test-audit.md +++ b/docs/solid-skipped-test-audit.md @@ -11,7 +11,7 @@ Parameterized declarations expand to multiple cases. This change does not alter | Menu parts matrix | Active exports and interaction tests already cover opening, groups, nested menus and controlled state. React opens the parent for TriggerItem. | Restore every anatomy part assertion after opening the parent and observing its content, so nested TriggerItem has its parent context. | | NumberInput fraction digits | Active stepping, blur clamping, formatting setup and ValueText tests do not verify fractional rounding/padding. React/Vue FractionDigits examples use defaultValue. | Replace fixed controlled value with defaultValue; verify maximum rounding after edit/blur and minimum padding after a second edit/blur. A fixed value without parent acceptance correctly stays `1.00`. | | OtpInput Field required | Active editing, disabled, readonly and Field feedback cases do not verify aggregate native validation; React/Vue do not provide equivalent required coverage. | Assert required on the aggregate hidden text input, not each digit; verify native valueMissing, assembled value, and reactive removal of required. | -| Popover Escape | Active trigger/close-button, controlled and presence tests do not replace keyboard dismissal. Vue already opens before Escape. | Assert visible dialog before sending Escape, then await hidden title. The old test incorrectly expected the just-opened title to be hidden before sending the key. | +| Popover Escape | Active trigger/close-button, controlled and presence tests do not replace keyboard dismissal. Vue already opens before Escape. | Assert visible dialog and actual deferred keydown-listener readiness before sending Escape, then await hidden title and listener cleanup. The old test incorrectly expected the just-opened title to be hidden before sending the key. | | Select item selection | Active close-on-select, callback and multiple-selection tests already cover selection behavior. React/Vue have single-selection text assertions. | Restore single-selection coverage and strengthen it with hidden form value, reopened selected state, and replacing the selection. No claim that selection was wholly untested. | | Select lazy mount/unmount | Active lazy-mount-only test covers initial absence and first mount, not removal or remount. React/Vue cover removal. | Await actual removal on close, then verify remount on reopening. Move the nested InitialValue test to suite scope so it is registered independently. | | Signature exports matrix | Active anatomy/rendering and Field tests do not verify the complete public namespace. React/Vue exclude the internal SegmentPath anatomy entry. | Restore public anatomy exports, exclude internal SVG SegmentPath, and include Context, HiddenInput and RootProvider. Segment renders the internal path; no public SegmentPath component is promised. | @@ -26,3 +26,8 @@ menu's shared browser helper cannot load in the DOM pool, and an unchanged Signa label-focus test fails there. Local Chromium startup is blocked by the environment's IPC restriction. The normal browser suite and aggregate package checks must pass in CI before this change is ready. + +The first Chromium CI run passed on Node 24 but exposed a Popover Escape race on +Node 22: opening renders synchronously, while dismissal listeners register on the next +animation frame. The test now waits for the actual keydown listener using the existing +lifecycle tracker, not a fixed delay, and checks listener cleanup after dismissal. diff --git a/packages/solid/src/components/popover/test/popover.test.tsx b/packages/solid/src/components/popover/test/popover.test.tsx index a3224b06..57453ecf 100644 --- a/packages/solid/src/components/popover/test/popover.test.tsx +++ b/packages/solid/src/components/popover/test/popover.test.tsx @@ -2,6 +2,7 @@ import { render, screen, waitFor } from '@solidjs/testing-library' import user from '@testing-library/user-event' import { createSignal } from 'solid-js' import { Popover, popoverAnatomy, usePopover } from '../' +import { trackDocumentListeners } from '../../../../../../utils/test/document-listeners' import { expectExport, getExports, getParts } from '../../../setup-test' import { InitialOpen } from '../examples/InitialOpen' import { ComponentUnderTest } from './basic' @@ -46,11 +47,20 @@ describe('popover', () => { it('should hide the popover when escape is pressed', async () => { render(() => ) - await user.click(screen.getByText('click me')) - await waitFor(() => expect(screen.getByRole('dialog')).toBeVisible()) + const listeners = trackDocumentListeners(document, 'keydown') + try { + await user.click(screen.getByText('click me')) + await waitFor(() => expect(screen.getByRole('dialog')).toBeVisible()) + // Dismissal listeners are deferred; visible content alone is not readiness. + await waitFor(() => listeners.expectActive()) - await user.keyboard('[Escape]') - await waitFor(() => expect(screen.queryByText('title')).not.toBeVisible()) + await user.keyboard('[Escape]') + await waitFor(() => expect(screen.queryByText('title')).not.toBeVisible()) + await waitFor(() => listeners.expectEmpty()) + } + finally { + listeners.restore() + } }) it('should focus the first focusable element', async () => {