Skip to content

Commit 4500359

Browse files
committed
✨ Make finalize command resilient to missing builds
The finalize command now handles 404 errors gracefully instead of failing CI. When no build exists for a parallel ID (e.g., tests were skipped or no screenshots uploaded), it warns and exits 0. Added global --strict flag for users who want hard failures on any error. This flag is available on all commands. Closes VIZ-108
1 parent eeda9fb commit 4500359

3 files changed

Lines changed: 198 additions & 2 deletions

File tree

src/cli.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,10 @@ program
236236
.option('--json', 'Machine-readable output')
237237
.option('--color', 'Force colored output (even in non-TTY)')
238238
.option('--no-color', 'Disable colored output')
239+
.option(
240+
'--strict',
241+
'Fail on any error (default: be resilient, warn on non-critical issues)'
242+
)
239243
.configureHelp({ formatHelp });
240244

241245
// Load plugins before defining commands

src/commands/finalize.js

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@ import { loadConfig as defaultLoadConfig } from '../utils/config-loader.js';
1111
import * as defaultOutput from '../utils/output.js';
1212
import { writeSession as defaultWriteSession } from '../utils/session.js';
1313

14+
let MISSING_BUILD_HINTS = [
15+
' • No screenshots were uploaded with this parallel-id',
16+
' • Tests were skipped or failed before capturing screenshots',
17+
' • The parallel-id does not match what was used during test runs',
18+
];
19+
1420
/**
1521
* Finalize command implementation
1622
* @param {string} parallelId - Parallel ID to finalize
@@ -96,11 +102,49 @@ export async function finalizeCommand(
96102
} catch (error) {
97103
output.stopSpinner();
98104

99-
// Don't fail CI for Vizzly infrastructure issues (5xx errors)
100105
let status = error.context?.status;
106+
107+
// Don't fail CI for Vizzly infrastructure issues (5xx errors)
108+
// Note: --strict does NOT affect 5xx handling - infrastructure issues are out of user's control
101109
if (status >= 500) {
102110
output.warn('Vizzly API unavailable - finalize skipped.');
103-
return { success: true, result: { skipped: true } };
111+
return {
112+
success: true,
113+
result: { skipped: true, reason: 'api-unavailable' },
114+
};
115+
}
116+
117+
// Handle missing builds gracefully (404 errors)
118+
// This happens when: no screenshots were uploaded, tests were skipped, or parallel-id doesn't exist
119+
if (status === 404) {
120+
let isStrict = globalOptions.strict;
121+
122+
if (isStrict) {
123+
output.error(`No build found for parallel ID: ${parallelId}`);
124+
output.blank();
125+
output.info('This can happen when:');
126+
for (let hint of MISSING_BUILD_HINTS) {
127+
output.info(hint);
128+
}
129+
exit(1);
130+
return { success: false, reason: 'no-build-found', error };
131+
}
132+
133+
// Non-strict mode: warn but don't fail CI
134+
output.warn(
135+
`No build found for parallel ID: ${parallelId} - finalize skipped.`
136+
);
137+
if (globalOptions.verbose) {
138+
output.info('Possible reasons:');
139+
for (let hint of MISSING_BUILD_HINTS) {
140+
output.info(hint);
141+
}
142+
output.info('Use --strict flag to fail CI when no build is found.');
143+
}
144+
return {
145+
success: true,
146+
result: { skipped: true, reason: 'no-build-found' },
147+
};
104148
}
105149

106150
output.error('Failed to finalize parallel build', error);

tests/commands/finalize.test.js

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,12 +332,160 @@ describe('commands/finalize', () => {
332332

333333
assert.strictEqual(result.success, true);
334334
assert.strictEqual(result.result.skipped, true);
335+
assert.strictEqual(result.result.reason, 'api-unavailable');
335336
assert.strictEqual(exitCode, null);
336337
assert.ok(
337338
output.calls.some(
338339
c => c.method === 'warn' && c.args[0].includes('API unavailable')
339340
)
340341
);
341342
});
343+
344+
it('does not fail CI on 5xx even with --strict flag', async () => {
345+
let output = createMockOutput();
346+
let exitCode = null;
347+
348+
let apiError = new Error('API request failed: 503 - Service Unavailable');
349+
apiError.context = { status: 503 };
350+
351+
let result = await finalizeCommand(
352+
'parallel-123',
353+
{},
354+
{ strict: true },
355+
{
356+
loadConfig: async () => ({
357+
apiKey: 'test-token',
358+
apiUrl: 'https://api.test',
359+
}),
360+
createApiClient: () => ({ request: async () => ({}) }),
361+
finalizeParallelBuild: async () => {
362+
throw apiError;
363+
},
364+
output,
365+
exit: code => {
366+
exitCode = code;
367+
},
368+
}
369+
);
370+
371+
// 5xx errors should ALWAYS be resilient, even with --strict
372+
// Infrastructure issues are out of user's control
373+
assert.strictEqual(result.success, true);
374+
assert.strictEqual(result.result.skipped, true);
375+
assert.strictEqual(result.result.reason, 'api-unavailable');
376+
assert.strictEqual(exitCode, null);
377+
});
378+
379+
it('does not fail CI when no build found (404) in non-strict mode', async () => {
380+
let output = createMockOutput();
381+
let exitCode = null;
382+
383+
let apiError = new Error('API request failed: 404 - Not Found');
384+
apiError.context = { status: 404 };
385+
386+
let result = await finalizeCommand(
387+
'parallel-123',
388+
{},
389+
{},
390+
{
391+
loadConfig: async () => ({
392+
apiKey: 'test-token',
393+
apiUrl: 'https://api.test',
394+
}),
395+
createApiClient: () => ({ request: async () => ({}) }),
396+
finalizeParallelBuild: async () => {
397+
throw apiError;
398+
},
399+
output,
400+
exit: code => {
401+
exitCode = code;
402+
},
403+
}
404+
);
405+
406+
assert.strictEqual(result.success, true);
407+
assert.strictEqual(result.result.skipped, true);
408+
assert.strictEqual(result.result.reason, 'no-build-found');
409+
assert.strictEqual(exitCode, null);
410+
assert.ok(
411+
output.calls.some(
412+
c => c.method === 'warn' && c.args[0].includes('No build found')
413+
)
414+
);
415+
});
416+
417+
it('fails CI when no build found (404) in strict mode', async () => {
418+
let output = createMockOutput();
419+
let exitCode = null;
420+
421+
let apiError = new Error('API request failed: 404 - Not Found');
422+
apiError.context = { status: 404 };
423+
424+
let result = await finalizeCommand(
425+
'parallel-123',
426+
{},
427+
{ strict: true },
428+
{
429+
loadConfig: async () => ({
430+
apiKey: 'test-token',
431+
apiUrl: 'https://api.test',
432+
}),
433+
createApiClient: () => ({ request: async () => ({}) }),
434+
finalizeParallelBuild: async () => {
435+
throw apiError;
436+
},
437+
output,
438+
exit: code => {
439+
exitCode = code;
440+
},
441+
}
442+
);
443+
444+
assert.strictEqual(result.success, false);
445+
assert.strictEqual(result.reason, 'no-build-found');
446+
assert.strictEqual(exitCode, 1);
447+
assert.ok(
448+
output.calls.some(
449+
c => c.method === 'error' && c.args[0].includes('No build found')
450+
)
451+
);
452+
});
453+
454+
it('shows verbose hints when no build found in non-strict mode', async () => {
455+
let output = createMockOutput();
456+
457+
let apiError = new Error('API request failed: 404 - Not Found');
458+
apiError.context = { status: 404 };
459+
460+
await finalizeCommand(
461+
'parallel-123',
462+
{},
463+
{ verbose: true },
464+
{
465+
loadConfig: async () => ({
466+
apiKey: 'test-token',
467+
apiUrl: 'https://api.test',
468+
}),
469+
createApiClient: () => ({ request: async () => ({}) }),
470+
finalizeParallelBuild: async () => {
471+
throw apiError;
472+
},
473+
output,
474+
exit: () => {},
475+
}
476+
);
477+
478+
// Should show helpful hints in verbose mode
479+
assert.ok(
480+
output.calls.some(
481+
c => c.method === 'info' && c.args[0].includes('Possible reasons')
482+
)
483+
);
484+
assert.ok(
485+
output.calls.some(
486+
c => c.method === 'info' && c.args[0].includes('--strict')
487+
)
488+
);
489+
});
342490
});
343491
});

0 commit comments

Comments
 (0)