Skip to content

Commit

Permalink
feat: update the default comment on error
Browse files Browse the repository at this point in the history
  • Loading branch information
lampajr committed Apr 10, 2024
1 parent 7c42053 commit 8b0412b
Show file tree
Hide file tree
Showing 10 changed files with 75 additions and 29 deletions.
25 changes: 19 additions & 6 deletions dist/cli/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,9 @@ exports["default"] = ConfigsParser;
"use strict";

Object.defineProperty(exports, "__esModule", ({ value: true }));
exports.AuthTokenId = exports.MESSAGE_ERROR_PLACEHOLDER = void 0;
exports.AuthTokenId = exports.MESSAGE_TARGET_BRANCH_PLACEHOLDER = exports.MESSAGE_ERROR_PLACEHOLDER = void 0;
exports.MESSAGE_ERROR_PLACEHOLDER = "{{error}}";
exports.MESSAGE_TARGET_BRANCH_PLACEHOLDER = "{{target-branch}}";
var AuthTokenId;
(function (AuthTokenId) {
// github specific token
Expand Down Expand Up @@ -391,7 +392,7 @@ class PullRequestConfigsParser extends configs_parser_1.default {
}
getDefaultErrorComment() {
// TODO: fetch from arg or set default with placeholder {{error}}
return `Backporting failed: ${configs_types_1.MESSAGE_ERROR_PLACEHOLDER}`;
return `The backport to \`${configs_types_1.MESSAGE_TARGET_BRANCH_PLACEHOLDER}\` failed. Check the latest run for more details.`;
}
/**
* Parse the provided labels and return a list of target branches
Expand Down Expand Up @@ -1461,18 +1462,29 @@ exports["default"] = Logger;
"use strict";

Object.defineProperty(exports, "__esModule", ({ value: true }));
exports.injectError = void 0;
exports.injectTargetBranch = exports.injectError = void 0;
const configs_types_1 = __nccwpck_require__(4753);
/**
* Inject the error message in the provided `message`.
* This is inject in place of the MESSAGE_ERROR_PLACEHOLDER placeholder
* This is injected in place of the MESSAGE_ERROR_PLACEHOLDER placeholder
* @param message string that needs to be updated
* @param errMsg the error message that needs to be injected
*/
const injectError = (message, errMsg) => {
return message.replace(configs_types_1.MESSAGE_ERROR_PLACEHOLDER, errMsg);
};
exports.injectError = injectError;
/**
* Inject the target branch into the provided `message`.
* This is injected in place of the MESSAGE_TARGET_BRANCH_PLACEHOLDER placeholder
* @param message string that needs to be updated
* @param targetBranch the target branch to inject
* @returns
*/
const injectTargetBranch = (message, targetBranch) => {
return message.replace(configs_types_1.MESSAGE_TARGET_BRANCH_PLACEHOLDER, targetBranch);
};
exports.injectTargetBranch = injectTargetBranch;


/***/ }),
Expand Down Expand Up @@ -1559,8 +1571,9 @@ class Runner {
this.logger.error(`Something went wrong backporting to ${pr.base}: ${error}`);
if (!configs.dryRun && configs.errorNotification.enabled && configs.errorNotification.message.length > 0) {
// notify the failure as comment in the original pull request
const comment = (0, runner_util_1.injectError)(configs.errorNotification.message, error);
gitApi.createPullRequestComment(configs.originalPullRequest.url, comment);
let comment = (0, runner_util_1.injectError)(configs.errorNotification.message, error);
comment = (0, runner_util_1.injectTargetBranch)(comment, pr.base);
await gitApi.createPullRequestComment(configs.originalPullRequest.url, comment);
}
failures.push(error);
}
Expand Down
25 changes: 19 additions & 6 deletions dist/gha/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,9 @@ exports["default"] = ConfigsParser;
"use strict";

Object.defineProperty(exports, "__esModule", ({ value: true }));
exports.AuthTokenId = exports.MESSAGE_ERROR_PLACEHOLDER = void 0;
exports.AuthTokenId = exports.MESSAGE_TARGET_BRANCH_PLACEHOLDER = exports.MESSAGE_ERROR_PLACEHOLDER = void 0;
exports.MESSAGE_ERROR_PLACEHOLDER = "{{error}}";
exports.MESSAGE_TARGET_BRANCH_PLACEHOLDER = "{{target-branch}}";
var AuthTokenId;
(function (AuthTokenId) {
// github specific token
Expand Down Expand Up @@ -356,7 +357,7 @@ class PullRequestConfigsParser extends configs_parser_1.default {
}
getDefaultErrorComment() {
// TODO: fetch from arg or set default with placeholder {{error}}
return `Backporting failed: ${configs_types_1.MESSAGE_ERROR_PLACEHOLDER}`;
return `The backport to \`${configs_types_1.MESSAGE_TARGET_BRANCH_PLACEHOLDER}\` failed. Check the latest run for more details.`;
}
/**
* Parse the provided labels and return a list of target branches
Expand Down Expand Up @@ -1426,18 +1427,29 @@ exports["default"] = Logger;
"use strict";

Object.defineProperty(exports, "__esModule", ({ value: true }));
exports.injectError = void 0;
exports.injectTargetBranch = exports.injectError = void 0;
const configs_types_1 = __nccwpck_require__(4753);
/**
* Inject the error message in the provided `message`.
* This is inject in place of the MESSAGE_ERROR_PLACEHOLDER placeholder
* This is injected in place of the MESSAGE_ERROR_PLACEHOLDER placeholder
* @param message string that needs to be updated
* @param errMsg the error message that needs to be injected
*/
const injectError = (message, errMsg) => {
return message.replace(configs_types_1.MESSAGE_ERROR_PLACEHOLDER, errMsg);
};
exports.injectError = injectError;
/**
* Inject the target branch into the provided `message`.
* This is injected in place of the MESSAGE_TARGET_BRANCH_PLACEHOLDER placeholder
* @param message string that needs to be updated
* @param targetBranch the target branch to inject
* @returns
*/
const injectTargetBranch = (message, targetBranch) => {
return message.replace(configs_types_1.MESSAGE_TARGET_BRANCH_PLACEHOLDER, targetBranch);
};
exports.injectTargetBranch = injectTargetBranch;


/***/ }),
Expand Down Expand Up @@ -1524,8 +1536,9 @@ class Runner {
this.logger.error(`Something went wrong backporting to ${pr.base}: ${error}`);
if (!configs.dryRun && configs.errorNotification.enabled && configs.errorNotification.message.length > 0) {
// notify the failure as comment in the original pull request
const comment = (0, runner_util_1.injectError)(configs.errorNotification.message, error);
gitApi.createPullRequestComment(configs.originalPullRequest.url, comment);
let comment = (0, runner_util_1.injectError)(configs.errorNotification.message, error);
comment = (0, runner_util_1.injectTargetBranch)(comment, pr.base);
await gitApi.createPullRequestComment(configs.originalPullRequest.url, comment);
}
failures.push(error);
}
Expand Down
1 change: 1 addition & 0 deletions src/service/configs/configs.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import { BackportPullRequest, GitPullRequest } from "@bp/service/git/git.types";

export const MESSAGE_ERROR_PLACEHOLDER = "{{error}}";
export const MESSAGE_TARGET_BRANCH_PLACEHOLDER = "{{target-branch}}";

export interface LocalGit {
user: string, // local git user
Expand Down
4 changes: 2 additions & 2 deletions src/service/configs/pullrequest/pr-configs-parser.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { getAsCleanedCommaSeparatedList, getAsCommaSeparatedList } from "@bp/service/args/args-utils";
import { Args } from "@bp/service/args/args.types";
import ConfigsParser from "@bp/service/configs/configs-parser";
import { Configs, MESSAGE_ERROR_PLACEHOLDER } from "@bp/service/configs/configs.types";
import { Configs, MESSAGE_TARGET_BRANCH_PLACEHOLDER } from "@bp/service/configs/configs.types";
import GitClient from "@bp/service/git/git-client";
import GitClientFactory from "@bp/service/git/git-client-factory";
import { BackportPullRequest, GitPullRequest } from "@bp/service/git/git.types";
Expand Down Expand Up @@ -72,7 +72,7 @@ export default class PullRequestConfigsParser extends ConfigsParser {

private getDefaultErrorComment(): string {
// TODO: fetch from arg or set default with placeholder {{error}}
return `Backporting failed: ${MESSAGE_ERROR_PLACEHOLDER}`;
return `The backport to \`${MESSAGE_TARGET_BRANCH_PLACEHOLDER}\` failed. Check the latest run for more details.`;
}

/**
Expand Down
15 changes: 13 additions & 2 deletions src/service/runner/runner-util.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,22 @@
import { MESSAGE_ERROR_PLACEHOLDER } from "@bp/service/configs/configs.types";
import { MESSAGE_ERROR_PLACEHOLDER, MESSAGE_TARGET_BRANCH_PLACEHOLDER } from "@bp/service/configs/configs.types";

/**
* Inject the error message in the provided `message`.
* This is inject in place of the MESSAGE_ERROR_PLACEHOLDER placeholder
* This is injected in place of the MESSAGE_ERROR_PLACEHOLDER placeholder
* @param message string that needs to be updated
* @param errMsg the error message that needs to be injected
*/
export const injectError = (message: string, errMsg: string): string => {
return message.replace(MESSAGE_ERROR_PLACEHOLDER, errMsg);
};

/**
* Inject the target branch into the provided `message`.
* This is injected in place of the MESSAGE_TARGET_BRANCH_PLACEHOLDER placeholder
* @param message string that needs to be updated
* @param targetBranch the target branch to inject
* @returns
*/
export const injectTargetBranch = (message: string, targetBranch: string): string => {
return message.replace(MESSAGE_TARGET_BRANCH_PLACEHOLDER, targetBranch);
};
7 changes: 4 additions & 3 deletions src/service/runner/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { BackportPullRequest, GitClientType, GitPullRequest } from "@bp/service/
import LoggerService from "@bp/service/logger/logger-service";
import LoggerServiceFactory from "@bp/service/logger/logger-service-factory";
import { inferGitClient, inferGitApiUrl, getGitTokenFromEnv } from "@bp/service/git/git-util";
import { injectError } from "./runner-util";
import { injectError, injectTargetBranch } from "./runner-util";

interface Git {
gitClientType: GitClientType;
Expand Down Expand Up @@ -95,8 +95,9 @@ export default class Runner {
this.logger.error(`Something went wrong backporting to ${pr.base}: ${error}`);
if (!configs.dryRun && configs.errorNotification.enabled && configs.errorNotification.message.length > 0) {
// notify the failure as comment in the original pull request
const comment = injectError(configs.errorNotification.message, error as string);
gitApi.createPullRequestComment(configs.originalPullRequest.url, comment);
let comment = injectError(configs.errorNotification.message, error as string);
comment = injectTargetBranch(comment, pr.base);
await gitApi.createPullRequestComment(configs.originalPullRequest.url, comment);
}
failures.push(error as string);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ describe("github pull request config parser", () => {
});
expect(configs.errorNotification).toEqual({
enabled: false,
message: "Backporting failed: {{error}}"
message: "The backport to `{{target-branch}}` failed. Check the latest run for more details."
});
});

Expand Down Expand Up @@ -962,7 +962,7 @@ describe("github pull request config parser", () => {

expect(configs.errorNotification).toEqual({
"enabled": true,
"message": "Backporting failed: {{error}}"
"message": "The backport to `{{target-branch}}` failed. Check the latest run for more details."
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ describe("gitlab merge request config parser", () => {
});
expect(configs.errorNotification).toEqual({
"enabled": false,
"message": "Backporting failed: {{error}}"
"message": "The backport to `{{target-branch}}` failed. Check the latest run for more details."
});
});

Expand Down Expand Up @@ -905,7 +905,7 @@ describe("gitlab merge request config parser", () => {

expect(configs.errorNotification).toEqual({
"enabled": true,
"message": "Backporting failed: {{error}}",
"message": "The backport to `{{target-branch}}` failed. Check the latest run for more details.",
});
});
});
7 changes: 3 additions & 4 deletions test/service/runner/cli-github-runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1238,7 +1238,6 @@ describe("cli runner", () => {

test("with multiple target branches, one failure and error notification enabled", async () => {
jest.spyOn(GitHubClient.prototype, "createPullRequest").mockImplementation((backport: BackportPullRequest) => {

throw new Error(`Mocked error: ${backport.base}`);
});

Expand Down Expand Up @@ -1323,9 +1322,9 @@ describe("cli runner", () => {
});
expect(GitHubClient.prototype.createPullRequest).toThrowError();
expect(GitHubClient.prototype.createPullRequestComment).toBeCalledTimes(3);
expect(GitHubClient.prototype.createPullRequestComment).toBeCalledWith("https://api.github.com/repos/owner/reponame/pulls/2368", "Backporting failed: Error: Mocked error: v1");
expect(GitHubClient.prototype.createPullRequestComment).toBeCalledWith("https://api.github.com/repos/owner/reponame/pulls/2368", "Backporting failed: Error: Mocked error: v2");
expect(GitHubClient.prototype.createPullRequestComment).toBeCalledWith("https://api.github.com/repos/owner/reponame/pulls/2368", "Backporting failed: Error: Mocked error: v3");
expect(GitHubClient.prototype.createPullRequestComment).toBeCalledWith("https://api.github.com/repos/owner/reponame/pulls/2368", "The backport to `v1` failed. Check the latest run for more details.");
expect(GitHubClient.prototype.createPullRequestComment).toBeCalledWith("https://api.github.com/repos/owner/reponame/pulls/2368", "The backport to `v2` failed. Check the latest run for more details.");
expect(GitHubClient.prototype.createPullRequestComment).toBeCalledWith("https://api.github.com/repos/owner/reponame/pulls/2368", "The backport to `v3` failed. Check the latest run for more details.");
});

test("with some failures and dry run enabled", async () => {
Expand Down
12 changes: 10 additions & 2 deletions test/service/runner/runner-util.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
import { injectError } from "@bp/service/runner/runner-util";
import { injectError, injectTargetBranch } from "@bp/service/runner/runner-util";

describe("check runner utilities", () => {
test("properly inject error message", () => {
expect(injectError("Original message: {{error}}", "to inject")).toStrictEqual("Original message: to inject");
});

test("missing placeholder in the original message", () => {
test("missing error placeholder in the original message", () => {
expect(injectError("Original message: {{wrong}}", "to inject")).toStrictEqual("Original message: {{wrong}}");
});

test("properly inject target branch into message", () => {
expect(injectTargetBranch("Original message: {{target-branch}}", "to inject")).toStrictEqual("Original message: to inject");
});

test("missing target branch placeholder in the original message", () => {
expect(injectTargetBranch("Original message: {{wrong}}", "to inject")).toStrictEqual("Original message: {{wrong}}");
});
});

0 comments on commit 8b0412b

Please sign in to comment.