Disable tips after 10 runs (#17101)

This commit is contained in:
Adib234
2026-01-22 15:46:18 -05:00
committed by GitHub
parent 5d68d8cda5
commit 016a94ffaf
11 changed files with 327 additions and 91 deletions

View File

@@ -0,0 +1,45 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { vi } from 'vitest';
/**
* A fake implementation of PersistentState for testing.
* It keeps state in memory and provides spies for get and set.
*/
export class FakePersistentState {
private data: Record<string, unknown> = {};
get = vi.fn().mockImplementation((key: string) => this.data[key]);
set = vi.fn().mockImplementation((key: string, value: unknown) => {
this.data[key] = value;
});
/**
* Helper to reset the fake state between tests.
*/
reset() {
this.data = {};
this.get.mockClear();
this.set.mockClear();
}
/**
* Helper to clear mock call history without wiping data.
*/
mockClear() {
this.get.mockClear();
this.set.mockClear();
}
/**
* Helper to set initial data for the fake.
*/
setData(data: Record<string, unknown>) {
this.data = { ...data };
}
}

View File

@@ -28,6 +28,13 @@ import { type HistoryItemToolGroup, StreamingState } from '../ui/types.js';
import { ToolActionsProvider } from '../ui/contexts/ToolActionsContext.js';
import { type Config } from '@google/gemini-cli-core';
import { FakePersistentState } from './persistentStateFake.js';
export const persistentStateMock = new FakePersistentState();
vi.mock('../utils/persistentState.js', () => ({
persistentState: persistentStateMock,
}));
// Wrapper around ink-testing-library's render that ensures act() is called
export const render = (
@@ -191,6 +198,7 @@ export const renderWithProviders = (
config = configProxy as unknown as Config,
useAlternateBuffer = true,
uiActions,
persistentState,
}: {
shellFocus?: boolean;
settings?: LoadedSettings;
@@ -200,6 +208,10 @@ export const renderWithProviders = (
config?: Config;
useAlternateBuffer?: boolean;
uiActions?: Partial<UIActions>;
persistentState?: {
get?: typeof persistentStateMock.get;
set?: typeof persistentStateMock.set;
};
} = {},
): ReturnType<typeof render> & { simulateClick: typeof simulateClick } => {
const baseState: UIState = new Proxy(
@@ -220,6 +232,15 @@ export const renderWithProviders = (
},
) as UIState;
if (persistentState?.get) {
persistentStateMock.get.mockImplementation(persistentState.get);
}
if (persistentState?.set) {
persistentStateMock.set.mockImplementation(persistentState.set);
}
persistentStateMock.mockClear();
const terminalWidth = width ?? baseState.terminalWidth;
let finalSettings = settings;
if (useAlternateBuffer !== undefined) {

View File

@@ -4,12 +4,15 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect, vi } from 'vitest';
import {
renderWithProviders,
persistentStateMock,
} from '../../test-utils/render.js';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { AlternateBufferQuittingDisplay } from './AlternateBufferQuittingDisplay.js';
import { ToolCallStatus } from '../types.js';
import type { HistoryItem, HistoryItemWithoutId } from '../types.js';
import { Text } from 'ink';
import { renderWithProviders } from '../../test-utils/render.js';
import type { Config } from '@google/gemini-cli-core';
vi.mock('../utils/terminalSetup.js', () => ({
@@ -98,6 +101,9 @@ const mockConfig = {
} as unknown as Config;
describe('AlternateBufferQuittingDisplay', () => {
beforeEach(() => {
vi.clearAllMocks();
});
const baseUIState = {
terminalWidth: 80,
mainAreaWidth: 80,
@@ -112,6 +118,7 @@ describe('AlternateBufferQuittingDisplay', () => {
};
it('renders with active and pending tool messages', () => {
persistentStateMock.setData({ tipsShown: 0 });
const { lastFrame } = renderWithProviders(
<AlternateBufferQuittingDisplay />,
{
@@ -127,6 +134,7 @@ describe('AlternateBufferQuittingDisplay', () => {
});
it('renders with empty history and no pending items', () => {
persistentStateMock.setData({ tipsShown: 0 });
const { lastFrame } = renderWithProviders(
<AlternateBufferQuittingDisplay />,
{
@@ -142,6 +150,7 @@ describe('AlternateBufferQuittingDisplay', () => {
});
it('renders with history but no pending items', () => {
persistentStateMock.setData({ tipsShown: 0 });
const { lastFrame } = renderWithProviders(
<AlternateBufferQuittingDisplay />,
{
@@ -157,6 +166,7 @@ describe('AlternateBufferQuittingDisplay', () => {
});
it('renders with pending items but no history', () => {
persistentStateMock.setData({ tipsShown: 0 });
const { lastFrame } = renderWithProviders(
<AlternateBufferQuittingDisplay />,
{
@@ -172,6 +182,7 @@ describe('AlternateBufferQuittingDisplay', () => {
});
it('renders with user and gemini messages', () => {
persistentStateMock.setData({ tipsShown: 0 });
const history: HistoryItem[] = [
{ id: 1, type: 'user', text: 'Hello Gemini' },
{ id: 2, type: 'gemini', text: 'Hello User!' },

View File

@@ -4,31 +4,20 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { renderWithProviders } from '../../test-utils/render.js';
import {
renderWithProviders,
persistentStateMock,
} from '../../test-utils/render.js';
import { AppHeader } from './AppHeader.js';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { describe, it, expect, vi } from 'vitest';
import { makeFakeConfig } from '@google/gemini-cli-core';
import crypto from 'node:crypto';
const persistentStateMock = vi.hoisted(() => ({
get: vi.fn(),
set: vi.fn(),
}));
vi.mock('../../utils/persistentState.js', () => ({
persistentState: persistentStateMock,
}));
vi.mock('../utils/terminalSetup.js', () => ({
getTerminalProgram: () => null,
}));
describe('<AppHeader />', () => {
beforeEach(() => {
vi.clearAllMocks();
persistentStateMock.get.mockReturnValue({});
});
it('should render the banner with default text', () => {
const mockConfig = makeFakeConfig();
const uiState = {
@@ -42,7 +31,10 @@ describe('<AppHeader />', () => {
const { lastFrame, unmount } = renderWithProviders(
<AppHeader version="1.0.0" />,
{ config: mockConfig, uiState },
{
config: mockConfig,
uiState,
},
);
expect(lastFrame()).toContain('This is the default banner');
@@ -63,7 +55,10 @@ describe('<AppHeader />', () => {
const { lastFrame, unmount } = renderWithProviders(
<AppHeader version="1.0.0" />,
{ config: mockConfig, uiState },
{
config: mockConfig,
uiState,
},
);
expect(lastFrame()).toContain('There are capacity issues');
@@ -83,7 +78,10 @@ describe('<AppHeader />', () => {
const { lastFrame, unmount } = renderWithProviders(
<AppHeader version="1.0.0" />,
{ config: mockConfig, uiState },
{
config: mockConfig,
uiState,
},
);
expect(lastFrame()).not.toContain('Banner');
@@ -104,7 +102,10 @@ describe('<AppHeader />', () => {
const { lastFrame, unmount } = renderWithProviders(
<AppHeader version="1.0.0" />,
{ config: mockConfig, uiState },
{
config: mockConfig,
uiState,
},
);
expect(lastFrame()).toContain('This is the default banner');
@@ -124,7 +125,10 @@ describe('<AppHeader />', () => {
const { lastFrame, unmount } = renderWithProviders(
<AppHeader version="1.0.0" />,
{ config: mockConfig, uiState },
{
config: mockConfig,
uiState,
},
);
expect(lastFrame()).not.toContain('This is the default banner');
@@ -133,7 +137,6 @@ describe('<AppHeader />', () => {
});
it('should not render the default banner if shown count is 5 or more', () => {
persistentStateMock.get.mockReturnValue(5);
const mockConfig = makeFakeConfig();
const uiState = {
history: [],
@@ -143,9 +146,21 @@ describe('<AppHeader />', () => {
},
};
persistentStateMock.setData({
defaultBannerShownCount: {
[crypto
.createHash('sha256')
.update(uiState.bannerData.defaultText)
.digest('hex')]: 5,
},
});
const { lastFrame, unmount } = renderWithProviders(
<AppHeader version="1.0.0" />,
{ config: mockConfig, uiState },
{
config: mockConfig,
uiState,
},
);
expect(lastFrame()).not.toContain('This is the default banner');
@@ -154,7 +169,6 @@ describe('<AppHeader />', () => {
});
it('should increment the version count when default banner is displayed', () => {
persistentStateMock.get.mockReturnValue({});
const mockConfig = makeFakeConfig();
const uiState = {
history: [],
@@ -164,6 +178,10 @@ describe('<AppHeader />', () => {
},
};
// Set tipsShown to 10 or more to prevent Tips from incrementing its count
// and interfering with the expected persistentState.set call.
persistentStateMock.setData({ tipsShown: 10 });
const { unmount } = renderWithProviders(<AppHeader version="1.0.0" />, {
config: mockConfig,
uiState,
@@ -194,10 +212,87 @@ describe('<AppHeader />', () => {
const { lastFrame, unmount } = renderWithProviders(
<AppHeader version="1.0.0" />,
{ config: mockConfig, uiState },
{
config: mockConfig,
uiState,
},
);
expect(lastFrame()).not.toContain('First line\\nSecond line');
unmount();
});
it('should render Tips when tipsShown is less than 10', () => {
const mockConfig = makeFakeConfig();
const uiState = {
history: [],
bannerData: {
defaultText: 'First line\\nSecond line',
warningText: '',
},
bannerVisible: true,
};
persistentStateMock.setData({ tipsShown: 5 });
const { lastFrame, unmount } = renderWithProviders(
<AppHeader version="1.0.0" />,
{
config: mockConfig,
uiState,
},
);
expect(lastFrame()).toContain('Tips');
expect(persistentStateMock.set).toHaveBeenCalledWith('tipsShown', 6);
unmount();
});
it('should NOT render Tips when tipsShown is 10 or more', () => {
const mockConfig = makeFakeConfig();
persistentStateMock.setData({ tipsShown: 10 });
const { lastFrame, unmount } = renderWithProviders(
<AppHeader version="1.0.0" />,
{
config: mockConfig,
},
);
expect(lastFrame()).not.toContain('Tips');
unmount();
});
it('should show tips until they have been shown 10 times (persistence flow)', () => {
persistentStateMock.setData({ tipsShown: 9 });
const mockConfig = makeFakeConfig();
const uiState = {
history: [],
bannerData: {
defaultText: 'First line\\nSecond line',
warningText: '',
},
bannerVisible: true,
};
// First session
const session1 = renderWithProviders(<AppHeader version="1.0.0" />, {
config: mockConfig,
uiState,
});
expect(session1.lastFrame()).toContain('Tips');
expect(persistentStateMock.get('tipsShown')).toBe(10);
session1.unmount();
// Second session - state is persisted in the fake
const session2 = renderWithProviders(<AppHeader version="1.0.0" />, {
config: mockConfig,
});
expect(session2.lastFrame()).not.toContain('Tips');
session2.unmount();
});
});

View File

@@ -12,6 +12,7 @@ import { useConfig } from '../contexts/ConfigContext.js';
import { useUIState } from '../contexts/UIStateContext.js';
import { Banner } from './Banner.js';
import { useBanner } from '../hooks/useBanner.js';
import { useTips } from '../hooks/useTips.js';
interface AppHeaderProps {
version: string;
@@ -23,6 +24,7 @@ export const AppHeader = ({ version }: AppHeaderProps) => {
const { nightly, mainAreaWidth, bannerData, bannerVisible } = useUIState();
const { bannerText } = useBanner(bannerData, config);
const { showTips } = useTips();
return (
<Box flexDirection="column">
@@ -38,9 +40,8 @@ export const AppHeader = ({ version }: AppHeaderProps) => {
)}
</>
)}
{!(settings.merged.ui.hideTips || config.getScreenReader()) && (
<Tips config={config} />
)}
{!(settings.merged.ui.hideTips || config.getScreenReader()) &&
showTips && <Tips config={config} />}
</Box>
);
};

View File

@@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { render } from '../../test-utils/render.js';
import { render, persistentStateMock } from '../../test-utils/render.js';
import { Notifications } from './Notifications.js';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { useAppContext, type AppState } from '../contexts/AppContext.js';
@@ -30,6 +30,7 @@ vi.mock('node:fs/promises', async () => {
access: vi.fn(),
writeFile: vi.fn(),
mkdir: vi.fn().mockResolvedValue(undefined),
unlink: vi.fn().mockResolvedValue(undefined),
};
});
vi.mock('node:os', () => ({
@@ -68,10 +69,11 @@ describe('Notifications', () => {
const mockUseUIState = vi.mocked(useUIState);
const mockUseIsScreenReaderEnabled = vi.mocked(useIsScreenReaderEnabled);
const mockFsAccess = vi.mocked(fs.access);
const mockFsWriteFile = vi.mocked(fs.writeFile);
const mockFsUnlink = vi.mocked(fs.unlink);
beforeEach(() => {
vi.clearAllMocks();
persistentStateMock.reset();
mockUseAppContext.mockReturnValue({
startupWarnings: [],
version: '1.0.0',
@@ -134,51 +136,47 @@ describe('Notifications', () => {
expect(lastFrame()).toMatchSnapshot();
});
it('renders screen reader nudge when enabled and not seen', async () => {
it('renders screen reader nudge when enabled and not seen (no legacy file)', async () => {
mockUseIsScreenReaderEnabled.mockReturnValue(true);
let rejectAccess: (err: Error) => void;
mockFsAccess.mockImplementation(
() =>
new Promise((_, reject) => {
rejectAccess = reject;
}),
);
persistentStateMock.setData({ hasSeenScreenReaderNudge: false });
mockFsAccess.mockRejectedValue(new Error('No legacy file'));
const { lastFrame } = render(<Notifications />);
// Trigger rejection inside act
await act(async () => {
rejectAccess(new Error('File not found'));
});
// Wait for effect to propagate
await vi.waitFor(() => {
expect(mockFsWriteFile).toHaveBeenCalled();
});
expect(lastFrame()).toContain('screen reader-friendly view');
expect(persistentStateMock.set).toHaveBeenCalledWith(
'hasSeenScreenReaderNudge',
true,
);
expect(lastFrame()).toMatchSnapshot();
});
it('does not render screen reader nudge when already seen', async () => {
it('migrates legacy screen reader nudge file', async () => {
mockUseIsScreenReaderEnabled.mockReturnValue(true);
persistentStateMock.setData({ hasSeenScreenReaderNudge: undefined });
mockFsAccess.mockResolvedValue(undefined);
let resolveAccess: (val: undefined) => void;
mockFsAccess.mockImplementation(
() =>
new Promise((resolve) => {
resolveAccess = resolve;
}),
);
render(<Notifications />);
await act(async () => {
await vi.waitFor(() => {
expect(persistentStateMock.set).toHaveBeenCalledWith(
'hasSeenScreenReaderNudge',
true,
);
expect(mockFsUnlink).toHaveBeenCalled();
});
});
});
it('does not render screen reader nudge when already seen in persistent state', async () => {
mockUseIsScreenReaderEnabled.mockReturnValue(true);
persistentStateMock.setData({ hasSeenScreenReaderNudge: true });
const { lastFrame } = render(<Notifications />);
// Trigger resolution inside act
await act(async () => {
resolveAccess(undefined);
});
expect(lastFrame()).toBe('');
expect(mockFsWriteFile).not.toHaveBeenCalled();
expect(persistentStateMock.set).not.toHaveBeenCalled();
});
});

View File

@@ -11,13 +11,9 @@ import { useUIState } from '../contexts/UIStateContext.js';
import { theme } from '../semantic-colors.js';
import { StreamingState } from '../types.js';
import { UpdateNotification } from './UpdateNotification.js';
import { persistentState } from '../../utils/persistentState.js';
import {
GEMINI_DIR,
Storage,
debugLogger,
homedir,
} from '@google/gemini-cli-core';
import { GEMINI_DIR, Storage, homedir } from '@google/gemini-cli-core';
import * as fs from 'node:fs/promises';
import path from 'node:path';
@@ -38,15 +34,20 @@ export const Notifications = () => {
const showInitError =
initError && streamingState !== StreamingState.Responding;
const [hasSeenScreenReaderNudge, setHasSeenScreenReaderNudge] = useState<
boolean | undefined
>(undefined);
const [hasSeenScreenReaderNudge, setHasSeenScreenReaderNudge] = useState(() =>
persistentState.get('hasSeenScreenReaderNudge'),
);
useEffect(() => {
const checkScreenReader = async () => {
const checkLegacyScreenReaderNudge = async () => {
if (hasSeenScreenReaderNudge !== undefined) return;
try {
await fs.access(screenReaderNudgeFilePath);
persistentState.set('hasSeenScreenReaderNudge', true);
setHasSeenScreenReaderNudge(true);
// Best effort cleanup of legacy file
await fs.unlink(screenReaderNudgeFilePath).catch(() => {});
} catch {
setHasSeenScreenReaderNudge(false);
}
@@ -54,28 +55,17 @@ export const Notifications = () => {
if (isScreenReaderEnabled) {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
checkScreenReader();
checkLegacyScreenReaderNudge();
}
}, [isScreenReaderEnabled]);
}, [isScreenReaderEnabled, hasSeenScreenReaderNudge]);
const showScreenReaderNudge =
isScreenReaderEnabled && hasSeenScreenReaderNudge === false;
useEffect(() => {
const writeScreenReaderNudgeFile = async () => {
if (showScreenReaderNudge) {
try {
await fs.mkdir(path.dirname(screenReaderNudgeFilePath), {
recursive: true,
});
await fs.writeFile(screenReaderNudgeFilePath, 'true');
} catch (error) {
debugLogger.error('Error storing screen reader nudge', error);
}
}
};
// eslint-disable-next-line @typescript-eslint/no-floating-promises
writeScreenReaderNudgeFile();
if (showScreenReaderNudge) {
persistentState.set('hasSeenScreenReaderNudge', true);
}
}, [showScreenReaderNudge]);
if (

View File

@@ -7,7 +7,7 @@ exports[`Notifications > renders init error 1`] = `
"
`;
exports[`Notifications > renders screen reader nudge when enabled and not seen 1`] = `
exports[`Notifications > renders screen reader nudge when enabled and not seen (no legacy file) 1`] = `
"You are currently in screen reader-friendly view. To switch out, open
/mock/home/.gemini/settings.json and remove the entry for "screenReader". This will disappear on
next run."

View File

@@ -0,0 +1,47 @@
/**
* @license
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import {
renderHookWithProviders,
persistentStateMock,
} from '../../test-utils/render.js';
import { describe, it, expect, beforeEach, vi } from 'vitest';
import { useTips } from './useTips.js';
describe('useTips()', () => {
beforeEach(() => {
vi.clearAllMocks();
});
it('should return false and call set(1) if state is undefined', () => {
const { result } = renderHookWithProviders(() => useTips());
expect(result.current.showTips).toBe(true);
expect(persistentStateMock.set).toHaveBeenCalledWith('tipsShown', 1);
expect(persistentStateMock.get('tipsShown')).toBe(1);
});
it('should return false and call set(6) if state is 5', () => {
persistentStateMock.setData({ tipsShown: 5 });
const { result } = renderHookWithProviders(() => useTips());
expect(result.current.showTips).toBe(true);
expect(persistentStateMock.get('tipsShown')).toBe(6);
});
it('should return true if state is 10', () => {
persistentStateMock.setData({ tipsShown: 10 });
const { result } = renderHookWithProviders(() => useTips());
expect(result.current.showTips).toBe(false);
expect(persistentStateMock.set).not.toHaveBeenCalled();
expect(persistentStateMock.get('tipsShown')).toBe(10);
});
});

View File

@@ -0,0 +1,26 @@
/**
* @license
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { useEffect, useState } from 'react';
import { persistentState } from '../../utils/persistentState.js';
interface UseTipsResult {
showTips: boolean;
}
export function useTips(): UseTipsResult {
const [tipsCount] = useState(() => persistentState.get('tipsShown') ?? 0);
const showTips = tipsCount < 10;
useEffect(() => {
if (showTips) {
persistentState.set('tipsShown', tipsCount + 1);
}
}, [tipsCount, showTips]);
return { showTips };
}

View File

@@ -12,6 +12,8 @@ const STATE_FILENAME = 'state.json';
interface PersistentStateData {
defaultBannerShownCount?: Record<string, number>;
tipsShown?: number;
hasSeenScreenReaderNudge?: boolean;
// Add other persistent state keys here as needed
}