Skip to content

Commit

Permalink
Added regex validation (#20)
Browse files Browse the repository at this point in the history
Created method which validates the regex expressions in a configuration.

Closes #16

I want to have a constant error type + a custom error log so I make it
return either a passed status or a failed status + error message so it
can be logged.
  • Loading branch information
Bullrich committed Jul 17, 2023
1 parent ff0abc9 commit 1f41374
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 7 deletions.
44 changes: 44 additions & 0 deletions src/file/validator.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Joi from "joi";

import { ActionLogger } from "../github/types";
import { ConfigurationFile, Rule } from "./types";

const ruleSchema = Joi.object<Rule>().keys({
Expand All @@ -20,3 +21,46 @@ export const schema = Joi.object<ConfigurationFile>().keys({
.optional()
.allow(null),
});

/** Evaluate if the regex expression inside a configuration are valid.
* @returns a tuple of type [boolean, string]. If the boolean is false, the string will contain an error message
* @example
* const [result, error] = validateRegularExpressions(myConfig);
* if (!result) {
* throw new Error(error);
* } else {
* runExpression(myConfig);
* }
*/
export const validateRegularExpressions = (
config: ConfigurationFile,
logger: ActionLogger,
): [true] | [false, string] => {
/** Regex evaluator */
const isRegexValid = (regex: string): boolean => {
try {
new RegExp(regex);
return true;
} catch (e) {
logger.error(e as Error);
return false;
}
};

for (const rule of config.rules) {
for (const condition of rule.condition.include) {
if (!isRegexValid(condition)) {
return [false, `Include condition '${condition}' is not a valid regex`];
}
}
if (rule.condition.exclude) {
for (const condition of rule.condition.exclude) {
if (!isRegexValid(condition)) {
return [false, `Exclude condition '${condition}' is not a valid regex`];
}
}
}
}

return [true];
};
11 changes: 9 additions & 2 deletions src/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { parse } from "yaml";

import { Inputs } from ".";
import { ConfigurationFile } from "./file/types";
import { schema } from "./file/validator";
import { schema, validateRegularExpressions } from "./file/validator";
import { PullRequestApi } from "./github/pullRequest";
import { ActionLogger } from "./github/types";

Expand All @@ -22,7 +22,14 @@ export class ActionRunner {

this.logger.info(`Obtained config at ${configLocation}`);

return validate<ConfigurationFile>(config, schema, { message: "Configuration file is invalid" });
const configFile = validate<ConfigurationFile>(config, schema, { message: "Configuration file is invalid" });

const [result, error] = validateRegularExpressions(configFile, this.logger);
if (!result) {
throw new Error(`Regular expression is invalid: ${error}`);
}

return configFile;
}

async runAction(inputs: Omit<Inputs, "repoToken">): Promise<boolean> {
Expand Down
26 changes: 21 additions & 5 deletions src/test/runner/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,13 @@
import { mock, MockProxy } from "jest-mock-extended";

import { PullRequestApi } from "../../github/pullRequest";
import { ActionLogger } from "../../github/types";
import { ActionRunner } from "../../runner";
import { TestLogger } from "../logger";

describe("Config Parsing", () => {
let api: MockProxy<PullRequestApi>;
let runner: ActionRunner;
let logger: ActionLogger;
let logger: TestLogger;
beforeEach(() => {
logger = new TestLogger();
api = mock<PullRequestApi>();
Expand All @@ -28,14 +27,31 @@ describe("Config Parsing", () => {
- 'example'
`);
const config = await runner.getConfigFile("");
expect(config.preventReviewRequests).toBeNull;
expect(config.preventReviewRequests).toBeUndefined();
});

test("should call GitHub api with path", async () => {
await expect(runner.getConfigFile("example-location")).rejects.toThrowError();
expect(api.getConfigFile).toHaveBeenCalledWith("example-location");
});

describe("regular expressions validator", () => {
test("should fail with invalid regular expression", async () => {
const invalidRegex = "(?(";
api.getConfigFile.mockResolvedValue(`
rules:
- name: Default review
condition:
include:
- '${invalidRegex}'
`);
await expect(runner.getConfigFile("")).rejects.toThrowError(
`Regular expression is invalid: Include condition '${invalidRegex}' is not a valid regex`,
);
expect(logger.logHistory).toContainEqual(`Invalid regular expression: /${invalidRegex}/: Invalid group`);
});
});

describe("preventReviewRequests field", () => {
test("should get team", async () => {
api.getConfigFile.mockResolvedValue(`
Expand Down Expand Up @@ -86,7 +102,7 @@ describe("Config Parsing", () => {
- 'example'
`);
const config = await runner.getConfigFile("");
expect(config.preventReviewRequests).toBeNull;
expect(config.preventReviewRequests).toBeUndefined();
});
});

Expand Down Expand Up @@ -137,7 +153,7 @@ describe("Config Parsing", () => {
- '.*'
`);
const config = await runner.getConfigFile("");
expect(config.rules[0].condition.exclude).toBeNull;
expect(config.rules[0].condition.exclude).toBeUndefined();
});
});
});

0 comments on commit 1f41374

Please sign in to comment.