Skip to content

Commit

Permalink
feat(ng-dev/pullapprove): support in operator in condition evaluation
Browse files Browse the repository at this point in the history
A few conditions in framework currently rely on the `in` operator. We
should support this even though it potentially going away when we
re-work how global approvals are performed. This makes the verify command
a little more future-proof and currently unblocks the dev-infra update.

It's worth noting that our Python -> JS conversion is very limited but
we can always expand when needed, or start running actual Python. Right
now this captures the most common cases of conditions we have seen across
Angular.
  • Loading branch information
devversion committed Mar 28, 2022
1 parent c05de8e commit 9b5b3d4
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 2 deletions.
10 changes: 8 additions & 2 deletions ng-dev/pullapprove/condition_evaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,14 @@ export function convertConditionToFunction(
/**
* Transforms a condition expression from PullApprove that is based on python
* so that it can be run inside JavaScript. Current transformations:
* 1. `not <..>` -> `!<..>`
*
* 1. `aExpr not in bExpr` --> `!bExpr.includes(aExpr)`
* 2. `aExpr in bExpr` --> `bExpr.includes(aExpr`)
* 3. `not expr` --> `!expr`
*/
function transformExpressionToJs(expression: string): string {
return expression.replace(/not\s+/g, '!');
return expression
.replace(/^(.+)\s+not in\s+(.+)$/, '!$2.includes($1)')
.replace(/^(.+)\s+in\s+(.+)$/, '$2.includes($1)')
.replace(/not\s+/g, '!');
}
4 changes: 4 additions & 0 deletions ng-dev/pullapprove/pullapprove_arrays.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ export class PullApproveGroupArray extends Array<PullApproveGroup> {
return new PullApproveGroupArray(...this.filter((s) => s.groupName.match(pattern)));
}

get approved() {
throw new PullApproveGroupStateDependencyError();
}

get pending() {
throw new PullApproveGroupStateDependencyError();
}
Expand Down
36 changes: 36 additions & 0 deletions ng-dev/pullapprove/verify.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,42 @@ describe('group parsing', () => {
expect(renovateGroup.testFile('packages/core/index.ts')).toBe(false);
expect(renovateGroup.conditions[0].unverifiable).toBe(true);
});

describe('in operator', () => {
it('should work', () => {
const groups = getGroupsFromYaml(`
groups:
pass:
conditions:
- "'a' in ['a', 'b']"
invalid:
conditions:
- "'a' in ['b']"
`);
const passGroup = getGroupByName(groups, 'pass')!;
const invalidGroup = getGroupByName(groups, 'invalid')!;

expect(passGroup.testFile('any')).toBe(true);
expect(invalidGroup.testFile('any')).toBe(false);
});

it('should support exclusion check', () => {
const groups = getGroupsFromYaml(`
groups:
invalid:
conditions:
- "'a' not in ['a', 'b']"
pass:
conditions:
- "'a' not in ['b']"
`);
const invalidGroup = getGroupByName(groups, 'invalid')!;
const passGroup = getGroupByName(groups, 'pass')!;

expect(invalidGroup.testFile('any')).toBe(false);
expect(passGroup.testFile('any')).toBe(true);
});
});
});

function getGroupByName(groups: PullApproveGroup[], name: string): PullApproveGroup | undefined {
Expand Down

0 comments on commit 9b5b3d4

Please sign in to comment.