From 85581fd3e14041ecd4cf39827a0014c6406e8979 Mon Sep 17 00:00:00 2001 From: Taylor Mullen Date: Tue, 31 Mar 2026 17:00:41 -0700 Subject: [PATCH] refactor(cli): address code review feedback for visual journey testing - Replace 'any' with 'TestableDOMNode' for Ink tree traversal in AppRig and custom matchers - Rename 'toVisuallyContain' to 'toContainComponent' for clarity - Remove SVG audit trail (component injection) to prevent snapshot churn --- packages/cli/src/test-utils/AppRig.tsx | 27 ++++++-- packages/cli/src/test-utils/customMatchers.ts | 64 ++++++++++--------- packages/cli/src/test-utils/render.tsx | 3 +- packages/cli/src/test-utils/svg.ts | 33 +--------- .../components/SuggestionsDisplay.ux.test.tsx | 4 +- 5 files changed, 59 insertions(+), 72 deletions(-) diff --git a/packages/cli/src/test-utils/AppRig.tsx b/packages/cli/src/test-utils/AppRig.tsx index 4a8334450e..0ed88cd835 100644 --- a/packages/cli/src/test-utils/AppRig.tsx +++ b/packages/cli/src/test-utils/AppRig.tsx @@ -730,8 +730,20 @@ export class AppRig { const rootNode = this.renderResult?.rootNode; if (!rootNode) return false; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const find = (node: any): boolean => { + type TestableDOMNode = import('ink').DOMNode & { + internal_componentName?: string; + internal_testId?: string; + attributes?: { + internal_componentName?: string; + internal_testId?: string; + }; + style?: { + internal_componentName?: string; + internal_testId?: string; + }; + }; + + const find = (node: TestableDOMNode): boolean => { if ( node.internal_componentName === componentName || node.internal_testId === componentName || @@ -742,15 +754,20 @@ export class AppRig { ) { return true; } - if (node.childNodes) { + if ('childNodes' in node && node.childNodes) { for (const child of node.childNodes) { - if (child.nodeName !== '#text' && find(child)) return true; + if ( + child.nodeName !== '#text' && + find(child as TestableDOMNode) + ) { + return true; + } } } return false; }; - return find(rootNode); + return find(rootNode as TestableDOMNode); }, { timeout, diff --git a/packages/cli/src/test-utils/customMatchers.ts b/packages/cli/src/test-utils/customMatchers.ts index a918989297..78c01ee709 100644 --- a/packages/cli/src/test-utils/customMatchers.ts +++ b/packages/cli/src/test-utils/customMatchers.ts @@ -17,7 +17,7 @@ export interface CustomMatchers { allowEmpty?: boolean; name?: string; }): Promise; - toVisuallyContain(componentName: string): R; + toContainComponent(componentName: string): R; toHaveOnlyValidCharacters(): R; } @@ -25,17 +25,30 @@ export interface CustomMatchers { // eslint-disable-next-line no-control-regex const invalidCharsRegex = /[\b\x1b]/; +type TestableDOMNode = DOMNode & { + internal_componentName?: string; + internal_testId?: string; + attributes?: { + internal_componentName?: string; + internal_testId?: string; + }; + style?: { + internal_componentName?: string; + internal_testId?: string; + }; +}; + /** * Traverses the Ink tree to find a node matching a predicate. */ function findInTree( - node: DOMNode, - predicate: (node: DOMNode) => boolean, -): DOMNode | undefined { + node: TestableDOMNode, + predicate: (node: TestableDOMNode) => boolean, +): TestableDOMNode | undefined { if (predicate(node)) return node; - if (node.nodeName !== '#text') { - for (const child of (node).childNodes) { - const found = findInTree(child, predicate); + if ('childNodes' in node && node.childNodes) { + for (const child of node.childNodes) { + const found = findInTree(child as TestableDOMNode, predicate); if (found) return found; } } @@ -45,7 +58,7 @@ function findInTree( /** * Checks if the Ink DOM tree contains a specific component by name or testId. */ -export function toVisuallyContain( +export function toContainComponent( this: Assertion, // eslint-disable-next-line @typescript-eslint/no-explicit-any received: any, @@ -55,37 +68,26 @@ export function toVisuallyContain( const { isNot } = this as any; const rootNode = received.rootNode || received.renderResult?.rootNode; - const svg = - typeof received.generateSvg === 'function' ? received.generateSvg() : ''; - // 1. Check logical tree presence (Automatic via Ink Root) - const isTreePresent = rootNode - ? !!findInTree(rootNode, (node) => { + // Check logical tree presence (Automatic via Ink Root) + const pass = rootNode + ? !!findInTree(rootNode as TestableDOMNode, (node) => { if (node.nodeName === '#text') return false; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const el = node as any; const match = - el.internal_componentName === componentName || - el.internal_testId === componentName || - el.attributes?.internal_componentName === componentName || - el.attributes?.internal_testId === componentName || - el.style?.internal_componentName === componentName || - el.style?.internal_testId === componentName; + node.internal_componentName === componentName || + node.internal_testId === componentName || + node.attributes?.internal_componentName === componentName || + node.attributes?.internal_testId === componentName || + node.style?.internal_componentName === componentName || + node.style?.internal_testId === componentName; return match; }) : false; - // 2. Check physical presence in the SVG audit trail (Fallback) - const isPhysicallyPresent = svg.includes( - ``, - ); - - const pass = isTreePresent || isPhysicallyPresent; - return { pass, message: () => - `Expected component "${componentName}" ${isNot ? 'NOT ' : ''}to be present in the Ink tree or SVG metadata.`, + `Expected component "${componentName}" ${isNot ? 'NOT ' : ''}to be present in the Ink tree.`, }; } @@ -191,7 +193,7 @@ function toHaveOnlyValidCharacters(this: Assertion, buffer: TextBuffer) { expect.extend({ toHaveOnlyValidCharacters, toMatchSvgSnapshot, - toVisuallyContain, + toContainComponent, // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any); @@ -208,6 +210,6 @@ declare module 'vitest' { allowEmpty?: boolean; name?: string; }): Promise; - toVisuallyContain(componentName: string): T; + toContainComponent(componentName: string): T; } } diff --git a/packages/cli/src/test-utils/render.tsx b/packages/cli/src/test-utils/render.tsx index 65cd885e7e..5a048c3b50 100644 --- a/packages/cli/src/test-utils/render.tsx +++ b/packages/cli/src/test-utils/render.tsx @@ -467,8 +467,7 @@ export const render = async ( generateSvg: stdout.generateSvg, terminal: state.terminal, get rootNode() { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - return (instance as any).rootNode as DOMElement; + return (instance as InkInstance & { rootNode: DOMElement }).rootNode; }, waitUntilReady: () => stdout.waitUntilReady(), capturedOverflowState: undefined, diff --git a/packages/cli/src/test-utils/svg.ts b/packages/cli/src/test-utils/svg.ts index a37f520bf0..92d3f53c2f 100644 --- a/packages/cli/src/test-utils/svg.ts +++ b/packages/cli/src/test-utils/svg.ts @@ -5,14 +5,9 @@ */ import type { Terminal } from '@xterm/headless'; -import { type DOMElement } from 'ink'; -export const generateSvgForTerminal = ( - terminal: Terminal, - options: { rootNode?: DOMElement } = {}, -): string => { +export const generateSvgForTerminal = (terminal: Terminal): string => { const activeBuffer = terminal.buffer.active; - const { rootNode } = options; const getHexColor = ( isRGB: boolean, @@ -213,32 +208,6 @@ export const generateSvgForTerminal = ( finalizeBlock(line.length); } - // Inject unique component names from the tree as comments for auditing - if (rootNode) { - const components = new Set(); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const collect = (node: any) => { - if (node.internal_componentName) - components.add(node.internal_componentName); - if (node.internal_testId) components.add(node.internal_testId); - if (node.attributes?.internal_componentName) - components.add(node.attributes.internal_componentName); - if (node.attributes?.internal_testId) - components.add(node.attributes.internal_testId); - if (node.style?.internal_componentName) - components.add(node.style.internal_componentName); - if (node.style?.internal_testId) - components.add(node.style.internal_testId); - if (node.childNodes) { - for (const child of node.childNodes) collect(child); - } - }; - collect(rootNode); - for (const name of components) { - svg += ` \n`; - } - } - svg += ` \n`; return svg; }; diff --git a/packages/cli/src/ui/components/SuggestionsDisplay.ux.test.tsx b/packages/cli/src/ui/components/SuggestionsDisplay.ux.test.tsx index fed9d8c933..c39e729872 100644 --- a/packages/cli/src/ui/components/SuggestionsDisplay.ux.test.tsx +++ b/packages/cli/src/ui/components/SuggestionsDisplay.ux.test.tsx @@ -38,7 +38,7 @@ describe('SuggestionsDisplay UX Journey', () => { it('should visually show the suggestions display when / is typed', async () => { // Initially should not have suggestions - expect(rig).not.toVisuallyContain(SuggestionsDisplay.name); + expect(rig).not.toContainComponent(SuggestionsDisplay.name); // Type '/' to trigger suggestions await rig.type('/'); @@ -47,7 +47,7 @@ describe('SuggestionsDisplay UX Journey', () => { await rig.waitForComponent(SuggestionsDisplay.name); // Assert that the component is now present in the tree - expect(rig).toVisuallyContain(SuggestionsDisplay.name); + expect(rig).toContainComponent(SuggestionsDisplay.name); // Also verify text for sanity expect(rig.lastFrame).toContain('about');