Merge pull request #12877 from Budibase/remove_test_flakiness

Refactor test helpers

packages/server/src/api/controllers/query/import/tests/index.spec.js

@@ -42,7 +42,7 @@ const datasets = {
 }
 
 describe("Rest Importer", () => {
-  const config = new TestConfig(false)
+  const config = new TestConfig()
 
   beforeAll(async () => {
     await config.init()

packages/server/src/api/routes/public/tests/compare.spec.ts

@@ -12,7 +12,7 @@ let apiKey: string, table: Table, app: App, makeRequest: any
 
 beforeAll(async () => {
   app = await config.init()
-  table = await config.updateTable()
+  table = await config.upsertTable()
   apiKey = await config.generateApiKey()
   makeRequest = generateMakeRequest(apiKey)
 })
@@ -69,7 +69,7 @@ describe("check the applications endpoints", () => {
 describe("check the tables endpoints", () => {
   it("should allow retrieving tables through search", async () => {
     await config.createApp("new app 1")
-    table = await config.updateTable()
+    table = await config.upsertTable()
     const res = await makeRequest("post", "/tables/search")
     expect(res).toSatisfyApiSpec()
   })
@@ -108,7 +108,7 @@ describe("check the tables endpoints", () => {
 describe("check the rows endpoints", () => {
   let row: Row
   it("should allow retrieving rows through search", async () => {
-    table = await config.updateTable()
+    table = await config.upsertTable()
     const res = await makeRequest("post", `/tables/${table._id}/rows/search`, {
       query: {},
     })

packages/server/src/api/routes/tests/query.seq.spec.ts

@@ -1,5 +1,4 @@
-const tk = require("timekeeper")
+import tk from "timekeeper"
-tk.freeze(Date.now())
 
 // Mock out postgres for this
 jest.mock("pg")
@@ -17,16 +16,24 @@ jest.mock("@budibase/backend-core", () => {
     },
   }
 })
-const setup = require("./utilities")
+import * as setup from "./utilities"
-const { checkBuilderEndpoint } = require("./utilities/TestFunctions")
+import { checkBuilderEndpoint } from "./utilities/TestFunctions"
-const { checkCacheForDynamicVariable } = require("../../../threads/utils")
+import { checkCacheForDynamicVariable } from "../../../threads/utils"
+
 const { basicQuery, basicDatasource } = setup.structures
-const { events, db: dbCore } = require("@budibase/backend-core")
+import { events, db as dbCore } from "@budibase/backend-core"
+import { Datasource, Query, SourceName } from "@budibase/types"
+
+tk.freeze(Date.now())
+
+const mockIsProdAppID = dbCore.isProdAppID as jest.MockedFunction<
+  typeof dbCore.isProdAppID
+>
 
 describe("/queries", () => {
   let request = setup.getRequest()
   let config = setup.getConfig()
-  let datasource, query
+  let datasource: Datasource & Required<Pick<Datasource, "_id">>, query: Query
 
   afterAll(setup.afterAll)
 
@@ -40,18 +47,7 @@ describe("/queries", () => {
     await setupTest()
   })
 
-  async function createInvalidIntegration() {
+  const createQuery = async (query: Query) => {
-    const datasource = await config.createDatasource({
-      datasource: {
-        ...basicDatasource().datasource,
-        source: "INVALID_INTEGRATION",
-      },
-    })
-    const query = await config.createQuery()
-    return { datasource, query }
-  }
-
-  const createQuery = async query => {
     return request
       .post(`/api/queries`)
       .send(query)
@@ -67,7 +63,7 @@ describe("/queries", () => {
       jest.clearAllMocks()
       const res = await createQuery(query)
 
-      expect(res.res.statusMessage).toEqual(
+      expect((res as any).res.statusMessage).toEqual(
         `Query ${query.name} saved successfully.`
       )
       expect(res.body).toEqual({
@@ -92,7 +88,7 @@ describe("/queries", () => {
       query._rev = res.body._rev
       await createQuery(query)
 
-      expect(res.res.statusMessage).toEqual(
+      expect((res as any).res.statusMessage).toEqual(
         `Query ${query.name} saved successfully.`
       )
       expect(res.body).toEqual({
@@ -168,8 +164,8 @@ describe("/queries", () => {
 
     it("should remove sensitive info for prod apps", async () => {
       // Mock isProdAppID to pretend we are using a prod app
-      dbCore.isProdAppID.mockClear()
+      mockIsProdAppID.mockClear()
-      dbCore.isProdAppID.mockImplementation(() => true)
+      mockIsProdAppID.mockImplementation(() => true)
 
       const query = await config.createQuery()
       const res = await request
@@ -184,7 +180,7 @@ describe("/queries", () => {
 
       // Reset isProdAppID mock
       expect(dbCore.isProdAppID).toHaveBeenCalledTimes(1)
-      dbCore.isProdAppID.mockImplementation(() => false)
+      mockIsProdAppID.mockImplementation(() => false)
     })
   })
 
@@ -211,10 +207,11 @@ describe("/queries", () => {
     })
 
     it("should apply authorization to endpoint", async () => {
+      const query = await config.createQuery()
       await checkBuilderEndpoint({
         config,
         method: "DELETE",
-        url: `/api/queries/${config._id}/${config._rev}`,
+        url: `/api/queries/${query._id}/${query._rev}`,
       })
     })
   })
@@ -272,20 +269,21 @@ describe("/queries", () => {
     })
 
     it("should fail with invalid integration type", async () => {
-      let error
+      const response = await config.api.datasource.create(
-      try {
+        {
-        await createInvalidIntegration()
+          ...basicDatasource().datasource,
-      } catch (err) {
+          source: "INVALID_INTEGRATION" as SourceName,
-        error = err
+        },
-      }
+        { expectStatus: 500, rawResponse: true }
-      expect(error).toBeDefined()
+      )
-      expect(error.message).toBe("No datasource implementation found.")
+
+      expect(response.body.message).toBe("No datasource implementation found.")
     })
   })
 
   describe("variables", () => {
-    async function preview(datasource, fields) {
+    async function preview(datasource: Datasource, fields: any) {
-      return config.previewQuery(request, config, datasource, fields)
+      return config.previewQuery(request, config, datasource, fields, undefined)
     }
 
     it("should work with static variables", async () => {
@@ -370,11 +368,19 @@ describe("/queries", () => {
   })
 
   describe("Current User Request Mapping", () => {
-    async function previewGet(datasource, fields, params) {
+    async function previewGet(
+      datasource: Datasource,
+      fields: any,
+      params: any
+    ) {
       return config.previewQuery(request, config, datasource, fields, params)
     }
 
-    async function previewPost(datasource, fields, params) {
+    async function previewPost(
+      datasource: Datasource,
+      fields: any,
+      params: any
+    ) {
       return config.previewQuery(
         request,
         config,
@@ -394,14 +400,18 @@ describe("/queries", () => {
           emailHdr: "{{[user].[email]}}",
         },
       })
-      const res = await previewGet(datasource, {
+      const res = await previewGet(
-        path: "www.google.com",
+        datasource,
-        queryString: "email={{[user].[email]}}",
+        {
-        headers: {
+          path: "www.google.com",
-          queryHdr: "{{[user].[firstName]}}",
+          queryString: "email={{[user].[email]}}",
-          secondHdr: "1234",
+          headers: {
+            queryHdr: "{{[user].[firstName]}}",
+            secondHdr: "1234",
+          },
         },
-      })
+        undefined
+      )
 
       const parsedRequest = JSON.parse(res.body.extra.raw)
       expect(parsedRequest.opts.headers).toEqual({

packages/server/src/api/routes/tests/row.spec.ts

@@ -581,7 +581,7 @@ describe.each([
         tableId: InternalTable.USER_METADATA,
       }
 
-      let table = await config.api.table.create({
+      let table = await config.api.table.save({
         name: "TestTable",
         type: "table",
         sourceType: TableSourceType.INTERNAL,
@@ -1690,7 +1690,7 @@ describe.each([
           tableConfig.sourceType = TableSourceType.EXTERNAL
         }
       }
-      const table = await config.api.table.create({
+      const table = await config.api.table.save({
         ...tableConfig,
         schema: {
           ...tableConfig.schema,

packages/server/src/api/routes/tests/table.spec.ts

@@ -438,7 +438,7 @@ describe("/tables", () => {
     })
 
     it("should successfully migrate a one-to-many user relationship to a user column", async () => {
-      const table = await config.api.table.create({
+      const table = await config.api.table.save({
         name: "table",
         type: "table",
         sourceId: INTERNAL_TABLE_SOURCE_ID,
@@ -496,7 +496,7 @@ describe("/tables", () => {
       // We found a bug just after releasing this feature where if the row was created from the
       // users table, not the table linking to it, the migration would succeed but lose the data.
       // This happened because the order of the documents in the link was reversed.
-      const table = await config.api.table.create({
+      const table = await config.api.table.save({
         name: "table",
         type: "table",
         sourceId: INTERNAL_TABLE_SOURCE_ID,
@@ -554,7 +554,7 @@ describe("/tables", () => {
     })
 
     it("should successfully migrate a many-to-many user relationship to a users column", async () => {
-      const table = await config.api.table.create({
+      const table = await config.api.table.save({
         name: "table",
         type: "table",
         sourceId: INTERNAL_TABLE_SOURCE_ID,
@@ -611,7 +611,7 @@ describe("/tables", () => {
     })
 
     it("should successfully migrate a many-to-one user relationship to a users column", async () => {
-      const table = await config.api.table.create({
+      const table = await config.api.table.save({
         name: "table",
         type: "table",
         sourceId: INTERNAL_TABLE_SOURCE_ID,
@@ -670,7 +670,7 @@ describe("/tables", () => {
     describe("unhappy paths", () => {
       let table: Table
       beforeAll(async () => {
-        table = await config.api.table.create({
+        table = await config.api.table.save({
           name: "table",
           type: "table",
           sourceId: INTERNAL_TABLE_SOURCE_ID,

packages/server/src/automations/tests/updateRow.spec.ts

@@ -67,7 +67,7 @@ describe("test the update row action", () => {
       tableId: InternalTable.USER_METADATA,
     }
 
-    let table = await config.api.table.create({
+    let table = await config.api.table.save({
       name: uuid.v4(),
       type: "table",
       sourceType: TableSourceType.INTERNAL,
@@ -120,7 +120,7 @@ describe("test the update row action", () => {
       tableId: InternalTable.USER_METADATA,
     }
 
-    let table = await config.api.table.create({
+    let table = await config.api.table.save({
       name: uuid.v4(),
       type: "table",
       sourceType: TableSourceType.INTERNAL,

packages/server/src/db/tests/linkController.spec.ts

@@ -324,7 +324,7 @@ describe("test the link controller", () => {
       name: "link",
       autocolumn: true,
     }
-    await config.updateTable(table)
+    await config.upsertTable(table)
   })
 
   it("should be able to remove a linked field from a table, even if the linked table does not exist", async () => {

packages/server/src/migrations/functions/usageQuotas/tests/syncRows.spec.ts

@@ -5,7 +5,7 @@ import { QuotaUsageType, StaticQuotaName } from "@budibase/types"
 import { db as dbCore, context } from "@budibase/backend-core"
 
 describe("syncRows", () => {
-  let config = new TestConfig(false)
+  const config = new TestConfig()
 
   beforeEach(async () => {
     await config.init()

packages/server/src/sdk/app/rows/tests/internal.spec.ts

@@ -8,10 +8,10 @@ import {
   FieldType,
   Table,
   AutoFieldSubType,
+  AutoColumnFieldMetadata,
 } from "@budibase/types"
 
 import TestConfiguration from "../../../../tests/utilities/TestConfiguration"
-import { cache } from "@budibase/backend-core"
 
 tk.freeze(Date.now())
 
@@ -213,8 +213,10 @@ describe("sdk >> rows >> internal", () => {
       )
 
       const persistedTable = await config.getTable(table._id)
-      expect((table as any).schema.id.lastID).toBe(0)
+      expect((table.schema.id as AutoColumnFieldMetadata).lastID).toBe(0)
-      expect(persistedTable.schema.id.lastID).toBe(20)
+      expect((persistedTable.schema.id as AutoColumnFieldMetadata).lastID).toBe(
+        20
+      )
     })
   })
 })

packages/server/src/sdk/tests/tables.spec.ts

@@ -9,7 +9,7 @@ describe("tables", () => {
 
   beforeAll(async () => {
     await config.init()
-    table = await config.api.table.create(basicTable())
+    table = await config.api.table.save(basicTable())
   })
 
   describe("getTables", () => {

packages/server/src/tests/utilities/TestConfiguration.ts

@@ -27,7 +27,18 @@ import {
   sessions,
   tenancy,
 } from "@budibase/backend-core"
-import * as controllers from "./controllers"
+import {
+  app as appController,
+  deploy as deployController,
+  role as roleController,
+  automation as automationController,
+  webhook as webhookController,
+  query as queryController,
+  screen as screenController,
+  layout as layoutController,
+  view as viewController,
+} from "./controllers"
+
 import { cleanup } from "../../utilities/fileSystem"
 import newid from "../../db/newid"
 import { generateUserMetadataID } from "../../db/utils"
@@ -44,13 +55,14 @@ import {
   RelationshipFieldMetadata,
   RelationshipType,
   Row,
-  SearchFilters,
+  SearchParams,
   SourceName,
   Table,
   TableSourceType,
   User,
   UserRoles,
   View,
+  WithRequired,
 } from "@budibase/types"
 
 import API from "./api"
@@ -543,11 +555,7 @@ class TestConfiguration {
     // clear any old app
     this.appId = null
     this.app = await context.doInTenant(this.tenantId!, async () => {
-      const app = await this._req(
+      const app = await this._req({ name: appName }, null, appController.create)
-        { name: appName },
-        null,
-        controllers.app.create
-      )
       this.appId = app.appId!
       return app
     })
@@ -563,7 +571,7 @@ class TestConfiguration {
   }
 
   async publish() {
-    await this._req(null, null, controllers.deploy.publishApp)
+    await this._req(null, null, deployController.publishApp)
     // @ts-ignore
     const prodAppId = this.getAppId().replace("_dev", "")
     this.prodAppId = prodAppId
@@ -578,7 +586,7 @@ class TestConfiguration {
     const response = await this._req(
       null,
       { appId: this.appId },
-      controllers.app.unpublish
+      appController.unpublish
     )
     this.prodAppId = null
     this.prodApp = null
@@ -587,14 +595,16 @@ class TestConfiguration {
 
   // TABLE
 
-  async updateTable(
+  async upsertTable(
     config?: TableToBuild,
     { skipReassigning } = { skipReassigning: false }
   ): Promise<Table> {
     config = config || basicTable()
-    config.sourceType = config.sourceType || TableSourceType.INTERNAL
+    const response = await this.api.table.save({
-    config.sourceId = config.sourceId || INTERNAL_TABLE_SOURCE_ID
+      ...config,
-    const response = await this._req(config, null, controllers.table.save)
+      sourceType: config.sourceType || TableSourceType.INTERNAL,
+      sourceId: config.sourceId || INTERNAL_TABLE_SOURCE_ID,
+    })
     if (!skipReassigning) {
       this.table = response
     }
@@ -612,7 +622,7 @@ class TestConfiguration {
     if (!config.sourceId) {
       config.sourceId = INTERNAL_TABLE_SOURCE_ID
     }
-    return this.updateTable(config, options)
+    return this.upsertTable(config, options)
   }
 
   async createExternalTable(
@@ -627,12 +637,12 @@ class TestConfiguration {
       config.sourceId = this.datasource._id
       config.sourceType = TableSourceType.EXTERNAL
     }
-    return this.updateTable(config, options)
+    return this.upsertTable(config, options)
   }
 
   async getTable(tableId?: string) {
     tableId = tableId || this.table!._id!
-    return this._req(null, { tableId }, controllers.table.find)
+    return this.api.table.get(tableId)
   }
 
   async createLinkedTable(
@@ -680,37 +690,35 @@ class TestConfiguration {
     if (!this.table) {
       throw "Test requires table to be configured."
     }
-    const tableId = (config && config.tableId) || this.table._id
+    const tableId = (config && config.tableId) || this.table._id!
     config = config || basicRow(tableId!)
-    return this._req(config, { tableId }, controllers.row.save)
+    return this.api.row.save(tableId, config)
   }
 
   async getRow(tableId: string, rowId: string): Promise<Row> {
-    return this._req(null, { tableId, rowId }, controllers.row.find)
+    const res = await this.api.row.get(tableId, rowId)
+    return res.body
   }
 
   async getRows(tableId: string) {
     if (!tableId && this.table) {
       tableId = this.table._id!
     }
-    return this._req(null, { tableId }, controllers.row.fetch)
+    return this.api.row.fetch(tableId)
   }
 
-  async searchRows(tableId: string, searchParams: SearchFilters = {}) {
+  async searchRows(tableId: string, searchParams?: SearchParams) {
     if (!tableId && this.table) {
       tableId = this.table._id!
     }
-    const body = {
+    return this.api.row.search(tableId, searchParams)
-      query: searchParams,
-    }
-    return this._req(body, { tableId }, controllers.row.search)
   }
 
   // ROLE
 
   async createRole(config?: any) {
     config = config || basicRole()
-    return this._req(config, null, controllers.role.save)
+    return this._req(config, null, roleController.save)
   }
 
   // VIEW
@@ -723,7 +731,7 @@ class TestConfiguration {
       tableId: this.table!._id,
       name: generator.guid(),
     }
-    return this._req(view, null, controllers.view.v1.save)
+    return this._req(view, null, viewController.v1.save)
   }
 
   async createView(
@@ -753,13 +761,13 @@ class TestConfiguration {
       delete config._rev
     }
     this.automation = (
-      await this._req(config, null, controllers.automation.create)
+      await this._req(config, null, automationController.create)
     ).automation
     return this.automation
   }
 
   async getAllAutomations() {
-    return this._req(null, null, controllers.automation.fetch)
+    return this._req(null, null, automationController.fetch)
   }
 
   async deleteAutomation(automation?: any) {
@@ -770,7 +778,7 @@ class TestConfiguration {
     return this._req(
       null,
       { id: automation._id, rev: automation._rev },
-      controllers.automation.destroy
+      automationController.destroy
     )
   }
 
@@ -779,28 +787,27 @@ class TestConfiguration {
       throw "Must create an automation before creating webhook."
     }
     config = config || basicWebhook(this.automation._id)
-    return (await this._req(config, null, controllers.webhook.save)).webhook
+
+    return (await this._req(config, null, webhookController.save)).webhook
   }
 
   // DATASOURCE
 
   async createDatasource(config?: {
     datasource: Datasource
-  }): Promise<Datasource> {
+  }): Promise<WithRequired<Datasource, "_id">> {
     config = config || basicDatasource()
-    const response = await this._req(config, null, controllers.datasource.save)
+    const response = await this.api.datasource.create(config.datasource)
-    this.datasource = response.datasource
+    this.datasource = response
-    return this.datasource!
+    return { ...this.datasource, _id: this.datasource!._id! }
   }
 
-  async updateDatasource(datasource: Datasource): Promise<Datasource> {
+  async updateDatasource(
-    const response = await this._req(
+    datasource: Datasource
-      datasource,
+  ): Promise<WithRequired<Datasource, "_id">> {
-      { datasourceId: datasource._id },
+    const response = await this.api.datasource.update(datasource)
-      controllers.datasource.update
+    this.datasource = response
-    )
+    return { ...this.datasource, _id: this.datasource!._id! }
-    this.datasource = response.datasource
-    return this.datasource!
   }
 
   async restDatasource(cfg?: any) {
@@ -815,6 +822,7 @@ class TestConfiguration {
 
   async dynamicVariableDatasource() {
     let datasource = await this.restDatasource()
+
     const basedOnQuery = await this.createQuery({
       ...basicQuery(datasource._id!),
       fields: {
@@ -886,21 +894,21 @@ class TestConfiguration {
       throw "No datasource created for query."
     }
     config = config || basicQuery(this.datasource!._id!)
-    return this._req(config, null, controllers.query.save)
+    return this._req(config, null, queryController.save)
   }
 
   // SCREEN
 
   async createScreen(config?: any) {
     config = config || basicScreen()
-    return this._req(config, null, controllers.screen.save)
+    return this._req(config, null, screenController.save)
   }
 
   // LAYOUT
 
   async createLayout(config?: any) {
     config = config || basicLayout()
-    return await this._req(config, null, controllers.layout.save)
+    return await this._req(config, null, layoutController.save)
   }
 }
 

packages/server/src/tests/utilities/api/datasource.ts

@@ -2,20 +2,23 @@ import {
   CreateDatasourceRequest,
   Datasource,
   VerifyDatasourceRequest,
-  VerifyDatasourceResponse,
 } from "@budibase/types"
 import TestConfiguration from "../TestConfiguration"
 import { TestAPI } from "./base"
+import supertest from "supertest"
 
 export class DatasourceAPI extends TestAPI {
   constructor(config: TestConfiguration) {
     super(config)
   }
 
-  create = async (
+  create = async <B extends boolean = false>(
     config: Datasource,
-    { expectStatus } = { expectStatus: 200 }
+    {
-  ): Promise<Datasource> => {
+      expectStatus,
+      rawResponse,
+    }: { expectStatus?: number; rawResponse?: B } = {}
+  ): Promise<B extends false ? Datasource : supertest.Response> => {
     const body: CreateDatasourceRequest = {
       datasource: config,
       tablesFilter: [],
@@ -25,8 +28,11 @@ export class DatasourceAPI extends TestAPI {
       .send(body)
       .set(this.config.defaultHeaders())
       .expect("Content-Type", /json/)
-      .expect(expectStatus)
+      .expect(expectStatus || 200)
-    return result.body.datasource as Datasource
+    if (rawResponse) {
+      return result as any
+    }
+    return result.body.datasource
   }
 
   update = async (

packages/server/src/tests/utilities/api/row.ts

@@ -7,6 +7,7 @@ import {
   BulkImportRequest,
   BulkImportResponse,
   SearchRowResponse,
+  SearchParams,
 } from "@budibase/types"
 import TestConfiguration from "../TestConfiguration"
 import { TestAPI } from "./base"
@@ -154,10 +155,12 @@ export class RowAPI extends TestAPI {
 
   search = async (
     sourceId: string,
+    params?: SearchParams,
     { expectStatus } = { expectStatus: 200 }
   ): Promise<SearchRowResponse> => {
     const request = this.request
       .post(`/api/${sourceId}/search`)
+      .send(params)
       .set(this.config.defaultHeaders())
       .expect(expectStatus)
 

packages/server/src/tests/utilities/api/table.ts

@@ -13,7 +13,7 @@ export class TableAPI extends TestAPI {
     super(config)
   }
 
-  create = async (
+  save = async (
     data: SaveTableRequest,
     { expectStatus } = { expectStatus: 200 }
   ): Promise<SaveTableResponse> => {

packages/server/src/tests/utilities/structures.ts

@@ -21,7 +21,7 @@ import {
   Table,
   INTERNAL_TABLE_SOURCE_ID,
   TableSourceType,
-  AutomationIOType,
+  Query,
 } from "@budibase/types"
 
 const { BUILTIN_ROLE_IDS } = roles
@@ -360,7 +360,7 @@ export function basicDatasource(): { datasource: Datasource } {
   }
 }
 
-export function basicQuery(datasourceId: string) {
+export function basicQuery(datasourceId: string): Query {
   return {
     datasourceId: datasourceId,
     name: "New Query",
@@ -368,6 +368,8 @@ export function basicQuery(datasourceId: string) {
     fields: {},
     schema: {},
     queryVerb: "read",
+    transformer: null,
+    readable: true,
   }
 }
 

packages/types/src/shared/typeUtils.ts

@@ -7,3 +7,5 @@ export type ISO8601 = string
 export type RequiredKeys<T> = {
   [K in keyof Required<T>]: T[K]
 }
+
+export type WithRequired<T, K extends keyof T> = T & Required<Pick<T, K>>