Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[steps] Add better workflow support for if condition #497

Merged
merged 3 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 43 additions & 15 deletions packages/eas-build-job/src/step.ts
Original file line number Diff line number Diff line change
@@ -1,30 +1,30 @@
import { z } from 'zod';

const StepOutputZ = z.object({
name: z.string(),
required: z.boolean().optional(),
});

const CommonStepZ = z.object({
/**
* Unique identifier for the step.
*
* @example
* id: step1
*/
id: z.string().optional(),
id: z
.string()
.optional()
.describe(
`ID of the step. Useful for later referencing the job's outputs. Example: step with id "setup" and an output "platform" will expose its value under "steps.setup.outputs.platform".`
),
/**
* Expression that determines whether the step should run.
* Based on the GitHub Actions job step `if` field (https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsif).
*/
if: z.string().optional(),
if: z.string().optional().describe('Expression that determines whether the step should run.'),
/**
* The name of the step.
*
* @example
* name: 'Step 1'
*/
name: z.string().optional(),
name: z.string().optional().describe('Name of the step.'),
/**
* The working directory to run the step in.
*
Expand All @@ -33,7 +33,12 @@ const CommonStepZ = z.object({
*
* @default depends on the project settings
*/
working_directory: z.string().optional(),
working_directory: z
.string()
.optional()
.describe(
`Working directory to run the step in. Relative paths like "./assets" or "assets" are resolved from the app's base directory. Absolute paths like "/apps/mobile" are resolved from the repository root.`
),
/**
* Env variables override for the step.
*
Expand All @@ -42,7 +47,10 @@ const CommonStepZ = z.object({
* MY_ENV_VAR: my-value
* ANOTHER_ENV_VAR: another-value
*/
env: z.record(z.string()).optional(),
env: z
.record(z.string())
.optional()
.describe('Additional environment variables to set for the step.'),
});

export const FunctionStepZ = CommonStepZ.extend({
Expand All @@ -56,7 +64,11 @@ export const FunctionStepZ = CommonStepZ.extend({
* @example
* uses: my-custom-function
*/
uses: z.string(),
uses: z
.string()
.describe(
'Name of the function to use for this step. See https://docs.expo.dev/custom-builds/schema/#built-in-eas-functions for available functions.'
),
/**
* The arguments to pass to the function.
*
Expand All @@ -70,7 +82,7 @@ export const FunctionStepZ = CommonStepZ.extend({
* - value1
* arg4: ${{ steps.step1.outputs.test }}
*/
with: z.record(z.unknown()).optional(),
with: z.record(z.unknown()).optional().describe('Inputs to the function.'),

run: z.never().optional(),
shell: z.never().optional(),
Expand All @@ -92,7 +104,7 @@ export const ShellStepZ = CommonStepZ.extend({
* npx expo prebuild
* pod install
*/
run: z.string(),
run: z.string().describe('Shell script to run in the step.'),
/**
* The shell to run the "run" command with.
*
Expand All @@ -101,7 +113,7 @@ export const ShellStepZ = CommonStepZ.extend({
*
* @default 'bash'
*/
shell: z.string().optional(),
shell: z.string().optional().describe('Shell to run the "run" command with.'),
/**
* The outputs of the step.
*
Expand All @@ -113,7 +125,23 @@ export const ShellStepZ = CommonStepZ.extend({
* required: false
* - name: my_optional_output_without_required
*/
outputs: z.array(StepOutputZ).optional(),
outputs: z
.array(
z.preprocess(
(input) => {
// We allow a shorthand for outputs
if (typeof input === 'string') {
return { name: input, required: false };
}
return input;
},
z.object({
name: z.string(),
required: z.boolean().optional(),
})
)
)
.optional(),

uses: z.never().optional(),
with: z.never().optional(),
Expand Down
10 changes: 4 additions & 6 deletions packages/steps/src/BuildStep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,17 +308,14 @@ export class BuildStep extends BuildStepOutputAccessor {

let ifCondition = this.ifCondition;

if (ifCondition.startsWith('${') && ifCondition.endsWith('}')) {
if (ifCondition.startsWith('${{') && ifCondition.endsWith('}}')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we allow for ${{ sth }} pattern in other places like inside of run? If we don't it's strange to support it only for if

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do!

ifCondition = ifCondition.slice(3, -2);
} else if (ifCondition.startsWith('${') && ifCondition.endsWith('}')) {
ifCondition = ifCondition.slice(2, -1);
}

return Boolean(
jsepEval(ifCondition, {
success: () => !hasAnyPreviousStepFailed,
failure: () => hasAnyPreviousStepFailed,
always: () => true,
never: () => false,
env: this.getScriptEnv(),
inputs:
this.inputs?.reduce(
(acc, input) => {
Expand All @@ -331,6 +328,7 @@ export class BuildStep extends BuildStepOutputAccessor {
runtimePlatform: this.ctx.global.runtimePlatform,
...this.ctx.global.staticContext,
},
...this.getInterpolationContext(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this includes deleted things, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea

})
);
}
Expand Down
46 changes: 32 additions & 14 deletions packages/steps/src/__tests__/BuildStep-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1067,6 +1067,20 @@ describe(BuildStep.prototype.shouldExecuteStep, () => {
expect(step.shouldExecuteStep()).toBe(true);
});

it('can use the general interpolation context', () => {
const ctx = createGlobalContextMock();
ctx.updateEnv({
CONFIG_JSON: '{"foo": "bar"}',
});
const step = new BuildStep(ctx, {
id: 'test1',
displayName: 'Test 1',
command: 'echo 123',
ifCondition: 'fromJSON(env.CONFIG_JSON).foo == "bar"',
});
expect(step.shouldExecuteStep()).toBe(true);
});

it('returns true when a simplified dynamic expression matches', () => {
const ctx = createGlobalContextMock();
const step = new BuildStep(ctx, {
Expand Down Expand Up @@ -1129,23 +1143,27 @@ describe(BuildStep.prototype.shouldExecuteStep, () => {
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() }',
});
expect(step.shouldExecuteStep()).toBe(true);
for (const ifCondition of ['${ failure() }', '${{ failure() }}']) {
const step = new BuildStep(ctx, {
id: 'test1',
displayName: 'Test 1',
command: 'echo 123',
ifCondition,
});
expect(step.shouldExecuteStep()).toBe(true);
}
});

it('returns false when if condition is failure and previous steps have not failed', () => {
const ctx = createGlobalContextMock();
const step = new BuildStep(ctx, {
id: 'test1',
displayName: 'Test 1',
command: 'echo 123',
ifCondition: '${ failure() }',
});
expect(step.shouldExecuteStep()).toBe(false);
for (const ifCondition of ['${ failure() }', '${{ failure() }}']) {
const step = new BuildStep(ctx, {
id: 'test1',
displayName: 'Test 1',
command: 'echo 123',
ifCondition,
});
expect(step.shouldExecuteStep()).toBe(false);
}
});
});
Loading