diff --git a/docs/solid-skipped-test-audit.md b/docs/solid-skipped-test-audit.md new file mode 100644 index 00000000..7d9d33f0 --- /dev/null +++ b/docs/solid-skipped-test-audit.md @@ -0,0 +1,33 @@ +# 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 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. | + +## 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. + +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/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..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' @@ -43,14 +44,23 @@ 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()) + 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 () => { 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) })