diff --git a/packages/steps/src/BuildStep.ts b/packages/steps/src/BuildStep.ts index 7b09b9ae75..de0aed5b49 100644 --- a/packages/steps/src/BuildStep.ts +++ b/packages/steps/src/BuildStep.ts @@ -83,6 +83,10 @@ export class BuildStepOutputAccessor { return Object.values(this.outputById); } + public get wasExecuted(): boolean { + return this.executed; + } + public getOutputValueByName(name: string): string | undefined { if (!this.executed) { throw new BuildStepRuntimeError( diff --git a/packages/steps/src/CompositeBuildStep.ts b/packages/steps/src/CompositeBuildStep.ts index 891b4e16f9..723875ecde 100644 --- a/packages/steps/src/CompositeBuildStep.ts +++ b/packages/steps/src/CompositeBuildStep.ts @@ -59,7 +59,7 @@ export class CompositeBuildStep extends BuildStep { displayName, outputs, fn, - // `scope` already applies the call-site `if`, so `always()` disables the default skip-on-failure. + // Call-site `if` lives on `scope`. `always()` survives a child failure; `shouldExecuteStep` still skips a call that never ran so outputs stay unset. ifCondition: '${{ always() }}', compositeFunctionScope: scope, }); @@ -87,6 +87,19 @@ export class CompositeBuildStep extends BuildStep { } } + public override shouldExecuteStep(options: { runByDefault: boolean }): boolean { + if (!super.shouldExecuteStep(options)) { + return false; + } + return options.runByDefault || this.anyAuthoredChildExecuted(); + } + + private anyAuthoredChildExecuted(): boolean { + return this.children.some(child => + child instanceof CompositeBuildStep ? child.anyAuthoredChildExecuted() : child.wasExecuted + ); + } + public getFlattenedSteps(): BuildStep[] { const flattened = this.children.flatMap(child => child instanceof CompositeBuildStep ? child.getFlattenedSteps() : [child] diff --git a/packages/steps/src/__tests__/BuildWorkflow-hooks-test.ts b/packages/steps/src/__tests__/BuildWorkflow-hooks-test.ts index 364c0b6b61..de2a2190a0 100644 --- a/packages/steps/src/__tests__/BuildWorkflow-hooks-test.ts +++ b/packages/steps/src/__tests__/BuildWorkflow-hooks-test.ts @@ -835,6 +835,80 @@ describe('BuildWorkflow hook execution', () => { expect(statuses['setup']).toBe(BuildStepStatus.SKIPPED); }); + it('skips the outputs node of an always() composite entry whose children an earlier entry suppressed', async () => { + const workflow = await parseAsync({ + steps: [{ uses: 'eas/install_node_modules' }], + hooks: { + before_install_node_modules: [ + { uses: 'test/first-boom', id: 'first-boom' }, + { uses: './.eas/functions/setup', id: 'setup', if: '${{ always() }}' }, + ], + }, + externalFunctions: [ + anchorFunction(), + recordingFunction('first-boom', { failWith: new Error('first failed') }), + recordingFunction('plain-child'), + versionFunction('read-version', '1.2.3'), + ], + compositeFunctionCatalog: { + './.eas/functions/setup': { + outputs: { version: { value: '${{ steps.read.outputs.version }}' } }, + runs: { + steps: [ + { id: 'plain', uses: 'test/plain-child' }, + { id: 'read', uses: 'test/read-version' }, + ], + }, + }, + }, + }); + await expect(workflow.executeAsync()).rejects.toThrow('first failed'); + expect(executionLog).toEqual(['first-boom']); + const statuses = hookStepStatuses(workflow); + expect(statuses['setup__plain']).toBe(BuildStepStatus.SKIPPED); + expect(statuses['setup__read']).toBe(BuildStepStatus.SKIPPED); + expect(statuses['setup']).toBe(BuildStepStatus.SKIPPED); + expect(metrics).toEqual([ + { anchor: 'install_node_modules', timing: 'before', result: 'failed' }, + ]); + }); + + it('skips the outputs node of an after composite entry whose first child has an unevaluable if', async () => { + const workflow = await parseAsync({ + steps: [{ uses: 'eas/install_node_modules' }], + hooks: { + after_install_node_modules: [{ uses: './.eas/functions/cleanup', id: 'cleanup' }], + }, + externalFunctions: [ + anchorFunction(), + recordingFunction('broken-gate'), + recordingFunction('plain-child'), + ], + compositeFunctionCatalog: { + './.eas/functions/cleanup': { + outputs: { note: { value: 'done' } }, + runs: { + steps: [ + { + id: 'broken', + uses: 'test/broken-gate', + if: '${{ nonexistent.object.property }}', + }, + { id: 'plain', uses: 'test/plain-child' }, + ], + }, + }, + }, + }); + await expect(workflow.executeAsync()).rejects.toThrow(); + expect(executionLog).toEqual(['anchor']); + const statuses = hookStepStatuses(workflow); + expect(statuses['cleanup__broken']).toBe(BuildStepStatus.SKIPPED); + expect(statuses['cleanup__plain']).toBe(BuildStepStatus.SKIPPED); + expect(statuses['cleanup']).toBe(BuildStepStatus.SKIPPED); + expect(metrics).toEqual([]); + }); + it('a later hook step consumes the composite call outputs via steps.', async () => { const captured: unknown[] = []; const workflow = await parseAsync({ diff --git a/packages/steps/src/__tests__/StepsConfigParser-composite-functions-outputs-test.ts b/packages/steps/src/__tests__/StepsConfigParser-composite-functions-outputs-test.ts index ea7e4cbb4d..d152e5dc3a 100644 --- a/packages/steps/src/__tests__/StepsConfigParser-composite-functions-outputs-test.ts +++ b/packages/steps/src/__tests__/StepsConfigParser-composite-functions-outputs-test.ts @@ -319,6 +319,172 @@ describe('StepsConfigParser local composite functions', () => { expect(outputsStep.getOutputValueByName('version_expr')).toBe(''); }); + it('skips the outputs step when an earlier failure suppressed every inner step', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { + [SETUP]: { + outputs: { version: { value: '${{ steps.read.outputs.version }}' } }, + runs: { + steps: [{ id: 'read', uses: 'test/set-version' }], + }, + }, + }, + steps: [ + { id: 'boom', uses: 'test/fail' }, + { uses: SETUP, id: 'setup', if: '${{ always() }}' }, + { + id: 'after', + uses: 'test/passthrough', + if: '${{ always() }}', + with: { value: '${{ steps.setup.outputs.version }}' }, + }, + ], + externalFunctions: [setVersionFunction(), failingFunction(), passThroughFunction()], + }); + + const error = await getErrorAsync(() => workflow.executeAsync()); + expect(error.message).toBe('inner failed'); + + const innerStep = workflow.buildSteps.find(s => s.id === 'setup__read'); + const outputsStep = workflow.buildSteps.find(s => s.id === 'setup'); + const afterStep = workflow.buildSteps.find(s => s.id === 'after'); + expect(innerStep?.status).toBe(BuildStepStatus.SKIPPED); + expect(outputsStep?.status).toBe(BuildStepStatus.SKIPPED); + expect(afterStep?.getOutputValueByName('out')).toBe(''); + expect(() => outputsStep?.getOutputValueByName('version')).toThrow( + 'has not been executed yet' + ); + }); + + it('runs the outputs step when an inner step opted past the earlier failure with always()', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { + [SETUP]: { + outputs: { + version: { value: '${{ steps.read.outputs.version }}' }, + skipped: { value: '${{ steps.later.outputs.out }}' }, + }, + runs: { + steps: [ + { id: 'read', uses: 'test/set-version', if: '${{ always() }}' }, + { id: 'later', uses: 'test/passthrough', with: { value: 'x' } }, + ], + }, + }, + }, + steps: [ + { id: 'boom', uses: 'test/fail' }, + { uses: SETUP, id: 'setup', if: '${{ always() }}' }, + ], + externalFunctions: [setVersionFunction(), failingFunction(), passThroughFunction()], + }); + + const error = await getErrorAsync(() => workflow.executeAsync()); + expect(error.message).toBe('inner failed'); + + const outputsStep = workflow.buildSteps.find(s => s.id === 'setup'); + expect(workflow.buildSteps.find(s => s.id === 'setup__read')?.status).toBe( + BuildStepStatus.SUCCESS + ); + expect(workflow.buildSteps.find(s => s.id === 'setup__later')?.status).toBe( + BuildStepStatus.SKIPPED + ); + expect(outputsStep?.status).toBe(BuildStepStatus.SUCCESS); + expect(outputsStep?.getOutputValueByName('version')).toBe('$(echo injected)'); + expect(outputsStep?.getOutputValueByName('skipped')).toBe(''); + }); + + it('skips the outer outputs step when a nested call without outputs was fully suppressed', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { + './.eas/functions/outer': { + outputs: { version: { value: '${{ steps.mid.outputs.version }}' } }, + runs: { steps: [{ uses: './.eas/functions/inner', id: 'mid' }] }, + }, + './.eas/functions/inner': { + runs: { steps: [{ id: 'read', uses: 'test/set-version' }] }, + }, + }, + steps: [ + { id: 'boom', uses: 'test/fail' }, + { uses: './.eas/functions/outer', id: 'top', if: '${{ always() }}' }, + ], + externalFunctions: [setVersionFunction(), failingFunction()], + }); + + const error = await getErrorAsync(() => workflow.executeAsync()); + expect(error.message).toBe('inner failed'); + + expect(workflow.buildSteps.find(s => s.id === 'top__mid__read')?.status).toBe( + BuildStepStatus.SKIPPED + ); + expect(workflow.buildSteps.find(s => s.id === 'top')?.status).toBe(BuildStepStatus.SKIPPED); + }); + + it('runs the outer outputs step when a grandchild of a nested call executed', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { + './.eas/functions/outer': { + outputs: { version: { value: '${{ steps.mid.outputs.inner_version }}' } }, + runs: { + steps: [{ uses: './.eas/functions/inner', id: 'mid', if: '${{ always() }}' }], + }, + }, + './.eas/functions/inner': { + outputs: { inner_version: { value: '${{ steps.read.outputs.version }}' } }, + runs: { steps: [{ id: 'read', uses: 'test/set-version', if: '${{ always() }}' }] }, + }, + }, + steps: [ + { id: 'boom', uses: 'test/fail' }, + { uses: './.eas/functions/outer', id: 'top', if: '${{ always() }}' }, + ], + externalFunctions: [setVersionFunction(), failingFunction()], + }); + + const error = await getErrorAsync(() => workflow.executeAsync()); + expect(error.message).toBe('inner failed'); + + expect(workflow.buildSteps.find(s => s.id === 'top__mid__read')?.status).toBe( + BuildStepStatus.SUCCESS + ); + expect(workflow.buildSteps.find(s => s.id === 'top')?.getOutputValueByName('version')).toBe( + '$(echo injected)' + ); + }); + + it('runs the outer outputs step when a grandchild of a nested call without outputs executed via always()', async () => { + const workflow = await parseCompositeFunctions({ + catalog: { + './.eas/functions/outer': { + outputs: { version: { value: '${{ steps.mid.outputs.version }}' } }, + runs: { + steps: [{ uses: './.eas/functions/inner', id: 'mid', if: '${{ always() }}' }], + }, + }, + './.eas/functions/inner': { + runs: { steps: [{ id: 'read', uses: 'test/set-version', if: '${{ always() }}' }] }, + }, + }, + steps: [ + { id: 'boom', uses: 'test/fail' }, + { uses: './.eas/functions/outer', id: 'top', if: '${{ always() }}' }, + ], + externalFunctions: [setVersionFunction(), failingFunction()], + }); + + const error = await getErrorAsync(() => workflow.executeAsync()); + expect(error.message).toBe('inner failed'); + + expect(workflow.buildSteps.find(s => s.id === 'top__mid__read')?.status).toBe( + BuildStepStatus.SUCCESS + ); + expect(workflow.buildSteps.find(s => s.id === 'top__mid')).toBeUndefined(); + const outputsStep = workflow.buildSteps.find(s => s.id === 'top'); + expect(outputsStep?.status).toBe(BuildStepStatus.SUCCESS); + expect(outputsStep?.getOutputValueByName('version')).toBe(''); + }); + it('resolves an out-of-scope action output reference to an empty string', async () => { const workflow = await parseCompositeFunctions({ catalog: {