Merge branch 'main' into DOP-4474

mongodb · May 21, 2024 · eeab660 · eeab660
2 parents 3d38a9e + 228c1fb
commit eeab660
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 49 deletions.
diff --git a/Dockerfile b/Dockerfile
@@ -22,8 +22,8 @@ RUN cd ./modules/oas-page-builder \
 # where repo work will happen
 FROM ubuntu:20.04
 ARG WORK_DIRECTORY=/home/docsworker-xlarge
-ARG SNOOTY_PARSER_VERSION=0.16.6
-ARG SNOOTY_FRONTEND_VERSION=0.16.13
+ARG SNOOTY_PARSER_VERSION=0.16.7
+ARG SNOOTY_FRONTEND_VERSION=0.16.14
 ARG MUT_VERSION=0.11.2
 ARG REDOC_CLI_VERSION=1.2.3
 ARG NPM_BASE_64_AUTH

diff --git a/Dockerfile.local b/Dockerfile.local
@@ -1,8 +1,8 @@
 FROM arm64v8/ubuntu:20.04 as initial
 ARG NPM_BASE_64_AUTH
 ARG NPM_EMAIL
-ARG SNOOTY_PARSER_VERSION=0.16.6
-ARG SNOOTY_FRONTEND_VERSION=0.16.13
+ARG SNOOTY_PARSER_VERSION=0.16.7
+ARG SNOOTY_FRONTEND_VERSION=0.16.14
 ARG MUT_VERSION=0.11.2
 ARG REDOC_CLI_VERSION=1.2.3
 ARG NPM_BASE_64_AUTH

diff --git a/api/controllers/v1/slack.ts b/api/controllers/v1/slack.ts
@@ -50,7 +50,7 @@ export const DisplayRepoOptions = async (event: APIGatewayEvent): Promise<APIGat
 
   const entitledBranches = await buildEntitledGroupsList(entitlement, repoBranchesRepository);
   const resp = await slackConnector.displayRepoOptions(entitledBranches, key_val['trigger_id'], isAdmin);
-  if (resp?.status == 200 && resp?.data) {
+  if (resp?.status == 200 && resp?.data?.ok) {
     return {
       statusCode: 200,
       body: 'Model requested',

diff --git a/api/handlers/slack.ts b/api/handlers/slack.ts
@@ -23,45 +23,47 @@ export async function buildEntitledGroupsList(entitlement: any, repoBranchesRepo
   const repoOptions: any[] = [];
   for (const repo of entitlement.repos) {
     const [repoOwner, repoName, directoryPath] = repo.split('/');
-
     const branches = await repoBranchesRepository.getRepoBranches(repoName, directoryPath);
-    const options: any[] = [];
-    for (const branch of branches) {
-      const buildWithSnooty = branch['buildsWithSnooty'];
-      if (buildWithSnooty) {
-        const active = branch['active'];
-        const branchName = `${directoryPath ? `${directoryPath}/` : ''}${branch['gitBranchName']}`;
-        const repoPath = `${repoOwner}/${repoName}/${branchName}`;
-        let txt: string;
-        if (!active) {
-          txt = `(!inactive) ${repoPath}`;
-        } else {
-          txt = repoPath;
+
+    if (branches.length) {
+      const options: any[] = [];
+      for (const branch of branches) {
+        const buildWithSnooty = branch['buildsWithSnooty'];
+        if (buildWithSnooty) {
+          const active = branch['active'];
+          const branchName = `${directoryPath ? `${directoryPath}/` : ''}${branch['gitBranchName']}`;
+          const repoPath = `${repoOwner}/${repoName}/${branchName}`;
+          let txt: string;
+          if (!active) {
+            txt = `(!inactive) ${repoPath}`;
+          } else {
+            txt = repoPath;
+          }
+          options.push({
+            text: {
+              type: 'plain_text',
+              text: txt,
+            },
+            value: repoPath,
+          });
         }
-        options.push({
-          text: {
-            type: 'plain_text',
-            text: txt,
-          },
-          value: repoPath,
-        });
       }
-    }
 
-    const repoOption = {
-      label: {
-        type: 'plain_text',
-        text: repoName,
-      },
-      //sort the options by version number
-      options: options.sort((branchOne, branchTwo) =>
-        branchTwo.text.text
-          .toString()
-          .replace(/\d+/g, (n) => +n + 100000)
-          .localeCompare(branchOne.text.text.toString().replace(/\d+/g, (n) => +n + 100000))
-      ),
-    };
-    repoOptions.push(repoOption);
+      const repoOption = {
+        label: {
+          type: 'plain_text',
+          text: repoName,
+        },
+        //sort the options by version number
+        options: options.sort((branchOne, branchTwo) =>
+          branchTwo.text.text
+            .toString()
+            .replace(/\d+/g, (n) => +n + 100000)
+            .localeCompare(branchOne.text.text.toString().replace(/\d+/g, (n) => +n + 100000))
+        ),
+      };
+      repoOptions.push(repoOption);
+    }
   }
   return repoOptions.sort((repoOne, repoTwo) => repoOne.label.text.localeCompare(repoTwo.label.text));
 }

diff --git a/src/services/commandExecutor.ts b/src/services/commandExecutor.ts
@@ -65,7 +65,7 @@ export class ShellCommandExecutor implements ICommandExecutor {
       resp.status = CommandExecutorResponseStatus.success;
       return resp;
     } catch (error) {
-      resp.output = '';
+      resp.output = (error.stdout || '').trim();
       resp.error = error;
       resp.status = CommandExecutorResponseStatus.failed;
     }

diff --git a/tests/unit/services/shellCommandExecutor.test.ts b/tests/unit/services/shellCommandExecutor.test.ts
@@ -1,6 +1,5 @@
 import { ShellCommandExecutor } from '../../../src/services/commandExecutor';
 import cp from 'child_process';
-jest.mock('child_process');
 
 describe('ShellCommandExecutor Tests', () => {
   let commandExecutor: ShellCommandExecutor;
@@ -18,15 +17,26 @@ describe('ShellCommandExecutor Tests', () => {
   });
 
   describe('ShellCommandExecutor Tests', () => {
+    test('ShellCommandExecutor returns correct output on success', async () => {
+      const command = ["echo 'stdout output'", "echo 'stderr output' >&2"];
+      const resp = await commandExecutor.execute(command);
+
+      expect(resp.error.toString()).toStrictEqual('stderr output\n');
+      expect(resp.output).toBe('stdout output');
+      expect(resp.status).toBe('success');
+    });
+
     test('ShellCommandExecutor properly throws on system level error', async () => {
-      cp.exec.mockImplementation((command, options, callback) => {
-        callback(Error('Test error'), { stdErr: 'invalid command', stdout: 'test_repo_project_snooty_name' });
-      });
-      const resp = await commandExecutor.execute([]);
-      expect(resp.error).not.toBe(undefined);
-      expect(resp.output).toBe('');
+      const command = ["echo 'stdout output'", "echo 'stderr output' >&2", 'exit 1'];
+      const resp = await commandExecutor.execute(command);
+
+      // This is a strange interface, but I'm just here to fix a bug, not change the interface.
+      // The type of resp.error can be either a string or an Error instance.
+      expect(resp.error.toString()).toStrictEqual(
+        "Error: Command failed: echo 'stdout output' && echo 'stderr output' >&2 && exit 1\nstderr output\n"
+      );
+      expect(resp.output).toBe('stdout output');
       expect(resp.status).toBe('failed');
-      expect(cp.exec).toBeCalledTimes(1);
     });
   });
 });