Skip to content

Commit 84444f7

Browse files
committed
refactor(cli): complete remaining audit items — validate status, extract helpers, add 8 test cases
- Fix #28: Validate ResponsePayload status is string before cast in handleMessage() - Fix #43: Extract settleHandler() private method to reduce duplication - Fix #41: Normalize createMockWS → createMockWs for consistency - Fix #39: Extract captureStream() parameterized helper to replace captureStdout/captureStderr - Fix #47: Filter empty arrays in displayGenericPayload to avoid spurious output - Test #21-#22: Add ConnectionError edge cases (empty cause, non-Error cause) - Test #23: Add ServerFailureError without hashIdsFailed - Test #24: Add post-connect onerror rejection test - Test #25: Add displayGenericPayload failure path test - Test #26: Add NaN and Infinity timeout validation tests All tests pass, zero lint warnings.
1 parent eeeb437 commit 84444f7

5 files changed

Lines changed: 148 additions & 59 deletions

File tree

ui/cli/src/output/table.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,11 @@ export const outputTable = (payload: ResponsePayload): void => {
4242

4343
const displayGenericPayload = (payload: ResponsePayload): void => {
4444
const { status, ...rest } = payload
45-
if (Object.keys(rest).length > 0) {
46-
process.stdout.write(JSON.stringify(rest, null, 2) + '\n')
45+
const meaningful = Object.fromEntries(
46+
Object.entries(rest).filter(([, v]) => v != null && !(Array.isArray(v) && v.length === 0))
47+
)
48+
if (Object.keys(meaningful).length > 0) {
49+
process.stdout.write(JSON.stringify(meaningful, null, 2) + '\n')
4750
} else if (status === ResponseStatus.SUCCESS) {
4851
process.stdout.write(chalk.green('✓ Success\n'))
4952
} else {

ui/cli/tests/lifecycle.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,17 @@ await describe('lifecycle', async () => {
2020
assert.strictEqual(err.cause, cause)
2121
})
2222

23+
await it('should create ConnectionError without cause suffix when cause message is empty', () => {
24+
const err = new ConnectionError('ws://localhost:8080', new Error(''))
25+
assert.strictEqual(err.message, 'Failed to connect to ws://localhost:8080')
26+
})
27+
28+
await it('should create ConnectionError without cause suffix when cause is not an Error', () => {
29+
const err = new ConnectionError('ws://localhost:8080', 'string cause')
30+
assert.strictEqual(err.message, 'Failed to connect to ws://localhost:8080')
31+
assert.strictEqual(err.cause, 'string cause')
32+
})
33+
2334
await it('should export executeCommand function', () => {
2435
assert.strictEqual(typeof executeCommand, 'function')
2536
})

ui/cli/tests/output.test.ts

Lines changed: 29 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -9,32 +9,18 @@ import { printError } from '../src/output/human.js'
99
import { outputJson, outputJsonError } from '../src/output/json.js'
1010
import { outputTable } from '../src/output/table.js'
1111

12-
const captureStdout = (fn: () => void): string => {
12+
const captureStream = (stream: 'stderr' | 'stdout', fn: () => void): string => {
13+
const target = stream === 'stdout' ? process.stdout : process.stderr
1314
const chunks: string[] = []
14-
const original = process.stdout.write.bind(process.stdout)
15-
process.stdout.write = ((chunk: string): boolean => {
15+
const original = target.write.bind(target)
16+
target.write = ((chunk: string): boolean => {
1617
chunks.push(chunk)
1718
return true
18-
}) as typeof process.stdout.write
19+
}) as typeof target.write
1920
try {
2021
fn()
2122
} finally {
22-
process.stdout.write = original
23-
}
24-
return chunks.join('')
25-
}
26-
27-
const captureStderr = (fn: () => void): string => {
28-
const chunks: string[] = []
29-
const original = process.stderr.write.bind(process.stderr)
30-
process.stderr.write = ((chunk: string): boolean => {
31-
chunks.push(chunk)
32-
return true
33-
}) as typeof process.stderr.write
34-
try {
35-
fn()
36-
} finally {
37-
process.stderr.write = original
23+
target.write = original
3824
}
3925
return chunks.join('')
4026
}
@@ -57,7 +43,7 @@ await describe('output formatters', async () => {
5743
hashIdsSucceeded: ['cs-001', 'cs-002'],
5844
status: ResponseStatus.SUCCESS,
5945
}
60-
const output = captureStdout(() => {
46+
const output = captureStream('stdout', () => {
6147
outputJson(payload)
6248
})
6349
const parsed = JSON.parse(output) as typeof payload
@@ -67,15 +53,15 @@ await describe('output formatters', async () => {
6753

6854
await it('should write valid JSON to stdout for failure payload', () => {
6955
const payload = { status: ResponseStatus.FAILURE }
70-
const output = captureStdout(() => {
56+
const output = captureStream('stdout', () => {
7157
outputJson(payload)
7258
})
7359
const parsed = JSON.parse(output) as typeof payload
7460
assert.strictEqual(parsed.status, ResponseStatus.FAILURE)
7561
})
7662

7763
await it('should write error JSON to stdout', () => {
78-
const output = captureStdout(() => {
64+
const output = captureStream('stdout', () => {
7965
outputJsonError(new Error('test error'))
8066
})
8167
const parsed = JSON.parse(output) as { error: boolean; message: string; status: string }
@@ -85,7 +71,7 @@ await describe('output formatters', async () => {
8571
})
8672

8773
await it('should handle non-Error objects in JSON error output', () => {
88-
const output = captureStdout(() => {
74+
const output = captureStream('stdout', () => {
8975
outputJsonError('string error')
9076
})
9177
const parsed = JSON.parse(output) as { message: string }
@@ -97,30 +83,30 @@ await describe('output formatters', async () => {
9783
hashIdsSucceeded: ['cs-001'],
9884
status: ResponseStatus.SUCCESS,
9985
}
100-
const output = captureStdout(() => {
86+
const output = captureStream('stdout', () => {
10187
outputTable(payload)
10288
})
10389
assert.ok(output.includes('cs-001'))
10490
})
10591

10692
await it('should display generic payload when no hash IDs present', () => {
10793
const payload = { state: { version: '1.0' }, status: ResponseStatus.SUCCESS }
108-
const output = captureStdout(() => {
94+
const output = captureStream('stdout', () => {
10995
outputTable(payload)
11096
})
11197
assert.ok(output.includes('version'))
11298
})
11399

114100
await it('should write generic success when no hash IDs in table mode', () => {
115101
const payload = { status: ResponseStatus.SUCCESS }
116-
const output = captureStdout(() => {
102+
const output = captureStream('stdout', () => {
117103
outputTable(payload)
118104
})
119105
assert.ok(output.includes('Success'))
120106
})
121107

122108
await it('should write error message via printError', () => {
123-
const output = captureStderr(() => {
109+
const output = captureStream('stderr', () => {
124110
printError('oops')
125111
})
126112
assert.ok(output.includes('oops'))
@@ -132,7 +118,7 @@ await describe('output formatters', async () => {
132118
hashIdsSucceeded: ['cs-100'],
133119
status: ResponseStatus.SUCCESS,
134120
}
135-
const output = captureStdout(() => {
121+
const output = captureStream('stdout', () => {
136122
formatter.output(payload)
137123
})
138124
const parsed = JSON.parse(output) as typeof payload
@@ -145,15 +131,15 @@ await describe('output formatters', async () => {
145131
hashIdsSucceeded: ['cs-200'],
146132
status: ResponseStatus.SUCCESS,
147133
}
148-
const output = captureStdout(() => {
134+
const output = captureStream('stdout', () => {
149135
formatter.output(payload)
150136
})
151137
assert.ok(output.includes('cs-200'))
152138
})
153139

154140
await it('should handle error with JSON formatter', () => {
155141
const formatter = createFormatter(true)
156-
const output = captureStdout(() => {
142+
const output = captureStream('stdout', () => {
157143
formatter.error(new Error('json err'))
158144
})
159145
const parsed = JSON.parse(output) as { message: string }
@@ -162,7 +148,7 @@ await describe('output formatters', async () => {
162148

163149
await it('should handle error with table formatter', () => {
164150
const formatter = createFormatter(false)
165-
const output = captureStderr(() => {
151+
const output = captureStream('stderr', () => {
166152
formatter.error(new Error('table err'))
167153
})
168154
assert.ok(output.includes('table err'))
@@ -181,7 +167,7 @@ await describe('output formatters', async () => {
181167
],
182168
status: ResponseStatus.FAILURE,
183169
}
184-
const output = captureStderr(() => {
170+
const output = captureStream('stderr', () => {
185171
outputTable(payload)
186172
})
187173
assert.ok(output.includes('Station not found'))
@@ -201,7 +187,7 @@ await describe('output formatters', async () => {
201187
],
202188
status: ResponseStatus.FAILURE,
203189
}
204-
const output = captureStderr(() => {
190+
const output = captureStream('stderr', () => {
205191
outputTable(payload)
206192
})
207193
assert.ok(output.includes('Unknown error'))
@@ -221,7 +207,7 @@ await describe('output formatters', async () => {
221207
],
222208
status: ResponseStatus.FAILURE,
223209
}
224-
const output = captureStderr(() => {
210+
const output = captureStream('stderr', () => {
225211
outputTable(payload)
226212
})
227213
assert.ok(output.includes('(unknown)'))
@@ -233,10 +219,17 @@ await describe('output formatters', async () => {
233219
hashIdsFailed: ['cs-001'],
234220
status: ResponseStatus.FAILURE,
235221
}
236-
const output = captureStderr(() => {
222+
const output = captureStream('stderr', () => {
237223
outputTable(payload)
238224
})
239225
assert.ok(output.includes('cs-001'))
240226
assert.ok(!output.includes('Error'))
241227
})
228+
229+
await it('should display generic failure status on stderr', () => {
230+
const output = captureStream('stderr', () => {
231+
outputTable({ status: ResponseStatus.FAILURE })
232+
})
233+
assert.ok(output.includes('failure'))
234+
})
242235
})

ui/common/src/client/WebSocketClient.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -170,20 +170,24 @@ export class WebSocketClient {
170170
if (
171171
responsePayload == null ||
172172
typeof responsePayload !== 'object' ||
173-
!('status' in responsePayload)
173+
!('status' in responsePayload) ||
174+
typeof (responsePayload as { status: unknown }).status !== 'string'
174175
) {
175-
clearTimeout(handler.timeoutId)
176-
this.responseHandlers.delete(uuid)
176+
this.settleHandler(uuid, handler)
177177
handler.reject(new Error('Server sent malformed response payload'))
178178
return
179179
}
180-
clearTimeout(handler.timeoutId)
181-
this.responseHandlers.delete(uuid)
180+
this.settleHandler(uuid, handler)
182181
const payload = responsePayload as ResponsePayload
183182
if (payload.status === ResponseStatus.SUCCESS) {
184183
handler.resolve(payload)
185184
} else {
186185
handler.reject(new ServerFailureError(payload))
187186
}
188187
}
188+
189+
private settleHandler (uuid: UUIDv4, handler: ResponseHandler): void {
190+
clearTimeout(handler.timeoutId)
191+
this.responseHandlers.delete(uuid)
192+
}
189193
}

0 commit comments

Comments
 (0)