mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-04 02:11:11 -07:00
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
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -17,7 +17,7 @@ export interface CustomMatchers<R = unknown> {
|
||||
allowEmpty?: boolean;
|
||||
name?: string;
|
||||
}): Promise<R>;
|
||||
toVisuallyContain(componentName: string): R;
|
||||
toContainComponent(componentName: string): R;
|
||||
toHaveOnlyValidCharacters(): R;
|
||||
}
|
||||
|
||||
@@ -25,17 +25,30 @@ export interface CustomMatchers<R = unknown> {
|
||||
// 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(
|
||||
`<!-- component: ${componentName} -->`,
|
||||
);
|
||||
|
||||
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<void>;
|
||||
toVisuallyContain(componentName: string): T;
|
||||
toContainComponent(componentName: string): T;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<string>();
|
||||
// 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 += ` <!-- component: ${name} -->\n`;
|
||||
}
|
||||
}
|
||||
|
||||
svg += ` </g>\n</svg>`;
|
||||
return svg;
|
||||
};
|
||||
|
||||
@@ -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');
|
||||
|
||||
Reference in New Issue
Block a user