From 1d300c257760d5f09a7f12762f11f80f6b93d40e Mon Sep 17 00:00:00 2001
From: Peter Clement <PClmnt@users.noreply.github.com>
Date: Wed, 8 May 2024 14:08:34 +0100
Subject: [PATCH] Remove aws sdk global mock and update tests (#13637)

* Remove aws sdk global mock and update tests

* add awaits

* Minio healthcheck in tests.

* Bind to 127.0.0.1 instead of 0.0.0.0

* Fix port fetching for minio container.

* Actually fix port mapping this time.

* Pull minio container before running tests.

* Enable testcontainers debug logging.

* Promote minio container to always running in tests, like CouchDB.

* Remove testcontainers debug logging.

---------

Co-authored-by: Sam Rose <hello@samwho.dev>
---
 .github/workflows/budibase_ci.yml             |  3 +-
 globalSetup.ts                                | 17 ++++++++--
 .../src/objectStore/objectStore.ts            |  2 +-
 .../tests/core/utilities/index.ts             |  3 --
 .../tests/core/utilities/minio.ts             | 34 -------------------
 .../core/utilities/testContainerUtils.ts      |  8 +++++
 .../src/api/routes/tests/attachment.spec.ts   |  7 ++--
 .../src/api/routes/tests/backup.spec.ts       |  6 ++--
 .../server/src/api/routes/tests/row.spec.ts   |  4 +--
 .../src/api/routes/tests/static.spec.js       | 10 ++++++
 .../integrations/tests/aws-sdk.mock.ts}       |  7 +---
 .../src/integrations/tests/dynamodb.spec.ts   |  3 +-
 .../src/integrations/tests/rest.spec.ts       | 10 ------
 .../server/src/integrations/tests/s3.spec.ts  |  3 +-
 packages/server/src/tests/jestEnv.ts          |  3 ++
 .../tests/outputProcessing.spec.ts            |  8 ++---
 .../api/routes/global/tests/realEmail.spec.ts |  3 --
 17 files changed, 57 insertions(+), 74 deletions(-)
 delete mode 100644 packages/backend-core/tests/core/utilities/minio.ts
 rename packages/server/{__mocks__/aws-sdk.ts => src/integrations/tests/aws-sdk.mock.ts} (86%)

diff --git a/.github/workflows/budibase_ci.yml b/.github/workflows/budibase_ci.yml
index 7d09451614..7f1e08601a 100644
--- a/.github/workflows/budibase_ci.yml
+++ b/.github/workflows/budibase_ci.yml
@@ -170,7 +170,8 @@ jobs:
           docker pull mongo:7.0-jammy &
           docker pull mariadb:lts &
           docker pull testcontainers/ryuk:0.5.1 &
-          docker pull budibase/couchdb:v3.2.1-sql &
+          docker pull budibase/couchdb:v3.2.1-sqs &
+          docker pull minio/minio &
           docker pull redis &
 
           wait $(jobs -p)
diff --git a/globalSetup.ts b/globalSetup.ts
index dd1a7dbaa0..dd1454b6e1 100644
--- a/globalSetup.ts
+++ b/globalSetup.ts
@@ -46,7 +46,7 @@ export default async function setup() {
   await killContainers(containers)
 
   try {
-    let couchdb = new GenericContainer("budibase/couchdb:v3.2.1-sqs")
+    const couchdb = new GenericContainer("budibase/couchdb:v3.2.1-sqs")
       .withExposedPorts(5984, 4984)
       .withEnvironment({
         COUCHDB_PASSWORD: "budibase",
@@ -69,7 +69,20 @@ export default async function setup() {
         ).withStartupTimeout(20000)
       )
 
-    await couchdb.start()
+    const minio = new GenericContainer("minio/minio")
+      .withExposedPorts(9000)
+      .withCommand(["server", "/data"])
+      .withEnvironment({
+        MINIO_ACCESS_KEY: "budibase",
+        MINIO_SECRET_KEY: "budibase",
+      })
+      .withLabels({ "com.budibase": "true" })
+      .withReuse()
+      .withWaitStrategy(
+        Wait.forHttp("/minio/health/ready", 9000).withStartupTimeout(10000)
+      )
+
+    await Promise.all([couchdb.start(), minio.start()])
   } finally {
     lockfile.unlockSync(lockPath)
   }
diff --git a/packages/backend-core/src/objectStore/objectStore.ts b/packages/backend-core/src/objectStore/objectStore.ts
index aa5365c5c3..2bef91ffef 100644
--- a/packages/backend-core/src/objectStore/objectStore.ts
+++ b/packages/backend-core/src/objectStore/objectStore.ts
@@ -83,7 +83,7 @@ export function ObjectStore(
   bucket: string,
   opts: { presigning: boolean } = { presigning: false }
 ) {
-  const config: any = {
+  const config: AWS.S3.ClientConfiguration = {
     s3ForcePathStyle: true,
     signatureVersion: "v4",
     apiVersion: "2006-03-01",
diff --git a/packages/backend-core/tests/core/utilities/index.ts b/packages/backend-core/tests/core/utilities/index.ts
index b2f19a0286..787d69be2c 100644
--- a/packages/backend-core/tests/core/utilities/index.ts
+++ b/packages/backend-core/tests/core/utilities/index.ts
@@ -4,6 +4,3 @@ export { generator } from "./structures"
 export * as testContainerUtils from "./testContainerUtils"
 export * as utils from "./utils"
 export * from "./jestUtils"
-import * as minio from "./minio"
-
-export const objectStoreTestProviders = { minio }
diff --git a/packages/backend-core/tests/core/utilities/minio.ts b/packages/backend-core/tests/core/utilities/minio.ts
deleted file mode 100644
index cef33daa91..0000000000
--- a/packages/backend-core/tests/core/utilities/minio.ts
+++ /dev/null
@@ -1,34 +0,0 @@
-import { GenericContainer, Wait, StartedTestContainer } from "testcontainers"
-import { AbstractWaitStrategy } from "testcontainers/build/wait-strategies/wait-strategy"
-import env from "../../../src/environment"
-
-let container: StartedTestContainer | undefined
-
-class ObjectStoreWaitStrategy extends AbstractWaitStrategy {
-  async waitUntilReady(container: any, boundPorts: any, startTime?: Date) {
-    const logs = Wait.forListeningPorts()
-    await logs.waitUntilReady(container, boundPorts, startTime)
-  }
-}
-
-export async function start(): Promise<void> {
-  container = await new GenericContainer("minio/minio")
-    .withExposedPorts(9000)
-    .withCommand(["server", "/data"])
-    .withEnvironment({
-      MINIO_ACCESS_KEY: "budibase",
-      MINIO_SECRET_KEY: "budibase",
-    })
-    .withWaitStrategy(new ObjectStoreWaitStrategy().withStartupTimeout(30000))
-    .start()
-
-  const port = container.getMappedPort(9000)
-  env._set("MINIO_URL", `http://0.0.0.0:${port}`)
-}
-
-export async function stop() {
-  if (container) {
-    await container.stop()
-    container = undefined
-  }
-}
diff --git a/packages/backend-core/tests/core/utilities/testContainerUtils.ts b/packages/backend-core/tests/core/utilities/testContainerUtils.ts
index 32841e4c3a..1a25bb28f4 100644
--- a/packages/backend-core/tests/core/utilities/testContainerUtils.ts
+++ b/packages/backend-core/tests/core/utilities/testContainerUtils.ts
@@ -86,10 +86,18 @@ export function setupEnv(...envs: any[]) {
     throw new Error("CouchDB SQL port not found")
   }
 
+  const minio = getContainerByImage("minio/minio")
+
+  const minioPort = getExposedV4Port(minio, 9000)
+  if (!minioPort) {
+    throw new Error("Minio port not found")
+  }
+
   const configs = [
     { key: "COUCH_DB_PORT", value: `${couchPort}` },
     { key: "COUCH_DB_URL", value: `http://127.0.0.1:${couchPort}` },
     { key: "COUCH_DB_SQL_URL", value: `http://127.0.0.1:${couchSqlPort}` },
+    { key: "MINIO_URL", value: `http://127.0.0.1:${minioPort}` },
   ]
 
   for (const config of configs.filter(x => !!x.value)) {
diff --git a/packages/server/src/api/routes/tests/attachment.spec.ts b/packages/server/src/api/routes/tests/attachment.spec.ts
index aa02ea898e..7ff4e67bcc 100644
--- a/packages/server/src/api/routes/tests/attachment.spec.ts
+++ b/packages/server/src/api/routes/tests/attachment.spec.ts
@@ -4,10 +4,12 @@ import { APIError } from "@budibase/types"
 describe("/api/applications/:appId/sync", () => {
   let config = setup.getConfig()
 
-  afterAll(setup.afterAll)
   beforeAll(async () => {
     await config.init()
   })
+  afterAll(async () => {
+    setup.afterAll()
+  })
 
   describe("/api/attachments/process", () => {
     it("should accept an image file upload", async () => {
@@ -18,7 +20,8 @@ describe("/api/applications/:appId/sync", () => {
       expect(resp.length).toBe(1)
 
       let upload = resp[0]
-      expect(upload.url.endsWith(".jpg")).toBe(true)
+
+      expect(upload.url.split("?")[0].endsWith(".jpg")).toBe(true)
       expect(upload.extension).toBe("jpg")
       expect(upload.size).toBe(1)
       expect(upload.name).toBe("1px.jpg")
diff --git a/packages/server/src/api/routes/tests/backup.spec.ts b/packages/server/src/api/routes/tests/backup.spec.ts
index 7b56145f8e..78354dfe9c 100644
--- a/packages/server/src/api/routes/tests/backup.spec.ts
+++ b/packages/server/src/api/routes/tests/backup.spec.ts
@@ -1,16 +1,18 @@
+import { mocks } from "@budibase/backend-core/tests"
 import tk from "timekeeper"
 import * as setup from "./utilities"
 import { events } from "@budibase/backend-core"
 import sdk from "../../../sdk"
 import { checkBuilderEndpoint } from "./utilities/TestFunctions"
-import { mocks } from "@budibase/backend-core/tests"
 
 mocks.licenses.useBackups()
 
 describe("/backups", () => {
   let config = setup.getConfig()
 
-  afterAll(setup.afterAll)
+  afterAll(async () => {
+    setup.afterAll()
+  })
 
   beforeEach(async () => {
     tk.reset()
diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts
index 5e30c1b4cc..fd3158a11c 100644
--- a/packages/server/src/api/routes/tests/row.spec.ts
+++ b/packages/server/src/api/routes/tests/row.spec.ts
@@ -856,7 +856,7 @@ describe.each([
         await config.withEnv({ SELF_HOSTED: "true" }, async () => {
           return context.doInAppContext(config.getAppId(), async () => {
             const enriched = await outputProcessing(table, [row])
-            expect((enriched as Row[])[0].attachment.url).toBe(
+            expect((enriched as Row[])[0].attachment.url.split("?")[0]).toBe(
               `/files/signed/prod-budi-app-assets/${config.getProdAppId()}/attachments/${attachmentId}`
             )
           })
@@ -889,7 +889,7 @@ describe.each([
         await config.withEnv({ SELF_HOSTED: "true" }, async () => {
           return context.doInAppContext(config.getAppId(), async () => {
             const enriched = await outputProcessing(table, [row])
-            expect((enriched as Row[])[0].attachment[0].url).toBe(
+            expect((enriched as Row[])[0].attachment[0].url.split("?")[0]).toBe(
               `/files/signed/prod-budi-app-assets/${config.getProdAppId()}/attachments/${attachmentId}`
             )
           })
diff --git a/packages/server/src/api/routes/tests/static.spec.js b/packages/server/src/api/routes/tests/static.spec.js
index 30417c3a98..1358b5418a 100644
--- a/packages/server/src/api/routes/tests/static.spec.js
+++ b/packages/server/src/api/routes/tests/static.spec.js
@@ -1,3 +1,13 @@
+// Directly mock the AWS SDK
+jest.mock("aws-sdk", () => ({
+  S3: jest.fn(() => ({
+    getSignedUrl: jest.fn(
+      (operation, params) => `http://example.com/${params.Bucket}/${params.Key}`
+    ),
+    upload: jest.fn(() => ({ Contents: {} })),
+  })),
+}))
+
 const setup = require("./utilities")
 const { constants } = require("@budibase/backend-core")
 
diff --git a/packages/server/__mocks__/aws-sdk.ts b/packages/server/src/integrations/tests/aws-sdk.mock.ts
similarity index 86%
rename from packages/server/__mocks__/aws-sdk.ts
rename to packages/server/src/integrations/tests/aws-sdk.mock.ts
index d6d33f6c46..0422adfd3c 100644
--- a/packages/server/__mocks__/aws-sdk.ts
+++ b/packages/server/src/integrations/tests/aws-sdk.mock.ts
@@ -1,6 +1,3 @@
-import fs from "fs"
-import { join } from "path"
-
 const response = (body: any, extra?: any) => () => ({
   promise: () => body,
   ...extra,
@@ -62,9 +59,7 @@ class S3 {
         Body: "",
       },
       {
-        createReadStream: jest
-          .fn()
-          .mockReturnValue(fs.createReadStream(join(__dirname, "aws-sdk.ts"))),
+        createReadStream: jest.fn().mockReturnValue("stream"),
       }
     )
   )
diff --git a/packages/server/src/integrations/tests/dynamodb.spec.ts b/packages/server/src/integrations/tests/dynamodb.spec.ts
index 0215817907..c992bc8bfd 100644
--- a/packages/server/src/integrations/tests/dynamodb.spec.ts
+++ b/packages/server/src/integrations/tests/dynamodb.spec.ts
@@ -1,7 +1,6 @@
+jest.mock("aws-sdk", () => require("./aws-sdk.mock"))
 import { default as DynamoDBIntegration } from "../dynamodb"
 
-jest.mock("aws-sdk")
-
 class TestConfiguration {
   integration: any
 
diff --git a/packages/server/src/integrations/tests/rest.spec.ts b/packages/server/src/integrations/tests/rest.spec.ts
index 4b20017939..9877a826ec 100644
--- a/packages/server/src/integrations/tests/rest.spec.ts
+++ b/packages/server/src/integrations/tests/rest.spec.ts
@@ -28,7 +28,6 @@ jest.mock("uuid", () => ({ v4: () => "00000000-0000-0000-0000-000000000000" }))
 import { default as RestIntegration } from "../rest"
 import { RestAuthType } from "@budibase/types"
 import fetch from "node-fetch"
-import { objectStoreTestProviders } from "@budibase/backend-core/tests"
 import { Readable } from "stream"
 
 const FormData = require("form-data")
@@ -627,15 +626,6 @@ describe("REST Integration", () => {
   })
 
   describe("File Handling", () => {
-    beforeAll(async () => {
-      jest.unmock("aws-sdk")
-      await objectStoreTestProviders.minio.start()
-    })
-
-    afterAll(async () => {
-      await objectStoreTestProviders.minio.stop()
-    })
-
     it("uploads file to object store and returns signed URL", async () => {
       const responseData = Buffer.from("teest file contnt")
       const filename = "test.tar.gz"
diff --git a/packages/server/src/integrations/tests/s3.spec.ts b/packages/server/src/integrations/tests/s3.spec.ts
index b75340ab20..abe8fb9cf1 100644
--- a/packages/server/src/integrations/tests/s3.spec.ts
+++ b/packages/server/src/integrations/tests/s3.spec.ts
@@ -1,7 +1,6 @@
+jest.mock("aws-sdk", () => require("./aws-sdk.mock"))
 import { default as S3Integration } from "../s3"
 
-jest.mock("aws-sdk")
-
 class TestConfiguration {
   integration: any
 
diff --git a/packages/server/src/tests/jestEnv.ts b/packages/server/src/tests/jestEnv.ts
index 3b0c2a88d9..a44ce58f81 100644
--- a/packages/server/src/tests/jestEnv.ts
+++ b/packages/server/src/tests/jestEnv.ts
@@ -14,3 +14,6 @@ process.env.WORKER_URL = "http://localhost:10000"
 process.env.COUCH_DB_PASSWORD = "budibase"
 process.env.COUCH_DB_USER = "budibase"
 process.env.JWT_SECRET = "jwtsecret"
+process.env.MINIO_URL = "http://localhost"
+process.env.MINIO_ACCESS_KEY = "budibase"
+process.env.MINIO_SECRET_KEY = "budibase"
diff --git a/packages/server/src/utilities/rowProcessor/tests/outputProcessing.spec.ts b/packages/server/src/utilities/rowProcessor/tests/outputProcessing.spec.ts
index b1a20acba5..c22fd1c69d 100644
--- a/packages/server/src/utilities/rowProcessor/tests/outputProcessing.spec.ts
+++ b/packages/server/src/utilities/rowProcessor/tests/outputProcessing.spec.ts
@@ -100,13 +100,13 @@ describe("rowProcessor - outputProcessing", () => {
     }
 
     const output = await outputProcessing(table, row, { squash: false })
-    expect(output.attach[0].url).toBe(
+    expect(output.attach[0].url?.split("?")[0]).toBe(
       "/files/signed/prod-budi-app-assets/test.jpg"
     )
 
     row.attach[0].url = ""
     const output2 = await outputProcessing(table, row, { squash: false })
-    expect(output2.attach[0].url).toBe(
+    expect(output2.attach[0].url?.split("?")[0]).toBe(
       "/files/signed/prod-budi-app-assets/test.jpg"
     )
 
@@ -141,13 +141,13 @@ describe("rowProcessor - outputProcessing", () => {
     }
 
     const output = await outputProcessing(table, row, { squash: false })
-    expect(output.attach.url).toBe(
+    expect(output.attach.url?.split("?")[0]).toBe(
       "/files/signed/prod-budi-app-assets/test.jpg"
     )
 
     row.attach.url = ""
     const output2 = await outputProcessing(table, row, { squash: false })
-    expect(output2.attach.url).toBe(
+    expect(output2.attach?.url?.split("?")[0]).toBe(
       "/files/signed/prod-budi-app-assets/test.jpg"
     )
 
diff --git a/packages/worker/src/api/routes/global/tests/realEmail.spec.ts b/packages/worker/src/api/routes/global/tests/realEmail.spec.ts
index 8880e587c5..bda5f6334f 100644
--- a/packages/worker/src/api/routes/global/tests/realEmail.spec.ts
+++ b/packages/worker/src/api/routes/global/tests/realEmail.spec.ts
@@ -2,7 +2,6 @@ jest.unmock("node-fetch")
 jest.unmock("aws-sdk")
 import { TestConfiguration } from "../../../../tests"
 import { EmailTemplatePurpose } from "../../../../constants"
-import { objectStoreTestProviders } from "@budibase/backend-core/tests"
 import { objectStore } from "@budibase/backend-core"
 import tk from "timekeeper"
 import { EmailAttachment } from "@budibase/types"
@@ -19,12 +18,10 @@ describe("/api/global/email", () => {
 
   beforeAll(async () => {
     tk.reset()
-    await objectStoreTestProviders.minio.start()
     await config.beforeAll()
   })
 
   afterAll(async () => {
-    await objectStoreTestProviders.minio.stop()
     await config.afterAll()
   })