fix integration tests + couple of found bugs thanks for tests

helm/values-parity-stg.yaml

@@ -24,6 +24,7 @@ common:
     DB_CLIENT: postgres
     DB_PORT: 5432
     CMD_BOT_URL: https://command-bot.parity-stg.parity.io/
+    PIPELINE_SCRIPTS_REF: monorepo
     BOT_PR_COMMENT_MENTION: stgbot
   secrets:
     ALLOWED_ORGANIZATIONS: ref+vault://kv/gitlab/parity/mirrors/command-bot/devops-parity-stg#ALLOWED_ORGANIZATIONS

src/bot/events/onIssueCommentCreated.ts

@@ -15,9 +15,12 @@ import {
   removeReactionToComment,
   updateComment,
 } from "src/github";
+import { counters, getMetricsPrData } from "src/metrics";
 import { cancelTask, getNextTaskId, PullRequestTask, queueTask, serializeTaskQueuedDate } from "src/task";
 import { getLines } from "src/utils";
 
+const eventName = "issue_comment.created";
+
 export const onIssueCommentCreated: WebhookHandler<"issue_comment.created"> = async (ctx, octokit, payload) => {
   const { repositoryCloneDirectory, gitlab, logger } = ctx;
   const { issue, comment, repository, installation } = payload;
@@ -48,14 +51,15 @@ export const onIssueCommentCreated: WebhookHandler<"issue_comment.created"> = as
   let getError = (body: string) => new PullRequestError(pr, { body, requester, requesterCommentId: comment.id });
 
   try {
-    const commands: ParsedCommand[] = [];
+    const commands: (ParsedCommand | SkipEvent)[] = [];
+
+    if (!(await isRequesterAllowed(ctx, octokit, requester))) {
+      return getError("Requester could not be detected as a member of an allowed organization.");
+    }
+
     for (const line of getLines(comment.body)) {
       const parsedCommand = await parsePullRequestBotCommandLine(line, ctx, pr.repo);
 
-      if (parsedCommand instanceof SkipEvent) {
-        continue;
-      }
-
       if (parsedCommand instanceof Error) {
         return getError(parsedCommand.message);
       }
@@ -67,13 +71,18 @@ export const onIssueCommentCreated: WebhookHandler<"issue_comment.created"> = as
       return new SkipEvent("No commands found within a comment");
     }
 
-    if (!(await isRequesterAllowed(ctx, octokit, requester))) {
-      return getError("Requester could not be detected as a member of an allowed organization.");
-    }
-
     for (const parsedCommand of commands) {
       logger.debug({ parsedCommand }, "Processing parsed command");
 
+      if (parsedCommand instanceof SkipEvent) {
+        const skip = getMetricsPrData("skip", eventName, pr, parsedCommand.reason);
+        counters.commandsRun.inc({ ...skip });
+        logger.debug(
+          { command: comment.body, payload, ...skip },
+          `Skip command with reason: "${parsedCommand.reason}"`,
+        );
+      }
+
       if (parsedCommand instanceof HelpCommand) {
         await createComment(ctx, octokit, {
           ...commentParams,

src/bot/parse/parsePullRequestBotCommandLine.namedArgs.spec.ts

@@ -175,6 +175,11 @@ const dataProvider: DataProvider[] = [
     commandLine: "bot help",
     expectedResponse: new HelpCommand("http://cmd-bot.docs.com/static/docs/latest.html"),
   },
+  {
+    suitName: "help",
+    commandLine: "bot help -v PIPELINE_SCRIPTS_REF=help-branch",
+    expectedResponse: new HelpCommand("http://cmd-bot.docs.com/static/docs/latest.html"),
+  },
   { suitName: "clean", commandLine: "bot clean", expectedResponse: new CleanCommand() },
   { suitName: "clear", commandLine: "bot clear", expectedResponse: new CleanCommand() },
 
@@ -193,7 +198,21 @@ const dataProvider: DataProvider[] = [
     expectedResponse: new SkipEvent("Not a command"),
   },
   { suitName: "no subcommand - ignore", commandLine: "bot ", expectedResponse: new SkipEvent("Not a command") },
-  { suitName: "ignored command", commandLine: "bot merge", expectedResponse: new SkipEvent("Ignored command: merge") },
+  {
+    suitName: "ignored command merge",
+    commandLine: "bot merge",
+    expectedResponse: new SkipEvent("Ignored command: merge"),
+  },
+  {
+    suitName: "ignored command merge force",
+    commandLine: "bot merge force",
+    expectedResponse: new SkipEvent("Ignored command: merge"),
+  },
+  {
+    suitName: "ignored command rebase",
+    commandLine: "bot rebase",
+    expectedResponse: new SkipEvent("Ignored command: rebase"),
+  },
   {
     suitName: "ignored command 2",
     commandLine: "bot rebase",

src/bot/parse/parsePullRequestBotCommandLine.ts

@@ -14,7 +14,7 @@ export const parsePullRequestBotCommandLine = async (
   let commandLine = rawCommandLine.trim();
   const { botPullRequestCommentMention } = config;
 
-  // Add trailing whitespace so that bot can be differentiated from /cmd-[?]
+  // Add trailing whitespace so that bot can be differentiated
   if (!commandLine.startsWith(`${botPullRequestCommentMention} `)) {
     return new SkipEvent("Not a command");
   }

src/bot/setupEvent.ts

@@ -1,17 +1,10 @@
 import { Probot } from "probot";
 
 import { extractPullRequestData } from "src/bot/parse/extractPullRequestData";
-import {
-  FinishedEvent,
-  PullRequestError,
-  SkipEvent,
-  WebhookEventPayload,
-  WebhookEvents,
-  WebhookHandler,
-} from "src/bot/types";
+import { FinishedEvent, PullRequestError, WebhookEventPayload, WebhookEvents, WebhookHandler } from "src/bot/types";
 import { createComment, getOctokit, updateComment } from "src/github";
 import { logger as parentLogger } from "src/logger";
-import { CommandRunLabels, CommandRunType, counters, summaries } from "src/metrics";
+import { counters, getMetricsPrData, summaries } from "src/metrics";
 import { Context } from "src/types";
 
 export const setupEvent = <E extends WebhookEvents>(
@@ -31,9 +24,6 @@ export const setupEvent = <E extends WebhookEvents>(
     const installationId: number | undefined =
       "installation" in event.payload ? event.payload.installation?.id : undefined;
     const octokit = getOctokit(await bot.auth(installationId), ctx);
-    const getMetricsPrData = (type: CommandRunType, body?: string): CommandRunLabels => {
-      return { eventName, type, repo: prData.repo, pr: prData.number.toString(10), body };
-    };
 
     const commandHandlingDurationTimer = summaries.commandHandlingDuration.startTimer({
       eventName,
@@ -53,35 +43,28 @@ export const setupEvent = <E extends WebhookEvents>(
             body: `${comment.requester ? `@${comment.requester} ` : ""}${comment.body}`,
           };
 
-          const warn = getMetricsPrData("warn", comment.body);
+          const warn = getMetricsPrData("warn", eventName, prData, comment.body);
           counters.commandsRun.inc({ ...warn, repo: pr.repo, pr: pr.number });
 
           eventLogger.warn(sharedCommentParams, `Got PullRequestError ${pr.repo}#${pr.number} -> ${comment.body}`);
 
           comment.botCommentId
             ? await updateComment(ctx, octokit, { ...sharedCommentParams, comment_id: comment.botCommentId })
             : await createComment(ctx, octokit, sharedCommentParams);
-        } else if (result instanceof SkipEvent && !!result.reason.trim()) {
-          const skip = getMetricsPrData("skip", result.reason);
-          counters.commandsRun.inc({ ...skip });
-          eventLogger.debug(
-            { command: event.payload.comment.body, payload: event.payload, ...skip },
-            `Skip command with reason: "${result.reason}"`,
-          );
         } else if (result instanceof FinishedEvent) {
-          const ok = getMetricsPrData("ok", result.comment.body);
+          const ok = getMetricsPrData("ok", eventName, prData, result.comment.body);
           counters.commandsRun.inc({ ...ok, repo: result.pr.repo, pr: result.pr.number });
           eventLogger.info({ result }, "Finished command");
         } else {
           const message = "Unknown result type";
-          const error = getMetricsPrData("error", message);
+          const error = getMetricsPrData("error", eventName, prData, message);
           counters.commandsRun.inc({ ...error });
           eventLogger.error({ event: event.payload, result, ...error }, message);
         }
       })
       .catch((error: Error) => {
         const msg = "Exception caught in webhook handler";
-        const fatal = getMetricsPrData("fatal", `${msg}: ${error.message}`);
+        const fatal = getMetricsPrData("fatal", eventName, prData, `${msg}: ${error.message}`);
         counters.commandsRun.inc({ ...fatal });
         eventLogger.fatal({ body: error, ...fatal }, msg);
       })

src/commander/commander.ts

@@ -39,7 +39,8 @@ export function getCommanderFromConfiguration(
   root.addHelpCommand(false);
   root
     .command("help")
-    .exitOverride()
+    .addOption(getVariablesOption())
+    .exitOverride(variablesExitOverride)
     .action(() => {
       parsedCommand = new HelpCommand(docsPath);
     });
@@ -62,7 +63,7 @@ export function getCommanderFromConfiguration(
 
   for (const ignoredCommand of botPullRequestIgnoreCommands) {
     root
-      .command(ignoredCommand, { hidden: true })
+      .command(ignoredCommand)
       .exitOverride()
       .action(() => {
         parsedCommand = new SkipEvent(`Ignored command: ${ignoredCommand}`);

src/commander/fetchConfigsCommander.ts

@@ -18,7 +18,7 @@ type ExtendedFetchCommander = Command & {
 export function fetchConfigsCommander(ctx: LoggerContext, repo: string): ExtendedFetchCommander {
   const { botPullRequestCommentMention } = config;
   const root = new Command(botPullRequestCommentMention) as ExtendedFetchCommander;
-  let scriptsBranch = config.pipelineScripts.ref;
+  let scriptsBranch: string | undefined;
 
   root.addOption(getVariablesOption());
 

src/metrics.ts

@@ -1,5 +1,7 @@
 import client from "prom-client";
 
+import { PullRequestData } from "src/bot/types";
+
 // There are duplicated string literals here, but they aren't related to each other and aren't magic strings
 /* eslint-disable sonarjs/no-duplicate-string */
 export type CommandRunLabels = {
@@ -30,4 +32,13 @@ export const summaries: { commandHandlingDuration: client.Summary } = {
   }),
 };
 
+export const getMetricsPrData = (
+  type: CommandRunType,
+  eventName: string,
+  prData: PullRequestData,
+  body?: string,
+): CommandRunLabels => {
+  return { eventName, type, repo: prData.repo, pr: prData.number.toString(10), body };
+};
+
 client.collectDefaultMetrics({ prefix: "command_bot_" });

src/test/fixtures.ts

@@ -9,12 +9,12 @@ import { getPipelinePayload } from "./fixtures/gitlab/pipeline";
 export const webhookFixtures: Record<string, (params: CommentWebhookParams) => string> = {
   startCommandComment: (params) =>
     getCommentWebhookPayload({
-      ...{ body: "bot sample $ hi", org: "paritytech-stg", repo: "command-bot-test", login: "somedev123" },
+      ...{ body: "testbot sample --input=hi", org: "paritytech-stg", repo: "command-bot-test", login: "somedev123" },
       ...params,
     }),
   cancelCommandComment: (params) =>
     getCommentWebhookPayload({
-      ...{ body: "bot cancel", org: "paritytech-stg", repo: "command-bot-test", login: "somedev123" },
+      ...{ body: "testbot cancel", org: "paritytech-stg", repo: "command-bot-test", login: "somedev123" },
       ...params,
     }),
 };

src/test/github-ignore-comands.spec.ts

@@ -1,22 +1,44 @@
 import { ensureDefined } from "@eng-automation/js";
 import { beforeAll, describe, expect, test } from "@jest/globals";
 
+import { getRestFixtures } from "src/test/fixtures";
+import { getMockServers } from "src/test/setup/mockServers";
+
 import { triggerWebhook } from "./helpers";
 import { getBotInstance } from "./setup/bot";
 import { initRepo, startGitDaemons } from "./setup/gitDaemons";
 
+const jsonResponseHeaders = { "content-type": "application/json" };
+
+const restFixures = getRestFixtures({
+  github: {
+    org: "paritytech-stg",
+    repo: "command-bot-test",
+    prAuthor: "somedev123",
+    headBranch: "prBranch1",
+    comments: [{ author: "somedev123", body: "testbot merge", id: 500 }],
+  },
+  gitlab: { cmdBranch: "cmd-bot/4-1" },
+});
+
 type CommandDataProviderItem = {
   suitName: string;
   commandLine: string;
 };
 const commandsDataProvider: CommandDataProviderItem[] = [
-  { suitName: "[merge] command to ignore", commandLine: "bot merge" },
-  { suitName: "[merge *] command to ignore", commandLine: "bot merge force" },
-  { suitName: "[rebase] command to ignore", commandLine: "bot rebase" },
+  { suitName: "[merge] command to ignore", commandLine: "testbot merge" },
+  { suitName: "[merge *] command to ignore", commandLine: "testbot merge force" },
+  { suitName: "[rebase] command to ignore", commandLine: "testbot rebase" },
 ];
 
 beforeAll(async () => {
   const gitDaemons = await startGitDaemons();
+  const mockServers = ensureDefined(getMockServers());
+  await mockServers.gitHub
+    .forPost("/app/installations/25299948/access_tokens")
+    .thenReply(200, restFixures.github.appInstallationToken, jsonResponseHeaders);
+
+  await mockServers.gitHub.forGet("/organizations/123/members/somedev123").thenReply(204);
 
   await initRepo(gitDaemons.gitHub, "paritytech-stg", "command-bot-test.git", []);
   await initRepo(gitDaemons.gitHub, "somedev123", "command-bot-test.git", ["prBranch1"]);

src/test/github-job-cancellation.spec.ts

@@ -14,7 +14,7 @@ const restFixures = getRestFixtures({
     repo: "command-bot-test",
     prAuthor: "somedev123",
     headBranch: "prBranch1",
-    comments: [{ author: "somedev123", body: "bot sample $ hi", id: 500 }],
+    comments: [{ author: "somedev123", body: "testbot sample --input=hi", id: 500 }],
   },
   gitlab: { cmdBranch: "cmd-bot/4-1" },
 });
@@ -24,7 +24,7 @@ const jsonResponseHeaders = { "content-type": "application/json" };
 const mockedEndpoints: Record<string, MockedEndpoint> = {};
 
 describe("Job cancellation (GitHub webhook)", () => {
-  const cancelCommandRegex = /`bot cancel (.+?)`/;
+  const cancelCommandRegex = /`testbot cancel (.+?)`/;
   let lastCommentBody: string = "";
 
   const getCommentResponse = async (request: CompletedRequest) => {
@@ -96,7 +96,7 @@ describe("Job cancellation (GitHub webhook)", () => {
       () => lastCommentBody.match(cancelCommandRegex) !== null,
       100,
       50,
-      `Expected comment body to include "bot cancel". Instead, got: ${lastCommentBody}`,
+      `Expected comment body to include "testbot cancel". Instead, got: ${lastCommentBody}`,
     );
   });
 
@@ -108,7 +108,7 @@ describe("Job cancellation (GitHub webhook)", () => {
       .thenReply(200, restFixures.gitlab.cancelledPipeline, jsonResponseHeaders);
 
     const commandId = ensureDefined(lastCommentBody.match(cancelCommandRegex)?.[1]);
-    await triggerWebhook("cancelCommandComment", { body: `bot cancel ${commandId}` });
+    await triggerWebhook("cancelCommandComment", { body: `testbot cancel ${commandId}` });
 
     await until(async () => !(await mockedEndpoint.isPending()), 300, 50);
   });
@@ -133,7 +133,7 @@ describe("Job cancellation (GitHub webhook)", () => {
       .forPatch("/repos/paritytech-stg/command-bot-test/issues/comments/555")
       .thenCallback(getCommentResponse);
 
-    await triggerWebhook("cancelCommandComment", { body: `bot cancel` });
+    await triggerWebhook("cancelCommandComment", { body: `testbot cancel` });
 
     await until(async () => !(await mockedEndpoint.isPending()), 300, 50);
   });

src/test/github-job-failure.spec.ts

@@ -14,7 +14,7 @@ const restFixures = getRestFixtures({
     repo: "command-bot-test",
     prAuthor: "somedev123",
     headBranch: "prBranch1",
-    comments: [{ author: "somedev123", body: "bot sample $ hi", id: 500 }],
+    comments: [{ author: "somedev123", body: "testbot sample --input=hi", id: 500 }],
   },
   gitlab: { cmdBranch: "cmd-bot/4-1" },
 });

src/test/github-non-pipeline-cases.spec.ts

@@ -35,42 +35,44 @@ type CommandDataProviderItem = {
 const commandsDataProvider: CommandDataProviderItem[] = [
   {
     suitName: "[help] command",
-    commandLine: "bot help",
-    expected: { startMessage: "Here's a [link to docs](http://localhost:3000/static/docs/latest.html" },
+    commandLine: "testbot help",
+    expected: {
+      startMessage: "Here's a [link to docs](http://localhost:3000/static/docs/latest.html?repo=command-bot-test)",
+    },
   },
   {
     suitName: "[help] command with branch override",
-    commandLine: "bot help -v PIPELINE_SCRIPTS_REF=tests",
+    commandLine: "testbot help -v PIPELINE_SCRIPTS_REF=tests",
     expected: {
       startMessage:
         "Here's a [link to docs](http://localhost:3000/static/docs/1245345a4376f18f3ef98195055876ff293f8622.html",
     },
   },
   {
     suitName: "[wrong] command",
-    commandLine: "bot hrlp", // intentional typo
+    commandLine: "testbot hrlp", // intentional typo
     expected: {
       startMessage:
-        '@somedev123 Unknown command "hrlp"; Available ones are bench-all, bench-overhead, bench-vm, bench, fmt, merge, rebase, sample, try-runtime, update-ui. Refer to [help docs](http://localhost:3000/static/docs/latest.html) and/or [source code](https://github.com/paritytech/command-bot-scripts).',
+        '@somedev123 Unknown command "hrlp"; Available ones are bench-all, bench-bm, bench-overhead, bench, fmt, merge, rebase, sample, try-runtime, update-ui. Refer to [help docs](http://localhost:3000/static/docs/latest.html?repo=command-bot-test) and/or [source code](https://github.com/paritytech/command-bot-scripts).',
     },
   },
   {
     suitName: "[not allowed] command",
-    commandLine: "bot bench-all", // command-bot-test repo is not in a list of repos: [...] array
+    commandLine: "testbot bench-all", // command-bot-test repo is not in a list of repos: [...] array
     expected: {
       startMessage:
-        '@somedev123 The command: "bench-all" is not supported in **command-bot-test** repository. Refer to [help docs](http://localhost:3000/static/docs/latest.html) and/or [source code](https://github.com/paritytech/command-bot-scripts).',
+        '@somedev123 Missing arguments for command "bench-all". Refer to [help docs](http://localhost:3000/static/docs/latest.html?repo=command-bot-test) and/or [source code](https://github.com/paritytech/command-bot-scripts).',
     },
   },
   {
     suitName: "[cancel] command for no jobs",
-    commandLine: "bot cancel",
+    commandLine: "testbot cancel",
     expected: { startMessage: "@somedev123 No task is being executed for this pull request" },
   },
   // TODO: add test for clean after moving to opstooling-testing
   // {
   //   suitName: "[clean] command",
-  //   commandLine: "bot clean",
+  //   commandLine: "testbot clean",
   //   expected: {
   //     startReaction: "eyes",
   //     finishReaction: "+1"