Skip to content

Commit dcdeddf

Browse files
NTaylorMullenNTaylorMullen
committed
Address code review comments from Jacob in PR 16378
- Use custom waitFor in AppContainer tests - Add vi.restoreAllMocks() to afterEach blocks and fix mock reset in beforeEach - Refactor terminal title tests to use it.each - Regenerate docs/schemas to ensure consistency Fixes #16367
1 parent 48bc7c0 commit dcdeddf

2 files changed

Lines changed: 122 additions & 104 deletions

File tree

‎packages/cli/src/ui/AppContainer.test.tsx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,8 @@ describe('AppContainer State Management', () => {
250250
beforeEach(() => {
251251
vi.clearAllMocks();
252252

253+
mockIdeClient.getInstance.mockReturnValue(new Promise(() => {}));
254+
253255
// Initialize mock stdout for terminal title tests
254256

255257
mocks.mockStdout.write.mockClear();
@@ -404,6 +406,7 @@ describe('AppContainer State Management', () => {
404406

405407
afterEach(() => {
406408
cleanup();
409+
vi.restoreAllMocks();
407410
});
408411

409412
describe('Basic Rendering', () => {
@@ -1271,7 +1274,7 @@ describe('AppContainer State Management', () => {
12711274
});
12721275

12731276
// Now it should show Action Required
1274-
await vi.waitFor(() => {
1277+
await waitFor(() => {
12751278
const titleWrites = mocks.mockStdout.write.mock.calls.filter((call) =>
12761279
call[0].includes('\x1b]2;'),
12771280
);
@@ -1419,6 +1422,7 @@ describe('AppContainer State Management', () => {
14191422

14201423
afterEach(() => {
14211424
vi.useRealTimers();
1425+
vi.restoreAllMocks();
14221426
});
14231427

14241428
it('should set and clear the queue error message after a timeout', async () => {
@@ -1587,6 +1591,7 @@ describe('AppContainer State Management', () => {
15871591

15881592
afterEach(() => {
15891593
vi.useRealTimers();
1594+
vi.restoreAllMocks();
15901595
});
15911596

15921597
describe('CTRL+C', () => {
@@ -1724,6 +1729,7 @@ describe('AppContainer State Management', () => {
17241729

17251730
afterEach(() => {
17261731
vi.useRealTimers();
1732+
vi.restoreAllMocks();
17271733
});
17281734

17291735
describe.each([

‎packages/cli/src/utils/windowTitle.test.ts‎

Lines changed: 115 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -13,58 +13,91 @@ describe('computeTerminalTitle', () => {
1313
vi.unstubAllEnvs();
1414
});
1515

16-
it('should return idle state title with folder name', () => {
17-
const title = computeTerminalTitle({
18-
streamingState: StreamingState.Idle,
19-
isConfirming: false,
20-
folderName: 'my-project',
21-
showThoughts: false,
22-
useDynamicTitle: true,
23-
});
24-
25-
expect(title).toContain('◇ Ready (my-project)');
16+
it.each([
17+
{
18+
description: 'idle state title with folder name',
19+
args: {
20+
streamingState: StreamingState.Idle,
21+
isConfirming: false,
22+
folderName: 'my-project',
23+
showThoughts: false,
24+
useDynamicTitle: true,
25+
},
26+
expected: '◇ Ready (my-project)',
27+
},
28+
{
29+
description: 'legacy title when useDynamicTitle is false',
30+
args: {
31+
streamingState: StreamingState.Responding,
32+
isConfirming: false,
33+
folderName: 'my-project',
34+
showThoughts: true,
35+
useDynamicTitle: false,
36+
},
37+
expected: 'Gemini CLI (my-project)'.padEnd(80, ' '),
38+
exact: true,
39+
},
40+
{
41+
description:
42+
'active state title with "Working..." when thoughts are disabled',
43+
args: {
44+
streamingState: StreamingState.Responding,
45+
thoughtSubject: 'Reading files',
46+
isConfirming: false,
47+
folderName: 'my-project',
48+
showThoughts: false,
49+
useDynamicTitle: true,
50+
},
51+
expected: '✦ Working... (my-project)',
52+
},
53+
{
54+
description:
55+
'active state title with thought subject and suffix when thoughts are short enough',
56+
args: {
57+
streamingState: StreamingState.Responding,
58+
thoughtSubject: 'Short thought',
59+
isConfirming: false,
60+
folderName: 'my-project',
61+
showThoughts: true,
62+
useDynamicTitle: true,
63+
},
64+
expected: '✦ Short thought (my-project)',
65+
},
66+
{
67+
description:
68+
'fallback active title with suffix if no thought subject is provided even when thoughts are enabled',
69+
args: {
70+
streamingState: StreamingState.Responding,
71+
thoughtSubject: undefined,
72+
isConfirming: false,
73+
folderName: 'my-project',
74+
showThoughts: true,
75+
useDynamicTitle: true,
76+
},
77+
expected: '✦ Working... (my-project)'.padEnd(80, ' '),
78+
exact: true,
79+
},
80+
{
81+
description: 'action required state when confirming',
82+
args: {
83+
streamingState: StreamingState.Idle,
84+
isConfirming: true,
85+
folderName: 'my-project',
86+
showThoughts: false,
87+
useDynamicTitle: true,
88+
},
89+
expected: '✋ Action Required (my-project)',
90+
},
91+
])('should return $description', ({ args, expected, exact }) => {
92+
const title = computeTerminalTitle(args);
93+
if (exact) {
94+
expect(title).toBe(expected);
95+
} else {
96+
expect(title).toContain(expected);
97+
}
2698
expect(title.length).toBe(80);
2799
});
28100

29-
it('should return legacy title when useDynamicTitle is false', () => {
30-
const title = computeTerminalTitle({
31-
streamingState: StreamingState.Responding,
32-
isConfirming: false,
33-
folderName: 'my-project',
34-
showThoughts: true, // Should be ignored
35-
useDynamicTitle: false,
36-
});
37-
38-
expect(title).toBe('Gemini CLI (my-project)'.padEnd(80, ' '));
39-
});
40-
41-
it('should return active state title with "Working..." when thoughts are disabled', () => {
42-
const title = computeTerminalTitle({
43-
streamingState: StreamingState.Responding,
44-
thoughtSubject: 'Reading files',
45-
isConfirming: false,
46-
folderName: 'my-project',
47-
showThoughts: false,
48-
useDynamicTitle: true,
49-
});
50-
51-
expect(title).toContain('✦ Working... (my-project)');
52-
expect(title.length).toBe(80);
53-
});
54-
55-
it('should return active state title with thought subject and suffix when thoughts are short enough', () => {
56-
const title = computeTerminalTitle({
57-
streamingState: StreamingState.Responding,
58-
thoughtSubject: 'Short thought',
59-
isConfirming: false,
60-
folderName: 'my-project',
61-
showThoughts: true,
62-
useDynamicTitle: true,
63-
});
64-
65-
expect(title).toContain('✦ Short thought (my-project)');
66-
});
67-
68101
it('should return active state title with thought subject and NO suffix when thoughts are very long', () => {
69102
const longThought = 'A'.repeat(70);
70103
const title = computeTerminalTitle({
@@ -78,31 +111,7 @@ describe('computeTerminalTitle', () => {
78111

79112
expect(title).not.toContain('(my-project)');
80113
expect(title).toContain('✦ AAAAAAAAAAAAAAAA');
81-
});
82-
83-
it('should return fallback active title with suffix if no thought subject is provided even when thoughts are enabled', () => {
84-
const title = computeTerminalTitle({
85-
streamingState: StreamingState.Responding,
86-
thoughtSubject: undefined,
87-
isConfirming: false,
88-
folderName: 'my-project',
89-
showThoughts: true,
90-
useDynamicTitle: true,
91-
});
92-
93-
expect(title).toBe('✦ Working... (my-project)'.padEnd(80, ' '));
94-
});
95-
96-
it('should return action required state when confirming', () => {
97-
const title = computeTerminalTitle({
98-
streamingState: StreamingState.Idle,
99-
isConfirming: true,
100-
folderName: 'my-project',
101-
showThoughts: false,
102-
useDynamicTitle: true,
103-
});
104-
105-
expect(title).toContain('✋ Action Required (my-project)');
114+
expect(title.length).toBe(80);
106115
});
107116

108117
it('should truncate long thought subjects when thoughts are enabled', () => {
@@ -135,6 +144,7 @@ describe('computeTerminalTitle', () => {
135144
expect(title).not.toContain('\x00');
136145
expect(title).not.toContain('\x07');
137146
expect(title).not.toContain('\x1B');
147+
expect(title.length).toBe(80);
138148
});
139149

140150
it('should prioritize CLI_TITLE environment variable over folder name when thoughts are disabled', () => {
@@ -150,39 +160,41 @@ describe('computeTerminalTitle', () => {
150160

151161
expect(title).toContain('◇ Ready (EnvOverride)');
152162
expect(title).not.toContain('my-project');
153-
});
154-
155-
it('should truncate very long folder names to fit within 80 characters', () => {
156-
const longFolderName = 'A'.repeat(100);
157-
const title = computeTerminalTitle({
158-
streamingState: StreamingState.Idle,
159-
isConfirming: false,
160-
folderName: longFolderName,
161-
showThoughts: false,
162-
useDynamicTitle: true,
163-
});
164-
165163
expect(title.length).toBe(80);
166-
expect(title).toContain('◇ Ready (AAAAA');
167-
expect(title).toContain('...)');
168164
});
169165

170-
it('should truncate very long CLI_TITLE to fit within 80 characters', () => {
171-
const longTitle = 'B'.repeat(100);
172-
vi.stubEnv('CLI_TITLE', longTitle);
173-
174-
const title = computeTerminalTitle({
175-
streamingState: StreamingState.Idle,
176-
isConfirming: false,
166+
it.each([
167+
{
168+
name: 'folder name',
169+
folderName: 'A'.repeat(100),
170+
expected: '◇ Ready (AAAAA',
171+
},
172+
{
173+
name: 'CLI_TITLE',
177174
folderName: 'my-project',
178-
showThoughts: false,
179-
useDynamicTitle: true,
180-
});
181-
182-
expect(title.length).toBe(80);
183-
expect(title).toContain('◇ Ready (BBBBB');
184-
expect(title).toContain('...)');
185-
});
175+
envTitle: 'B'.repeat(100),
176+
expected: '◇ Ready (BBBBB',
177+
},
178+
])(
179+
'should truncate very long $name to fit within 80 characters',
180+
({ folderName, envTitle, expected }) => {
181+
if (envTitle) {
182+
vi.stubEnv('CLI_TITLE', envTitle);
183+
}
184+
185+
const title = computeTerminalTitle({
186+
streamingState: StreamingState.Idle,
187+
isConfirming: false,
188+
folderName,
189+
showThoughts: false,
190+
useDynamicTitle: true,
191+
});
192+
193+
expect(title.length).toBe(80);
194+
expect(title).toContain(expected);
195+
expect(title).toContain('...)');
196+
},
197+
);
186198

187199
it('should truncate long folder name when useDynamicTitle is false', () => {
188200
const longFolderName = 'C'.repeat(100);

0 commit comments

Comments
 (0)