Skip to content

Commit 81bce84

Browse files
nigel-devnigel-dev
committed
feat(merge-train): surface test setup execution details for merge-gate visibility
- Add MergeTestReport type to expose setup command execution status - Track setup command success/failure/skip status in test results - Include setup output in merge result for user verification - Add test coverage for setup execution reporting in merge scenarios This enables users to verify merge-gate behavior by inspecting which setup commands ran, their status, and any output, improving debugging and transparency of the merge-train test gating process.
1 parent 9a15d24 commit 81bce84

4 files changed

Lines changed: 253 additions & 4 deletions

File tree

‎src/lib/merge-train.ts‎

Lines changed: 89 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,30 @@ import { getIntegrationWorktree } from './integration';
66
import { extractConflicts } from './utils';
77

88
export type MergeResult =
9-
| { success: true; mergedAt: string }
9+
| { success: true; mergedAt: string; testReport: MergeTestReport }
1010
| {
1111
success: false;
1212
type: 'conflict' | 'test_failure';
1313
files?: string[];
1414
output?: string;
15+
testReport?: MergeTestReport;
1516
};
1617

18+
type MergeSetupReport = {
19+
status: 'passed' | 'failed' | 'skipped';
20+
commands: string[];
21+
output?: string;
22+
};
23+
24+
export type MergeTestReport = {
25+
status: 'passed' | 'failed' | 'skipped';
26+
command?: string;
27+
output?: string;
28+
timedOut?: boolean;
29+
reason?: string;
30+
setup: MergeSetupReport;
31+
};
32+
1733
type MergeTrainConfig = {
1834
testCommand?: string;
1935
testTimeout?: number;
@@ -166,7 +182,13 @@ async function ensureTestDependencies(
166182
worktreePath: string,
167183
timeoutMs: number,
168184
setupCommands?: string[],
169-
): Promise<{ success: boolean; output: string; timedOut: boolean }> {
185+
): Promise<{
186+
success: boolean;
187+
output: string;
188+
timedOut: boolean;
189+
commands: string[];
190+
status: 'passed' | 'failed' | 'skipped';
191+
}> {
170192
const configuredSetupCommands = normalizeCommands(setupCommands);
171193
if (configuredSetupCommands.length > 0) {
172194
for (const command of configuredSetupCommands) {
@@ -181,6 +203,8 @@ async function ensureTestDependencies(
181203

182204
return {
183205
...setupResult,
206+
commands: configuredSetupCommands,
207+
status: 'failed',
184208
output: setupResult.output
185209
? `${prefix} (${command})\n${setupResult.output}`
186210
: `${prefix} (${command})`,
@@ -191,6 +215,8 @@ async function ensureTestDependencies(
191215
success: true,
192216
output: '',
193217
timedOut: false,
218+
commands: configuredSetupCommands,
219+
status: 'passed',
194220
};
195221
}
196222

@@ -200,6 +226,8 @@ async function ensureTestDependencies(
200226
success: true,
201227
output: '',
202228
timedOut: false,
229+
commands: [],
230+
status: 'skipped',
203231
};
204232
}
205233

@@ -209,6 +237,8 @@ async function ensureTestDependencies(
209237
success: true,
210238
output: '',
211239
timedOut: false,
240+
commands: [],
241+
status: 'skipped',
212242
};
213243
}
214244

@@ -221,7 +251,11 @@ async function ensureTestDependencies(
221251

222252
const installResult = await runTestCommand(worktreePath, installCommand, timeoutMs);
223253
if (installResult.success) {
224-
return installResult;
254+
return {
255+
...installResult,
256+
commands: [installCommand],
257+
status: 'passed',
258+
};
225259
}
226260

227261
const prefix = installResult.timedOut
@@ -230,6 +264,8 @@ async function ensureTestDependencies(
230264

231265
return {
232266
...installResult,
267+
commands: [installCommand],
268+
status: 'failed',
233269
output: installResult.output
234270
? `${prefix} (${installCommand})\n${installResult.output}`
235271
: `${prefix} (${installCommand})`,
@@ -400,6 +436,14 @@ export class MergeTrain {
400436
return {
401437
success: true,
402438
mergedAt: new Date().toISOString(),
439+
testReport: {
440+
status: 'skipped',
441+
reason: 'No test command configured or detected',
442+
setup: {
443+
status: 'skipped',
444+
commands: [],
445+
},
446+
},
403447
};
404448
}
405449

@@ -416,6 +460,16 @@ export class MergeTrain {
416460
success: false,
417461
type: 'test_failure',
418462
output: dependencySetupResult.output,
463+
testReport: {
464+
status: 'skipped',
465+
command: testCommand,
466+
reason: 'Dependency setup failed before tests could run',
467+
setup: {
468+
status: dependencySetupResult.status,
469+
commands: dependencySetupResult.commands,
470+
output: dependencySetupResult.output,
471+
},
472+
},
419473
};
420474
}
421475

@@ -427,6 +481,18 @@ export class MergeTrain {
427481
success: false,
428482
type: 'test_failure',
429483
output: `Test timed out after ${timeoutMs}ms`,
484+
testReport: {
485+
status: 'failed',
486+
command: testCommand,
487+
timedOut: true,
488+
output: testResult.output,
489+
reason: `Test timed out after ${timeoutMs}ms`,
490+
setup: {
491+
status: dependencySetupResult.status,
492+
commands: dependencySetupResult.commands,
493+
output: dependencySetupResult.output || undefined,
494+
},
495+
},
430496
};
431497
}
432498

@@ -436,12 +502,32 @@ export class MergeTrain {
436502
success: false,
437503
type: 'test_failure',
438504
output: testResult.output,
505+
testReport: {
506+
status: 'failed',
507+
command: testCommand,
508+
output: testResult.output,
509+
setup: {
510+
status: dependencySetupResult.status,
511+
commands: dependencySetupResult.commands,
512+
output: dependencySetupResult.output || undefined,
513+
},
514+
},
439515
};
440516
}
441517

442518
return {
443519
success: true,
444520
mergedAt: new Date().toISOString(),
521+
testReport: {
522+
status: 'passed',
523+
command: testCommand,
524+
output: testResult.output || undefined,
525+
setup: {
526+
status: dependencySetupResult.status,
527+
commands: dependencySetupResult.commands,
528+
output: dependencySetupResult.output || undefined,
529+
},
530+
},
445531
};
446532
}
447533

‎src/lib/orchestrator.ts‎

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type { PlanSpec, JobSpec, PlanStatus, CheckpointType } from './plan-types
44
import { loadPlan, savePlan, updatePlanJob, clearPlan, validateGhAuth } from './plan-state';
55
import { getDefaultBranch } from './git';
66
import { createIntegrationBranch, deleteIntegrationBranch } from './integration';
7-
import { MergeTrain } from './merge-train';
7+
import { MergeTrain, type MergeTestReport } from './merge-train';
88
import { addJob, getRunningJobs, updateJob, loadJobState, removeJob, type Job } from './job-state';
99
import { JobMonitor } from './monitor';
1010
import { removeReport } from './reports';
@@ -40,6 +40,23 @@ function isTerminalPlanStatus(status: PlanStatus): boolean {
4040
return TERMINAL_PLAN_STATUSES.includes(status);
4141
}
4242

43+
function compactOutput(output?: string, maxLength = 180): string | null {
44+
if (!output) {
45+
return null;
46+
}
47+
48+
const normalized = output.replace(/\s+/g, ' ').trim();
49+
if (!normalized) {
50+
return null;
51+
}
52+
53+
if (normalized.length <= maxLength) {
54+
return normalized;
55+
}
56+
57+
return `${normalized.slice(0, maxLength - 3)}...`;
58+
}
59+
4360
function toAdjacencyMap(jobs: JobSpec[]): Map<string, string[]> {
4461
const map = new Map<string, string[]>();
4562
for (const job of jobs) {
@@ -205,6 +222,54 @@ export class Orchestrator {
205222
this.notifyCallback(message);
206223
}
207224

225+
private formatTestReportSummary(testReport?: MergeTestReport): string | null {
226+
if (!testReport) {
227+
return null;
228+
}
229+
230+
const parts: string[] = [];
231+
232+
if (testReport.status === 'skipped') {
233+
parts.push('tests skipped');
234+
} else if (testReport.status === 'passed') {
235+
parts.push('tests passed');
236+
} else {
237+
parts.push('tests failed');
238+
}
239+
240+
if (testReport.command) {
241+
parts.push(`command: ${testReport.command}`);
242+
}
243+
244+
if (testReport.setup.status === 'passed') {
245+
if (testReport.setup.commands.length > 0) {
246+
parts.push(`setup passed: ${testReport.setup.commands.join(' && ')}`);
247+
} else {
248+
parts.push('setup skipped');
249+
}
250+
} else if (testReport.setup.status === 'failed') {
251+
parts.push(`setup failed: ${testReport.setup.commands.join(' && ')}`);
252+
} else {
253+
parts.push('setup skipped');
254+
}
255+
256+
if (testReport.reason) {
257+
parts.push(`reason: ${testReport.reason}`);
258+
}
259+
260+
const setupSnippet = compactOutput(testReport.setup.output);
261+
if (setupSnippet) {
262+
parts.push(`setup output: ${setupSnippet}`);
263+
}
264+
265+
const testSnippet = compactOutput(testReport.output);
266+
if (testSnippet) {
267+
parts.push(`test output: ${testSnippet}`);
268+
}
269+
270+
return parts.join(' | ');
271+
}
272+
208273
getCheckpoint(): CheckpointType | null {
209274
return this.checkpoint;
210275
}
@@ -449,6 +514,10 @@ export class Orchestrator {
449514
}
450515
this.showToast('Mission Control', `Job "${nextJob.name}" merged successfully.`, 'success');
451516
this.notify(`✅ Job "${nextJob.name}" merged successfully. (${mergedCount + 1}/${mergeOrder.length} merged)`);
517+
const testSummary = this.formatTestReportSummary(mergeResult.testReport);
518+
if (testSummary) {
519+
this.notify(`🧪 ${nextJob.name}: ${testSummary}`);
520+
}
452521
} else if (mergeResult.type === 'conflict') {
453522
await updatePlanJob(plan.id, nextJob.name, {
454523
status: 'conflict',
@@ -469,6 +538,11 @@ export class Orchestrator {
469538
error: mergeResult.output ?? 'merge train test failure',
470539
});
471540

541+
const testSummary = this.formatTestReportSummary(mergeResult.testReport);
542+
if (testSummary) {
543+
this.notify(`🧪 ${nextJob.name}: ${testSummary}`);
544+
}
545+
472546
if (this.isSupervisor(plan)) {
473547
await this.setCheckpoint('on_error', plan);
474548
return;

‎tests/lib/merge-train.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,11 @@ describe('MergeTrain', () => {
113113
const result = await train.processNext();
114114

115115
expect(result.success).toBe(true);
116+
if (result.success) {
117+
expect(result.testReport.status).toBe('passed');
118+
expect(result.testReport.command).toBe('true');
119+
expect(result.testReport.setup.status).toBe('skipped');
120+
}
116121
const status = await mustExec(
117122
['git', 'status', '--porcelain'],
118123
testRepo.integrationWorktree,
@@ -254,9 +259,35 @@ describe('MergeTrain', () => {
254259
const result = await train.processNext();
255260

256261
expect(result.success).toBe(true);
262+
if (result.success) {
263+
expect(result.testReport.setup.status).toBe('passed');
264+
expect(result.testReport.setup.commands).toEqual(['touch .deps-ready']);
265+
}
257266
expect(existsSync(join(testRepo.integrationWorktree, '.deps-ready'))).toBe(true);
258267
});
259268

269+
it('reports skipped tests when no test command is configured or detected', async () => {
270+
await createBranchCommit(testRepo.repoDir, 'feature-skip-tests', 'skip.txt', 'skip\n');
271+
272+
rmSync(join(testRepo.integrationWorktree, 'package.json'), {
273+
force: true,
274+
});
275+
276+
const train = new MergeTrain(testRepo.integrationWorktree, {
277+
testTimeout: 60000,
278+
});
279+
train.enqueue(makeJob('feature-skip-tests'));
280+
281+
const result = await train.processNext();
282+
283+
expect(result.success).toBe(true);
284+
if (result.success) {
285+
expect(result.testReport.status).toBe('skipped');
286+
expect(result.testReport.reason).toContain('No test command configured or detected');
287+
expect(result.testReport.setup.status).toBe('skipped');
288+
}
289+
});
290+
260291
it('rolls back merge when setup command fails', async () => {
261292
await createBranchCommit(
262293
testRepo.repoDir,
@@ -283,6 +314,8 @@ describe('MergeTrain', () => {
283314
if (!result.success) {
284315
expect(result.type).toBe('test_failure');
285316
expect(result.output).toContain('Dependency setup command failed');
317+
expect(result.testReport?.status).toBe('skipped');
318+
expect(result.testReport?.setup.status).toBe('failed');
286319
}
287320

288321
const headAfter = await mustExec(
@@ -312,6 +345,8 @@ describe('MergeTrain', () => {
312345
expect(result.success).toBe(false);
313346
if (!result.success) {
314347
expect(result.type).toBe('test_failure');
348+
expect(result.testReport?.status).toBe('failed');
349+
expect(result.testReport?.command).toBe('false');
315350
}
316351

317352
const headAfter = await mustExec(

0 commit comments

Comments
 (0)