Skip to content

Commit

Permalink
added output to action execution (#50)
Browse files Browse the repository at this point in the history 
Added an output to the action.

It will return a stringified JSON.

Resolves #17 and #43
  • Loading branch information
@Bullrich
Bullrich authored Aug 16, 2023
1 parent 8f9e8a7 commit 13b897c
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 36 deletions.
1 change: 1 addition & 0 deletions src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ runner
.runAction(inputs)
.then((result) => {
info(`Action run without problem. Evaluation result was '${result.conclusion}'`);
setOutput("report", JSON.stringify(result));
})
.catch((error) => {
console.error(error);
Expand Down
28 changes: 19 additions & 9 deletions src/runner.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { summary } from "@actions/core";
import { setOutput, summary } from "@actions/core";
import { parse } from "yaml";

import { Inputs } from ".";
Expand All @@ -24,6 +24,13 @@ type RuleReport = { name: string } & ReviewReport;

type ReviewState = [true] | [false, ReviewReport];

type PullRequestReport = {
/** List of files that were modified by the PR */
files: string[];
/** List of all the failed review requirements */
reports: RuleReport[];
};

/** Action in charge of running the GitHub action */
export class ActionRunner {
constructor(
Expand Down Expand Up @@ -57,13 +64,14 @@ export class ActionRunner {
* The action evaluates if the rules requirements are meet for a PR
* @returns an array of error reports for each failed rule. An empty array means no errors
*/
async validatePullRequest({ rules }: ConfigurationFile): Promise<RuleReport[]> {
async validatePullRequest({ rules }: ConfigurationFile): Promise<PullRequestReport> {
const errorReports: RuleReport[] = [];
const modifiedFiles = await this.prApi.listModifiedFiles();
ruleCheck: for (const rule of rules) {
try {
this.logger.info(`Validating rule '${rule.name}'`);
// We get all the files that were modified and match the rules condition
const files = await this.listFilesThatMatchRuleCondition(rule);
const files = this.listFilesThatMatchRuleCondition(modifiedFiles, rule);
// We check if there are any matches
if (files.length === 0) {
this.logger.info(`Skipping rule ${rule.name} as no condition matched`);
Expand Down Expand Up @@ -126,7 +134,7 @@ export class ActionRunner {
}
this.logger.info(`Finish validating '${rule.name}'`);
}
return errorReports;
return { files: modifiedFiles, reports: errorReports };
}

/** WIP - Class that will assign the requests for review */
Expand Down Expand Up @@ -269,8 +277,7 @@ export class ActionRunner {
}

/** Using the include and exclude condition, it returns a list of all the files in a PR that matches the criteria */
async listFilesThatMatchRuleCondition({ condition }: Rule): Promise<string[]> {
const files = await this.prApi.listModifiedFiles();
listFilesThatMatchRuleCondition(files: string[], { condition }: Rule): string[] {
let matches: string[] = [];
for (const regex of condition.include) {
for (const fileName of files) {
Expand All @@ -297,10 +304,11 @@ export class ActionRunner {
* 3. It generates a status check in the Pull Request
* 4. WIP - It assigns the required reviewers to review the PR
*/
async runAction(inputs: Omit<Inputs, "repoToken">): Promise<CheckData> {
async runAction(inputs: Omit<Inputs, "repoToken">): Promise<Pick<CheckData, "conclusion"> & PullRequestReport> {
const config = await this.getConfigFile(inputs.configLocation);

const reports = await this.validatePullRequest(config);
const prValidation = await this.validatePullRequest(config);
const { reports } = prValidation;

this.logger.info(reports.length > 0 ? "There was an error with the PR reviews." : "The PR has been successful");

Expand All @@ -309,7 +317,9 @@ export class ActionRunner {

this.requestReviewers(reports);

return checkRunData;
setOutput("report", JSON.stringify(prValidation));

return { conclusion: checkRunData.conclusion, ...prValidation };
}
}

Expand Down
18 changes: 8 additions & 10 deletions src/test/runner/runner.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,33 +30,31 @@ describe("Shared validations", () => {
});

describe("listFilesThatMatchRuleCondition tests", () => {
test("should get values that match the condition", async () => {
test("should get values that match the condition", () => {
const mockRule = { condition: { include: ["src"] } };
api.listModifiedFiles.mockResolvedValue(["src/index.ts", "README.md"]);
const result = await runner.listFilesThatMatchRuleCondition(mockRule as Rule);
const result = runner.listFilesThatMatchRuleCondition(["src/index.ts", "README.md"], mockRule as Rule);
expect(result).toContainEqual("src/index.ts");
});

test("should return only one file even if more than one rule matches it", async () => {
test("should return only one file even if more than one rule matches it", () => {
const mockRule = { condition: { include: ["\\.ts", "src"] } };
api.listModifiedFiles.mockResolvedValue(["src/index.ts"]);
const result = await runner.listFilesThatMatchRuleCondition(mockRule as Rule);
const result = runner.listFilesThatMatchRuleCondition(["src/index.ts"], mockRule as Rule);
expect(result).toEqual(["src/index.ts"]);
});

test("should include all the files with a global value", async () => {
test("should include all the files with a global value", () => {
const mockRule = { condition: { include: [".+", "src"] } };
const listedFiles = ["src/index.ts", ".github/workflows/review-bot.yml", "yarn-error.log"];
api.listModifiedFiles.mockResolvedValue(listedFiles);
const result = await runner.listFilesThatMatchRuleCondition(mockRule as Rule);
const result = runner.listFilesThatMatchRuleCondition(listedFiles, mockRule as Rule);
expect(result).toEqual(listedFiles);
});

test("should exclude files if they are captured by the include condition", async () => {
test("should exclude files if they are captured by the include condition", () => {
const mockRule = { condition: { include: [".+"], exclude: ["\\.yml"] } };
const listedFiles = ["src/index.ts", ".github/workflows/review-bot.yml", "yarn-error.log"];
api.listModifiedFiles.mockResolvedValue(listedFiles);
const result = await runner.listFilesThatMatchRuleCondition(mockRule as Rule);
const result = runner.listFilesThatMatchRuleCondition(listedFiles, mockRule as Rule);
expect(result).toContainEqual("src/index.ts");
expect(result).toContainEqual("yarn-error.log");
expect(result).not.toContain(".github/workflows/review-bot.yml");
Expand Down
23 changes: 14 additions & 9 deletions src/test/runner/validation/and.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ describe("'And' rule validation", () => {
],
};
api.listApprovedReviewsAuthors.mockResolvedValue([users[0]]);
const evaluation = await runner.validatePullRequest(config);
expect(evaluation).toHaveLength(0);
const { reports } = await runner.validatePullRequest(config);
expect(reports).toHaveLength(0);
});

test("should not report errors if the reviewer belong to both conditions", async () => {
Expand All @@ -54,8 +54,8 @@ describe("'And' rule validation", () => {
],
};
api.listApprovedReviewsAuthors.mockResolvedValue([users[0]]);
const evaluation = await runner.validatePullRequest(config);
expect(evaluation).toHaveLength(0);
const { reports } = await runner.validatePullRequest(config);
expect(reports).toHaveLength(0);
});
});
describe("errors", () => {
Expand All @@ -73,7 +73,8 @@ describe("'And' rule validation", () => {
},
],
};
const [result] = await runner.validatePullRequest(config);
const { reports } = await runner.validatePullRequest(config);
const [result] = reports;
expect(result.missingReviews).toEqual(2);
expect(result.missingUsers).toEqual(users);
expect(result.teamsToRequest).toContainEqual("abc");
Expand All @@ -96,7 +97,8 @@ describe("'And' rule validation", () => {
},
],
};
const [result] = await runner.validatePullRequest(config);
const { reports } = await runner.validatePullRequest(config);
const [result] = reports;
expect(result.missingReviews).toEqual(5);
});

Expand All @@ -115,7 +117,8 @@ describe("'And' rule validation", () => {
],
};
api.listApprovedReviewsAuthors.mockResolvedValue([users[2]]);
const [result] = await runner.validatePullRequest(config);
const { reports } = await runner.validatePullRequest(config);
const [result] = reports;
expect(result.missingReviews).toEqual(1);
});

Expand All @@ -136,7 +139,8 @@ describe("'And' rule validation", () => {
const teamCba = [users[0], users[1]];
teamsApi.getTeamMembers.calledWith("cba").mockResolvedValue(teamCba);
api.listApprovedReviewsAuthors.mockResolvedValue([users[2]]);
const [result] = await runner.validatePullRequest(config);
const { reports } = await runner.validatePullRequest(config);
const [result] = reports;
expect(result.missingReviews).toEqual(1);
expect(result.missingUsers).toEqual(teamCba);
expect(result.teamsToRequest).toEqual(["cba"]);
Expand All @@ -160,7 +164,8 @@ describe("'And' rule validation", () => {
],
};
api.listApprovedReviewsAuthors.mockResolvedValue([users[2]]);
const [result] = await runner.validatePullRequest(config);
const { reports } = await runner.validatePullRequest(config);
const [result] = reports;
expect(result.missingReviews).toEqual(2);
expect(result.missingUsers).toEqual(individualUsers);
expect(result.teamsToRequest).toHaveLength(0);
Expand Down
18 changes: 10 additions & 8 deletions src/test/runner/validation/or.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ describe("'Or' rule validation", () => {
],
};
api.listApprovedReviewsAuthors.mockResolvedValue([users[0]]);
const evaluation = await runner.validatePullRequest(config);
expect(evaluation).toHaveLength(0);
const { reports } = await runner.validatePullRequest(config);
expect(reports).toHaveLength(0);
});

test("should not report errors if the reviewer belong to both conditions", async () => {
Expand All @@ -54,8 +54,8 @@ describe("'Or' rule validation", () => {
],
};
api.listApprovedReviewsAuthors.mockResolvedValue([users[0]]);
const evaluation = await runner.validatePullRequest(config);
expect(evaluation).toHaveLength(0);
const { reports } = await runner.validatePullRequest(config);
expect(reports).toHaveLength(0);
});

test("should not report errors if the reviewer belong to one of the conditions", async () => {
Expand All @@ -74,8 +74,8 @@ describe("'Or' rule validation", () => {
],
};
api.listApprovedReviewsAuthors.mockResolvedValue([users[2]]);
const evaluation = await runner.validatePullRequest(config);
expect(evaluation).toHaveLength(0);
const { reports } = await runner.validatePullRequest(config);
expect(reports).toHaveLength(0);
});
});
describe("errors", () => {
Expand All @@ -96,7 +96,8 @@ describe("'Or' rule validation", () => {
],
};
api.listApprovedReviewsAuthors.mockResolvedValue([]);
const [result] = await runner.validatePullRequest(config);
const { reports } = await runner.validatePullRequest(config);
const [result] = reports;
expect(result.missingReviews).toEqual(1);
expect(result.missingUsers).toEqual(users);
expect(result.teamsToRequest).toContainEqual("abc");
Expand All @@ -120,7 +121,8 @@ describe("'Or' rule validation", () => {
],
};
api.listApprovedReviewsAuthors.mockResolvedValue([]);
const [result] = await runner.validatePullRequest(config);
const { reports } = await runner.validatePullRequest(config);
const [result] = reports;
expect(result.missingReviews).toEqual(1);
});
});
Expand Down

0 comments on commit 13b897c

Please sign in to comment.