feat: Add status check functions for scaffolder steps (#32890)
* feat: Add status check functions for scaffolder steps - Introduced `always()` and `failure()` functions to control step execution after failures. - Updated documentation to explain usage of new status check functions. - Enhanced NunjucksWorkflowRunner to process these functions in step conditions. - Added tests to verify behavior of steps using `always()` and `failure()`. Signed-off-by: ferin79 <ferinpatel79@gmail.com> * feat: Enhance status check functions in scaffolder steps - Updated documentation to clarify usage of status check functions with template expressions. - Modified tests to reflect changes in syntax for status checks. - Refactored NunjucksWorkflowRunner to ensure proper handling of status check functions in step conditions. Signed-off-by: ferin79 <ferinpatel79@gmail.com> * docs: Clarify usage of status check functions in writing templates - Removed redundant explanation about truthy conditions after step failure. - Streamlined the description for better clarity on status check functions. Signed-off-by: ferin79 <ferinpatel79@gmail.com> --------- Signed-off-by: ferin79 <ferinpatel79@gmail.com>
This commit is contained in:
@@ -2297,4 +2297,351 @@ describe('NunjucksWorkflowRunner', () => {
|
||||
expect(mockedPermissionApi.authorizeConditional).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('step status check functions (always/failure)', () => {
|
||||
let failingHandler: jest.Mock;
|
||||
let cleanupHandler: jest.Mock;
|
||||
|
||||
beforeEach(() => {
|
||||
failingHandler = jest.fn().mockRejectedValue(new Error('step failed'));
|
||||
cleanupHandler = jest.fn();
|
||||
|
||||
actionRegistry.register(
|
||||
createTemplateAction({
|
||||
id: 'failing-action',
|
||||
description: 'Action that always fails',
|
||||
handler: failingHandler,
|
||||
}),
|
||||
);
|
||||
|
||||
actionRegistry.register(
|
||||
createTemplateAction({
|
||||
id: 'cleanup-action',
|
||||
description: 'Cleanup action',
|
||||
handler: cleanupHandler,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should run step with if: ${{ always() }} even when a previous step failed', async () => {
|
||||
const task = createMockTaskWithSpec({
|
||||
steps: [
|
||||
{
|
||||
id: 'step1',
|
||||
name: 'Failing step',
|
||||
action: 'failing-action',
|
||||
},
|
||||
{
|
||||
id: 'step2',
|
||||
name: 'Always runs',
|
||||
action: 'cleanup-action',
|
||||
if: '${{ always() }}',
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
await expect(runner.execute(task)).rejects.toThrow('step failed');
|
||||
expect(cleanupHandler).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('should run step with if: ${{ failure() }} only when a previous step failed', async () => {
|
||||
const task = createMockTaskWithSpec({
|
||||
steps: [
|
||||
{
|
||||
id: 'step1',
|
||||
name: 'Failing step',
|
||||
action: 'failing-action',
|
||||
},
|
||||
{
|
||||
id: 'step2',
|
||||
name: 'Runs on failure',
|
||||
action: 'cleanup-action',
|
||||
if: '${{ failure() }}',
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
await expect(runner.execute(task)).rejects.toThrow('step failed');
|
||||
expect(cleanupHandler).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('should not run step with if: ${{ failure() }} when no step has failed', async () => {
|
||||
const task = createMockTaskWithSpec({
|
||||
steps: [
|
||||
{
|
||||
id: 'step1',
|
||||
name: 'Succeeding step',
|
||||
action: 'jest-mock-action',
|
||||
},
|
||||
{
|
||||
id: 'step2',
|
||||
name: 'Only on failure',
|
||||
action: 'cleanup-action',
|
||||
if: '${{ failure() }}',
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
await runner.execute(task);
|
||||
expect(fakeActionHandler).toHaveBeenCalledTimes(1);
|
||||
expect(cleanupHandler).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not run step with if: ${{ true }} after a previous step failed', async () => {
|
||||
const task = createMockTaskWithSpec({
|
||||
steps: [
|
||||
{
|
||||
id: 'step1',
|
||||
name: 'Failing step',
|
||||
action: 'failing-action',
|
||||
},
|
||||
{
|
||||
id: 'step2',
|
||||
name: 'Truthy but not a status check',
|
||||
action: 'cleanup-action',
|
||||
if: '${{ true }}',
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
await expect(runner.execute(task)).rejects.toThrow('step failed');
|
||||
expect(cleanupHandler).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should still throw the original error after running ${{ always() }} steps', async () => {
|
||||
const task = createMockTaskWithSpec({
|
||||
steps: [
|
||||
{
|
||||
id: 'step1',
|
||||
name: 'Failing step',
|
||||
action: 'failing-action',
|
||||
},
|
||||
{
|
||||
id: 'step2',
|
||||
name: 'Always step',
|
||||
action: 'cleanup-action',
|
||||
if: '${{ always() }}',
|
||||
},
|
||||
{
|
||||
id: 'step3',
|
||||
name: 'Should be skipped',
|
||||
action: 'jest-mock-action',
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
await expect(runner.execute(task)).rejects.toThrow('step failed');
|
||||
expect(cleanupHandler).toHaveBeenCalledTimes(1);
|
||||
// step3 should not run because it has no status check function
|
||||
expect(fakeActionHandler).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should continue running always() steps even if a cleanup step also fails', async () => {
|
||||
const failingCleanup = jest
|
||||
.fn()
|
||||
.mockRejectedValue(new Error('cleanup failed'));
|
||||
actionRegistry.register(
|
||||
createTemplateAction({
|
||||
id: 'failing-cleanup',
|
||||
description: 'Failing cleanup',
|
||||
handler: failingCleanup,
|
||||
}),
|
||||
);
|
||||
|
||||
const task = createMockTaskWithSpec({
|
||||
steps: [
|
||||
{
|
||||
id: 'step1',
|
||||
name: 'Failing step',
|
||||
action: 'failing-action',
|
||||
},
|
||||
{
|
||||
id: 'step2',
|
||||
name: 'Failing cleanup',
|
||||
action: 'failing-cleanup',
|
||||
if: '${{ always() }}',
|
||||
},
|
||||
{
|
||||
id: 'step3',
|
||||
name: 'Another cleanup',
|
||||
action: 'cleanup-action',
|
||||
if: '${{ always() }}',
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
// Should throw the first error (from step1)
|
||||
await expect(runner.execute(task)).rejects.toThrow('step failed');
|
||||
expect(failingCleanup).toHaveBeenCalledTimes(1);
|
||||
expect(cleanupHandler).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('should log all errors when multiple cleanup steps fail', async () => {
|
||||
const secondCleanupError = new Error('second cleanup failed');
|
||||
const thirdCleanupError = new Error('third cleanup failed');
|
||||
|
||||
const failingCleanup2 = jest.fn().mockRejectedValue(secondCleanupError);
|
||||
const failingCleanup3 = jest.fn().mockRejectedValue(thirdCleanupError);
|
||||
|
||||
actionRegistry.register(
|
||||
createTemplateAction({
|
||||
id: 'failing-cleanup-2',
|
||||
description: 'Second failing cleanup',
|
||||
handler: failingCleanup2,
|
||||
}),
|
||||
);
|
||||
|
||||
actionRegistry.register(
|
||||
createTemplateAction({
|
||||
id: 'failing-cleanup-3',
|
||||
description: 'Third failing cleanup',
|
||||
handler: failingCleanup3,
|
||||
}),
|
||||
);
|
||||
|
||||
const task = createMockTaskWithSpec({
|
||||
steps: [
|
||||
{
|
||||
id: 'step1',
|
||||
name: 'Failing step',
|
||||
action: 'failing-action',
|
||||
},
|
||||
{
|
||||
id: 'step2',
|
||||
name: 'First cleanup',
|
||||
action: 'failing-cleanup-2',
|
||||
if: '${{ always() }}',
|
||||
},
|
||||
{
|
||||
id: 'step3',
|
||||
name: 'Second cleanup',
|
||||
action: 'failing-cleanup-3',
|
||||
if: '${{ always() }}',
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
// Should throw the first error (from step1)
|
||||
await expect(runner.execute(task)).rejects.toThrow('step failed');
|
||||
|
||||
// All cleanup handlers should have been called
|
||||
expect(failingCleanup2).toHaveBeenCalledTimes(1);
|
||||
expect(failingCleanup3).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Subsequent errors should be logged
|
||||
expect(logger.error).toHaveBeenCalledWith(
|
||||
'Additional error in step step2 (First cleanup): second cleanup failed',
|
||||
secondCleanupError,
|
||||
);
|
||||
expect(logger.error).toHaveBeenCalledWith(
|
||||
'Additional error in step step3 (Second cleanup): third cleanup failed',
|
||||
thirdCleanupError,
|
||||
);
|
||||
|
||||
// Summary warning should be logged
|
||||
expect(logger.warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining(
|
||||
'Task failed with 3 errors. First error from step step1. Additional failures in: step2 (First cleanup), step3 (Second cleanup)',
|
||||
),
|
||||
);
|
||||
|
||||
// Task logs should contain additional error information
|
||||
expect(fakeTaskLog).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Additional error occurred'),
|
||||
{ stepId: 'step2', status: 'failed' },
|
||||
);
|
||||
expect(fakeTaskLog).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Additional error occurred'),
|
||||
{ stepId: 'step3', status: 'failed' },
|
||||
);
|
||||
});
|
||||
|
||||
it('should support failure() and always() together across multiple steps', async () => {
|
||||
const task = createMockTaskWithSpec({
|
||||
steps: [
|
||||
{
|
||||
id: 'step1',
|
||||
name: 'First step',
|
||||
action: 'jest-mock-action',
|
||||
},
|
||||
{
|
||||
id: 'step2',
|
||||
name: 'Should skip with template failure',
|
||||
action: 'cleanup-action',
|
||||
if: '${{ failure() }}',
|
||||
},
|
||||
{
|
||||
id: 'step3',
|
||||
name: 'Should run with template always',
|
||||
action: 'cleanup-action',
|
||||
if: '${{ always() }}',
|
||||
},
|
||||
{
|
||||
id: 'step4',
|
||||
name: 'Failing step',
|
||||
action: 'failing-action',
|
||||
},
|
||||
{
|
||||
id: 'step5',
|
||||
name: 'Should run with template failure after error',
|
||||
action: 'cleanup-action',
|
||||
if: '${{ failure() }}',
|
||||
},
|
||||
{
|
||||
id: 'step6',
|
||||
name: 'Should run with template always after error',
|
||||
action: 'cleanup-action',
|
||||
if: '${{ always() }}',
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
await expect(runner.execute(task)).rejects.toThrow('step failed');
|
||||
|
||||
// Verify execution order and counts
|
||||
expect(fakeActionHandler).toHaveBeenCalledTimes(1); // step1
|
||||
expect(cleanupHandler).toHaveBeenCalledTimes(3); // step3, step5, step6
|
||||
|
||||
// Verify the correct steps ran in the right order
|
||||
const taskLogCalls = fakeTaskLog.mock.calls.map(args =>
|
||||
stripAnsi(args[0]),
|
||||
);
|
||||
|
||||
// step1 should run
|
||||
expect(taskLogCalls).toContain('Beginning step First step');
|
||||
expect(taskLogCalls).toContain('Finished step First step');
|
||||
|
||||
// step2 should be skipped (no failure yet)
|
||||
expect(taskLogCalls).toContain(
|
||||
'Skipping step step2 because its if condition was false',
|
||||
);
|
||||
|
||||
// step3 should run (always)
|
||||
expect(taskLogCalls).toContain(
|
||||
'Beginning step Should run with template always',
|
||||
);
|
||||
expect(taskLogCalls).toContain(
|
||||
'Finished step Should run with template always',
|
||||
);
|
||||
|
||||
// step4 should fail
|
||||
expect(taskLogCalls).toContain('Beginning step Failing step');
|
||||
|
||||
// step5 should run (failure condition met)
|
||||
expect(taskLogCalls).toContain(
|
||||
'Beginning step Should run with template failure after error',
|
||||
);
|
||||
expect(taskLogCalls).toContain(
|
||||
'Finished step Should run with template failure after error',
|
||||
);
|
||||
|
||||
// step6 should run (always)
|
||||
expect(taskLogCalls).toContain(
|
||||
'Beginning step Should run with template always after error',
|
||||
);
|
||||
expect(taskLogCalls).toContain(
|
||||
'Finished step Should run with template always after error',
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -644,12 +644,28 @@ export class NunjucksWorkflowRunner implements WorkflowRunner {
|
||||
|
||||
this.environment = await this.getEnvironmentConfig();
|
||||
|
||||
// Track whether any step has failed, used by status check functions
|
||||
const taskState = { failed: false };
|
||||
|
||||
// Track whether a status check global (always/failure) was invoked during rendering
|
||||
const statusCheckInvoked = { value: false };
|
||||
|
||||
const renderTemplate = await SecureTemplater.loadRenderer({
|
||||
templateFilters: {
|
||||
...this.defaultTemplateFilters,
|
||||
...additionalTemplateFilters,
|
||||
},
|
||||
templateGlobals: additionalTemplateGlobals,
|
||||
templateGlobals: {
|
||||
...additionalTemplateGlobals,
|
||||
always: () => {
|
||||
statusCheckInvoked.value = true;
|
||||
return true;
|
||||
},
|
||||
failure: () => {
|
||||
statusCheckInvoked.value = true;
|
||||
return taskState.failed;
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
try {
|
||||
@@ -681,16 +697,77 @@ export class NunjucksWorkflowRunner implements WorkflowRunner {
|
||||
)
|
||||
: [{ result: AuthorizeResult.ALLOW }];
|
||||
|
||||
let firstError: Error | undefined;
|
||||
const allErrors: Array<{ step: TaskStep; error: Error }> = [];
|
||||
|
||||
for (const step of task.spec.steps) {
|
||||
await this.executeStep(
|
||||
task,
|
||||
step,
|
||||
context,
|
||||
renderTemplate,
|
||||
taskTrack,
|
||||
workspacePath,
|
||||
decision,
|
||||
);
|
||||
// If a previous step failed, only run steps whose `if` condition
|
||||
// invokes a status check global (${{ always() }} or ${{ failure() }})
|
||||
if (taskState.failed) {
|
||||
if (typeof step.if !== 'string') {
|
||||
await task.emitLog(
|
||||
`Skipping step ${step.id} because a previous step failed`,
|
||||
{ stepId: step.id, status: 'skipped' },
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
// Render the if condition to detect status check function usage
|
||||
statusCheckInvoked.value = false;
|
||||
this.render(step.if, context, renderTemplate);
|
||||
|
||||
if (!statusCheckInvoked.value) {
|
||||
await task.emitLog(
|
||||
`Skipping step ${step.id} because a previous step failed`,
|
||||
{ stepId: step.id, status: 'skipped' },
|
||||
);
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
await this.executeStep(
|
||||
task,
|
||||
step,
|
||||
context,
|
||||
renderTemplate,
|
||||
taskTrack,
|
||||
workspacePath,
|
||||
decision,
|
||||
);
|
||||
} catch (err) {
|
||||
const error = err as Error;
|
||||
allErrors.push({ step, error });
|
||||
|
||||
if (!firstError) {
|
||||
firstError = error;
|
||||
} else {
|
||||
// Log subsequent errors to preserve debugging information
|
||||
this.options.logger.error(
|
||||
`Additional error in step ${step.id} (${step.name}): ${error.message}`,
|
||||
error,
|
||||
);
|
||||
await task.emitLog(
|
||||
`Additional error occurred: ${error.message}\n${error.stack}`,
|
||||
{ stepId: step.id, status: 'failed' },
|
||||
);
|
||||
}
|
||||
taskState.failed = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (firstError) {
|
||||
// If there were multiple errors, add context to the first error
|
||||
if (allErrors.length > 1) {
|
||||
const additionalErrorSummary = allErrors
|
||||
.slice(1)
|
||||
.map(({ step }) => `${step.id} (${step.name})`)
|
||||
.join(', ');
|
||||
this.options.logger.warn(
|
||||
`Task failed with ${allErrors.length} errors. First error from step ${allErrors[0].step.id}. Additional failures in: ${additionalErrorSummary}`,
|
||||
);
|
||||
}
|
||||
throw firstError;
|
||||
}
|
||||
|
||||
const output = this.render(task.spec.output, context, renderTemplate);
|
||||
|
||||
Reference in New Issue
Block a user