Skip to content

Commit

Permalink
Add hasAnyPreviousStepFailed to BuildStepContext
Browse files Browse the repository at this point in the history
  • Loading branch information
@sjchmiela
sjchmiela committed Oct 15, 2024
1 parent 8a18c32 commit 2235734
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 47 deletions.
10 changes: 6 additions & 4 deletions packages/steps/src/BuildStep.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,11 @@ export class BuildStep extends BuildStepOutputAccessor {
);
}

public shouldExecuteStep(hasAnyPreviousStepsFailed: boolean): boolean {
public shouldExecuteStep(): boolean {
const hasAnyPreviousStepFailed = this.ctx.global.hasAnyPreviousStepFailed;

if (!this.ifCondition) {
return !hasAnyPreviousStepsFailed;
return !hasAnyPreviousStepFailed;
}

let ifCondition = this.ifCondition;
Expand All @@ -305,8 +307,8 @@ export class BuildStep extends BuildStepOutputAccessor {

return Boolean(
jsepEval(ifCondition, {
success: () => !hasAnyPreviousStepsFailed,
failure: () => hasAnyPreviousStepsFailed,
success: () => !hasAnyPreviousStepFailed,
failure: () => hasAnyPreviousStepFailed,
always: () => true,
never: () => false,
env: this.getScriptEnv(),
Expand Down
15 changes: 14 additions & 1 deletion packages/steps/src/BuildStepContext.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export class BuildStepGlobalContext {
public readonly runtimePlatform: BuildRuntimePlatform;
public readonly baseLogger: bunyan;
private didCheckOut = false;

private _hasAnyPreviousStepFailed = false;
private stepById: Record<string, BuildStepOutputAccessor> = {};

constructor(
Expand All @@ -65,6 +65,7 @@ export class BuildStepGlobalContext {
this.stepsInternalBuildDirectory = path.join(os.tmpdir(), 'eas-build', uuidv4());
this.runtimePlatform = provider.runtimePlatform;
this.baseLogger = provider.logger;
this._hasAnyPreviousStepFailed = false;
}

public get projectSourceDirectory(): string {
Expand Down Expand Up @@ -99,6 +100,10 @@ export class BuildStepGlobalContext {
this.stepById[step.id] = step;
}

public get steps(): BuildStepOutputAccessor[] {
return Object.values(this.stepById);
}

public getStepOutputValue(path: string): string | undefined {
const { stepId, outputId } = parseOutputPath(path);
if (!(stepId in this.stepById)) {
Expand Down Expand Up @@ -133,6 +138,14 @@ export class BuildStepGlobalContext {
);
}

public get hasAnyPreviousStepFailed(): boolean {
return this._hasAnyPreviousStepFailed;
}

public markAsFailed(): void {
this._hasAnyPreviousStepFailed = true;
}

public wasCheckedOut(): boolean {
return this.didCheckOut;
}
Expand Down
8 changes: 3 additions & 5 deletions packages/steps/src/BuildWorkflow.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ export class BuildWorkflow {
public readonly buildFunctions: BuildFunctionById;

constructor(
// @ts-expect-error ctx is not used in this class but let's keep it here for consistency
private readonly ctx: BuildStepGlobalContext,
{ buildSteps, buildFunctions }: { buildSteps: BuildStep[]; buildFunctions: BuildFunctionById }
) {
Expand All @@ -17,25 +16,24 @@ export class BuildWorkflow {

public async executeAsync(): Promise<void> {
let maybeError: Error | null = null;
let hasAnyPreviousStepFailed = false;
for (const step of this.buildSteps) {
let shouldExecuteStep = false;
try {
shouldExecuteStep = step.shouldExecuteStep(hasAnyPreviousStepFailed);
shouldExecuteStep = step.shouldExecuteStep();
} catch (err: any) {
step.ctx.logger.error({ err });
step.ctx.logger.error(
`Runner failed to evaluate if it should execute step "${step.displayName}", using step's if condition "${step.ifCondition}". This can be caused by trying to access non-existing object property. If you think this is a bug report it here: https://github.com/expo/eas-cli/issues.`
);
maybeError = maybeError ?? err;
hasAnyPreviousStepFailed = true;
this.ctx.markAsFailed();
}
if (shouldExecuteStep) {
try {
await step.executeAsync();
} catch (err: any) {
maybeError = maybeError ?? err;
hasAnyPreviousStepFailed = true;
this.ctx.markAsFailed();
}
} else {
step.skip();
Expand Down
29 changes: 13 additions & 16 deletions packages/steps/src/__tests__/BuildStep-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1017,14 +1017,14 @@ describe(BuildStep.deserialize, () => {
describe(BuildStep.prototype.shouldExecuteStep, () => {
it('returns true when if condition is always and previous steps failed', () => {
const ctx = createGlobalContextMock();
ctx.markAsFailed();
const step = new BuildStep(ctx, {
id: 'test1',
displayName: 'Test 1',
command: 'echo 123',
ifCondition: '${ always() }',
});
const hasAnyPreviousStepsFailed = true;
expect(step.shouldExecuteStep(hasAnyPreviousStepsFailed)).toBe(true);
expect(step.shouldExecuteStep()).toBe(true);
});

it('returns true when if condition is always and previous steps have not failed', () => {
Expand All @@ -1035,20 +1035,19 @@ describe(BuildStep.prototype.shouldExecuteStep, () => {
command: 'echo 123',
ifCondition: '${ always() }',
});
const hasAnyPreviousStepsFailed = false;
expect(step.shouldExecuteStep(hasAnyPreviousStepsFailed)).toBe(true);
expect(step.shouldExecuteStep()).toBe(true);
});

it('returns false when if condition is success and previous steps failed', () => {
const ctx = createGlobalContextMock();
ctx.markAsFailed();
const step = new BuildStep(ctx, {
id: 'test1',
displayName: 'Test 1',
command: 'echo 123',
ifCondition: '${ success() }',
});
const hasAnyPreviousStepsFailed = true;
expect(step.shouldExecuteStep(hasAnyPreviousStepsFailed)).toBe(false);
expect(step.shouldExecuteStep()).toBe(false);
});

it('returns true when a dynamic expression matches', () => {
Expand All @@ -1065,7 +1064,7 @@ describe(BuildStep.prototype.shouldExecuteStep, () => {
},
ifCondition: '${ env.NODE_ENV === "production" && env.LOCAL_ENV === "true" }',
});
expect(step.shouldExecuteStep(false)).toBe(true);
expect(step.shouldExecuteStep()).toBe(true);
});

it('returns true when a simplified dynamic expression matches', () => {
Expand All @@ -1079,7 +1078,7 @@ describe(BuildStep.prototype.shouldExecuteStep, () => {
},
ifCondition: "env.NODE_ENV === 'production'",
});
expect(step.shouldExecuteStep(false)).toBe(true);
expect(step.shouldExecuteStep()).toBe(true);
});

it('returns true when an input matches', () => {
Expand All @@ -1102,7 +1101,7 @@ describe(BuildStep.prototype.shouldExecuteStep, () => {
],
ifCondition: 'inputs.foo1 === "bar"',
});
expect(step.shouldExecuteStep(false)).toBe(true);
expect(step.shouldExecuteStep()).toBe(true);
});

it('returns true when an eas value matches', () => {
Expand All @@ -1113,7 +1112,7 @@ describe(BuildStep.prototype.shouldExecuteStep, () => {
command: 'echo 123',
ifCondition: 'eas.runtimePlatform === "linux"',
});
expect(step.shouldExecuteStep(false)).toBe(true);
expect(step.shouldExecuteStep()).toBe(true);
});

it('returns true when if condition is success and previous steps have not failed', () => {
Expand All @@ -1124,20 +1123,19 @@ describe(BuildStep.prototype.shouldExecuteStep, () => {
command: 'echo 123',
ifCondition: '${ success() }',
});
const hasAnyPreviousStepsFailed = false;
expect(step.shouldExecuteStep(hasAnyPreviousStepsFailed)).toBe(true);
expect(step.shouldExecuteStep()).toBe(true);
});

it('returns true when if condition is failure and previous steps failed', () => {
const ctx = createGlobalContextMock();
ctx.markAsFailed();
const step = new BuildStep(ctx, {
id: 'test1',
displayName: 'Test 1',
command: 'echo 123',
ifCondition: '${ failure() }',
});
const hasAnyPreviousStepsFailed = true;
expect(step.shouldExecuteStep(hasAnyPreviousStepsFailed)).toBe(true);
expect(step.shouldExecuteStep()).toBe(true);
});

it('returns false when if condition is failure and previous steps have not failed', () => {
Expand All @@ -1148,7 +1146,6 @@ describe(BuildStep.prototype.shouldExecuteStep, () => {
command: 'echo 123',
ifCondition: '${ failure() }',
});
const hasAnyPreviousStepsFailed = false;
expect(step.shouldExecuteStep(hasAnyPreviousStepsFailed)).toBe(false);
expect(step.shouldExecuteStep()).toBe(false);
});
});
42 changes: 21 additions & 21 deletions packages/steps/src/__tests__/BuildWorkflow-test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { instance, mock, verify, when, anything } from 'ts-mockito';
import { instance, mock, verify, when } from 'ts-mockito';

import { BuildStep } from '../BuildStep.js';
import { BuildWorkflow } from '../BuildWorkflow.js';
Expand All @@ -12,10 +12,10 @@ describe(BuildWorkflow, () => {
const mockBuildStep2 = mock<BuildStep>();
const mockBuildStep3 = mock<BuildStep>();
const mockBuildStep4 = mock<BuildStep>();
when(mockBuildStep4.shouldExecuteStep(anything())).thenReturn(true);
when(mockBuildStep3.shouldExecuteStep(anything())).thenReturn(true);
when(mockBuildStep2.shouldExecuteStep(anything())).thenReturn(true);
when(mockBuildStep1.shouldExecuteStep(anything())).thenReturn(true);
when(mockBuildStep4.shouldExecuteStep()).thenReturn(true);
when(mockBuildStep3.shouldExecuteStep()).thenReturn(true);
when(mockBuildStep2.shouldExecuteStep()).thenReturn(true);
when(mockBuildStep1.shouldExecuteStep()).thenReturn(true);

const buildSteps: BuildStep[] = [
instance(mockBuildStep1),
Expand All @@ -37,9 +37,9 @@ describe(BuildWorkflow, () => {
const mockBuildStep1 = mock<BuildStep>();
const mockBuildStep2 = mock<BuildStep>();
const mockBuildStep3 = mock<BuildStep>();
when(mockBuildStep3.shouldExecuteStep(anything())).thenReturn(true);
when(mockBuildStep2.shouldExecuteStep(anything())).thenReturn(true);
when(mockBuildStep1.shouldExecuteStep(anything())).thenReturn(true);
when(mockBuildStep3.shouldExecuteStep()).thenReturn(true);
when(mockBuildStep2.shouldExecuteStep()).thenReturn(true);
when(mockBuildStep1.shouldExecuteStep()).thenReturn(true);

const buildSteps: BuildStep[] = [
instance(mockBuildStep1),
Expand All @@ -61,10 +61,10 @@ describe(BuildWorkflow, () => {
const mockBuildStep2 = mock<BuildStep>();
const mockBuildStep3 = mock<BuildStep>();
const mockBuildStep4 = mock<BuildStep>();
when(mockBuildStep4.shouldExecuteStep(anything())).thenReturn(true);
when(mockBuildStep3.shouldExecuteStep(anything())).thenReturn(false);
when(mockBuildStep2.shouldExecuteStep(anything())).thenReturn(false);
when(mockBuildStep1.shouldExecuteStep(anything())).thenReturn(true);
when(mockBuildStep4.shouldExecuteStep()).thenReturn(true);
when(mockBuildStep3.shouldExecuteStep()).thenReturn(false);
when(mockBuildStep2.shouldExecuteStep()).thenReturn(false);
when(mockBuildStep1.shouldExecuteStep()).thenReturn(true);

const buildSteps: BuildStep[] = [
instance(mockBuildStep1),
Expand All @@ -87,9 +87,9 @@ describe(BuildWorkflow, () => {
const mockBuildStep1 = mock<BuildStep>();
const mockBuildStep2 = mock<BuildStep>();
const mockBuildStep3 = mock<BuildStep>();
when(mockBuildStep3.shouldExecuteStep(anything())).thenReturn(false);
when(mockBuildStep2.shouldExecuteStep(anything())).thenReturn(false);
when(mockBuildStep1.shouldExecuteStep(anything())).thenReturn(true);
when(mockBuildStep3.shouldExecuteStep()).thenReturn(false);
when(mockBuildStep2.shouldExecuteStep()).thenReturn(false);
when(mockBuildStep1.shouldExecuteStep()).thenReturn(true);
when(mockBuildStep1.executeAsync()).thenReject(new Error('Step 1 failed'));

const buildSteps: BuildStep[] = [
Expand All @@ -111,9 +111,9 @@ describe(BuildWorkflow, () => {
const mockBuildStep1 = mock<BuildStep>();
const mockBuildStep2 = mock<BuildStep>();
const mockBuildStep3 = mock<BuildStep>();
when(mockBuildStep3.shouldExecuteStep(anything())).thenReturn(true);
when(mockBuildStep2.shouldExecuteStep(anything())).thenReturn(true);
when(mockBuildStep1.shouldExecuteStep(anything())).thenReturn(true);
when(mockBuildStep3.shouldExecuteStep()).thenReturn(true);
when(mockBuildStep2.shouldExecuteStep()).thenReturn(true);
when(mockBuildStep1.shouldExecuteStep()).thenReturn(true);
when(mockBuildStep1.executeAsync()).thenReject(new Error('Step 1 failed'));

const buildSteps: BuildStep[] = [
Expand All @@ -135,9 +135,9 @@ describe(BuildWorkflow, () => {
const mockBuildStep1 = mock<BuildStep>();
const mockBuildStep2 = mock<BuildStep>();
const mockBuildStep3 = mock<BuildStep>();
when(mockBuildStep3.shouldExecuteStep(anything())).thenReturn(true);
when(mockBuildStep2.shouldExecuteStep(anything())).thenReturn(true);
when(mockBuildStep1.shouldExecuteStep(anything())).thenReturn(true);
when(mockBuildStep3.shouldExecuteStep()).thenReturn(true);
when(mockBuildStep2.shouldExecuteStep()).thenReturn(true);
when(mockBuildStep1.shouldExecuteStep()).thenReturn(true);
when(mockBuildStep1.executeAsync()).thenReject(new Error('Step 1 failed'));
when(mockBuildStep2.executeAsync()).thenReject(new Error('Step 2 failed'));
when(mockBuildStep3.executeAsync()).thenReject(new Error('Step 3 failed'));
Expand Down

0 comments on commit 2235734

Please sign in to comment.