Skip to content

Commit

Permalink
feat: add overridable to ignore acceptAllConditions
Browse files Browse the repository at this point in the history
  • Loading branch information
henrypoon committed Oct 7, 2021
1 parent d295136 commit 1ce754d
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 10 deletions.
11 changes: 11 additions & 0 deletions src/__tests__/__snapshots__/conditional.spec.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,17 @@ exports[`buildkite-graph Steps Command isEffectOf will not add steps if any effe
exports[`buildkite-graph Steps Command isEffectOf will not add steps if effect dependency is rejected structure 1`] = `""`;
exports[`buildkite-graph Steps Command isEffectOf will not override when acceptAllConditions is set but isOverridable returns false in a condition json_depends_on_accept_all 1`] = `
Object {
"steps": Array [],
}
`;
exports[`buildkite-graph Steps Command isEffectOf will not override when acceptAllConditions is set but isOverridable returns false in a condition yaml_depends_on_accept_all 1`] = `
"steps: []
"
`;
exports[`buildkite-graph Steps Command step addition dot 1`] = `
"digraph \\"whatever\\" {
graph [ compound =true ];
Expand Down
20 changes: 19 additions & 1 deletion src/__tests__/conditional.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ class MyConditional<T extends Step> extends Conditional<T> {
constructor(
step: ThingOrGenerator<T>,
private readonly accepted: ReturnType<Conditional<T>['accept']>,
overridable = true,
) {
super(step as any);
super(step as any, overridable);
}

accept(): ReturnType<Conditional<T>['accept']> {
Expand Down Expand Up @@ -177,6 +178,23 @@ describe('buildkite-graph', () => {
['json_depends_on_accept_all', 'yaml_depends_on_accept_all'],
);

createTest(
'will not override when acceptAllConditions is set but isOverridable returns false in a condition',
() => {
const acceptedTests = new MyConditional(
new CommandStep('run tests'),
false,
false,
);
const deployCoverage = new CommandStep(
'deploy coverage',
).isEffectOf(acceptedTests);

return new Pipeline('x').add(acceptedTests, deployCoverage);
},
['json_depends_on_accept_all', 'yaml_depends_on_accept_all'],
);

it('should not execute conditional.accept when acceptAllConditions is set', () => {
const acceptedTests = new MyConditional(
new CommandStep('run tests'),
Expand Down
14 changes: 13 additions & 1 deletion src/conditional.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ export type Generator<T> = () => T | Promise<T>;
export type ThingOrGenerator<T> = T | Promise<T> | Generator<T>;

export abstract class Conditional<T extends Step> {
constructor(private readonly guarded: ThingOrGenerator<T>) {}
constructor(
private readonly guarded: ThingOrGenerator<T>,
private readonly overridable = true,
) {}

get(): T | Promise<T> {
return typeof this.guarded === 'function' ? this.guarded() : this.guarded;
Expand All @@ -15,4 +18,13 @@ export abstract class Conditional<T extends Step> {
* The step is rejected if the returned boolean is false or the promise resolves to false.
*/
abstract accept(): boolean | Promise<boolean>;

/**
* Whether this condition could be overriden to accept when `acceptAllConditions` is set. `overridable` is
* set to true by default.
* @returns boolean
*/
isOverridable(): boolean {
return this.overridable;
}
}
17 changes: 10 additions & 7 deletions src/sortedSteps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,17 @@ export async function sortedSteps(
potentialEffectDependency,
);

// Accept all conditions/steps regardless
if (acceptAllConditions) {
addDependency(dependency);
s.dependsOn(potentialEffectDependency);
continue;
}

if (potentialEffectDependency instanceof Conditional) {
// Accept all conditions/steps regardless
if (
acceptAllConditions &&
potentialEffectDependency.isOverridable()
) {
addDependency(dependency);
s.dependsOn(potentialEffectDependency);
continue;
}

// in case it is a conditional we are interested in whether it that one was accepted or not
if (
(await potentialEffectDependency.accept()) ||
Expand Down
5 changes: 4 additions & 1 deletion src/unwrapSteps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ export async function unwrapSteps(
if (cache.has(s)) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
ret.push(cache.get(s)!);
} else if (acceptAllConditions || (await s.accept()) === true) {
} else if (
(acceptAllConditions && s.isOverridable()) ||
(await s.accept()) === true
) {
const cond = await s.get();
cache.set(s, cond);
ret.push(cond);
Expand Down

0 comments on commit 1ce754d

Please sign in to comment.