From cf5316ec8d3d2a86ac15f53c660dcd9e67104d5d Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 27 Mar 2023 19:38:49 +0100 Subject: [PATCH] General fixes for open handles, attempting to find and close all issues in server which are stopping shutdown of Jest suite. --- packages/backend-core/src/index.ts | 1 + packages/backend-core/src/queue/queue.ts | 7 +- packages/backend-core/src/redis/redis.ts | 5 +- packages/backend-core/src/timers/index.ts | 1 + packages/backend-core/src/timers/timers.ts | 22 +++++ packages/backend-core/tests/jestSetup.ts | 5 + .../src/api/routes/tests/appSync.spec.ts | 2 +- packages/server/src/app.ts | 3 +- .../server/src/automations/automationUtils.ts | 2 +- .../src/automations/tests/automation.spec.js | 84 ---------------- .../src/automations/tests/automation.spec.ts | 99 +++++++++++++++++++ packages/server/src/integrations/redis.ts | 2 +- .../integrations/tests/googlesheets.spec.ts | 4 + .../src/integrations/tests/redis.spec.ts | 14 ++- packages/server/src/tests/jestSetup.ts | 6 +- .../src/tests/utilities/TestConfiguration.ts | 2 + .../server/src/tests/utilities/structures.ts | 14 ++- packages/worker/src/index.ts | 2 + .../worker/src/tests/TestConfiguration.ts | 2 + packages/worker/src/tests/jestSetup.ts | 6 +- 20 files changed, 182 insertions(+), 101 deletions(-) create mode 100644 packages/backend-core/src/timers/index.ts create mode 100644 packages/backend-core/src/timers/timers.ts delete mode 100644 packages/server/src/automations/tests/automation.spec.js create mode 100644 packages/server/src/automations/tests/automation.spec.ts diff --git a/packages/backend-core/src/index.ts b/packages/backend-core/src/index.ts index 48569548e3..a6d5423756 100644 --- a/packages/backend-core/src/index.ts +++ b/packages/backend-core/src/index.ts @@ -24,6 +24,7 @@ export * as redis from "./redis" export * as locks from "./redis/redlockImpl" export * as utils from "./utils" export * as errors from "./errors" +export * as timers from "./timers" export { default as env } from "./environment" export { SearchParams } from "./db" // Add context to tenancy for backwards compatibility diff --git a/packages/backend-core/src/queue/queue.ts b/packages/backend-core/src/queue/queue.ts index c57ebafb1f..0658147709 100644 --- a/packages/backend-core/src/queue/queue.ts +++ b/packages/backend-core/src/queue/queue.ts @@ -4,6 +4,7 @@ import { JobQueue } from "./constants" import InMemoryQueue from "./inMemoryQueue" import BullQueue from "bull" import { addListeners, StalledFn } from "./listeners" +import * as timers from "../timers" const CLEANUP_PERIOD_MS = 60 * 1000 let QUEUES: BullQueue.Queue[] | InMemoryQueue[] = [] @@ -29,8 +30,8 @@ export function createQueue( } addListeners(queue, jobQueue, opts?.removeStalledCb) QUEUES.push(queue) - if (!cleanupInterval) { - cleanupInterval = setInterval(cleanup, CLEANUP_PERIOD_MS) + if (!cleanupInterval && !env.isTest()) { + cleanupInterval = timers.set(cleanup, CLEANUP_PERIOD_MS) // fire off an initial cleanup cleanup().catch(err => { console.error(`Unable to cleanup automation queue initially - ${err}`) @@ -41,7 +42,7 @@ export function createQueue( export async function shutdown() { if (cleanupInterval) { - clearInterval(cleanupInterval) + timers.clear(cleanupInterval) } if (QUEUES.length) { for (let queue of QUEUES) { diff --git a/packages/backend-core/src/redis/redis.ts b/packages/backend-core/src/redis/redis.ts index 951369496a..186865ccda 100644 --- a/packages/backend-core/src/redis/redis.ts +++ b/packages/backend-core/src/redis/redis.ts @@ -8,6 +8,7 @@ import { SEPARATOR, SelectableDatabase, } from "./utils" +import * as timers from "../timers" const RETRY_PERIOD_MS = 2000 const STARTUP_TIMEOUT_MS = 5000 @@ -117,9 +118,9 @@ function waitForConnection(selectDb: number = DEFAULT_SELECT_DB) { return } // check if the connection is ready - const interval = setInterval(() => { + const interval = timers.set(() => { if (CONNECTED) { - clearInterval(interval) + timers.clear(interval) resolve("") } }, 500) diff --git a/packages/backend-core/src/timers/index.ts b/packages/backend-core/src/timers/index.ts new file mode 100644 index 0000000000..c9d642709f --- /dev/null +++ b/packages/backend-core/src/timers/index.ts @@ -0,0 +1 @@ +export * from "./timers" diff --git a/packages/backend-core/src/timers/timers.ts b/packages/backend-core/src/timers/timers.ts new file mode 100644 index 0000000000..000be74821 --- /dev/null +++ b/packages/backend-core/src/timers/timers.ts @@ -0,0 +1,22 @@ +let intervals: NodeJS.Timeout[] = [] + +export function set(callback: () => any, period: number) { + const interval = setInterval(callback, period) + intervals.push(interval) + return interval +} + +export function clear(interval: NodeJS.Timeout) { + const idx = intervals.indexOf(interval) + if (idx !== -1) { + intervals.splice(idx, 1) + } + clearInterval(interval) +} + +export function cleanup() { + for (let interval of intervals) { + clearInterval(interval) + } + intervals = [] +} diff --git a/packages/backend-core/tests/jestSetup.ts b/packages/backend-core/tests/jestSetup.ts index e786086de6..be81fbff75 100644 --- a/packages/backend-core/tests/jestSetup.ts +++ b/packages/backend-core/tests/jestSetup.ts @@ -1,5 +1,6 @@ import "./logging" import env from "../src/environment" +import { cleanup } from "../src/timers" import { mocks, testContainerUtils } from "./utilities" // must explicitly enable fetch mock @@ -21,3 +22,7 @@ if (!process.env.CI) { } testContainerUtils.setupEnv(env) + +afterAll(() => { + cleanup() +}) diff --git a/packages/server/src/api/routes/tests/appSync.spec.ts b/packages/server/src/api/routes/tests/appSync.spec.ts index f82f62405e..bc35c81ae9 100644 --- a/packages/server/src/api/routes/tests/appSync.spec.ts +++ b/packages/server/src/api/routes/tests/appSync.spec.ts @@ -24,7 +24,7 @@ describe("/api/applications/:appId/sync", () => { return rows } - it("make sure its empty initially", async () => { + it("make sure that user metadata is correctly sync'd", async () => { const rows = await getUserMetadata() expect(rows.length).toBe(1) }) diff --git a/packages/server/src/app.ts b/packages/server/src/app.ts index 00f2aca7fc..a7070b3c19 100644 --- a/packages/server/src/app.ts +++ b/packages/server/src/app.ts @@ -27,7 +27,7 @@ import * as api from "./api" import * as automations from "./automations" import { Thread } from "./threads" import * as redis from "./utilities/redis" -import { events, logging, middleware } from "@budibase/backend-core" +import { events, logging, middleware, timers } from "@budibase/backend-core" import { initialise as initialiseWebsockets } from "./websocket" import { startup } from "./startup" const Sentry = require("@sentry/node") @@ -84,6 +84,7 @@ server.on("close", async () => { } shuttingDown = true console.log("Server Closed") + timers.cleanup() await automations.shutdown() await redis.shutdown() events.shutdown() diff --git a/packages/server/src/automations/automationUtils.ts b/packages/server/src/automations/automationUtils.ts index 254d9c624b..478acc8fab 100644 --- a/packages/server/src/automations/automationUtils.ts +++ b/packages/server/src/automations/automationUtils.ts @@ -23,7 +23,7 @@ import { LoopStep, LoopStepType, LoopInput } from "../definitions/automations" * @returns {object} The inputs object which has had all the various types supported by this function converted to their * primitive types. */ -export function cleanInputValues(inputs: Record, schema: any) { +export function cleanInputValues(inputs: Record, schema?: any) { if (schema == null) { return inputs } diff --git a/packages/server/src/automations/tests/automation.spec.js b/packages/server/src/automations/tests/automation.spec.js deleted file mode 100644 index b63d1f4b5b..0000000000 --- a/packages/server/src/automations/tests/automation.spec.js +++ /dev/null @@ -1,84 +0,0 @@ -jest.mock("../../threads/automation") -jest.mock("../../utilities/redis", () => ({ - init: jest.fn(), - checkTestFlag: () => { - return false - }, -})) - -jest.spyOn(global.console, "error") - -require("../../environment") -const automation = require("../index") -const thread = require("../../threads/automation") -const triggers = require("../triggers") -const { basicAutomation } = require("../../tests/utilities/structures") -const { wait } = require("../../utilities") -const { makePartial } = require("../../tests/utilities") -const { cleanInputValues } = require("../automationUtils") -const setup = require("./utilities") - -describe("Run through some parts of the automations system", () => { - let config = setup.getConfig() - - beforeAll(async () => { - await automation.init() - await config.init() - }) - - afterAll(setup.afterAll) - - it("should be able to init in builder", async () => { - await triggers.externalTrigger(basicAutomation(), { a: 1, appId: config.appId }) - await wait(100) - expect(thread.execute).toHaveBeenCalled() - }) - - it("should check coercion", async () => { - const table = await config.createTable() - const automation = basicAutomation() - automation.definition.trigger.inputs.tableId = table._id - automation.definition.trigger.stepId = "APP" - automation.definition.trigger.inputs.fields = { a: "number" } - await triggers.externalTrigger(automation, { - appId: config.getAppId(), - fields: { - a: "1" - } - }) - await wait(100) - expect(thread.execute).toHaveBeenCalledWith(makePartial({ - data: { - event: { - fields: { - a: 1 - } - } - } - }), expect.any(Function)) - }) - - it("should be able to clean inputs with the utilities", () => { - // can't clean without a schema - let output = cleanInputValues({a: "1"}) - expect(output.a).toBe("1") - output = cleanInputValues({a: "1", b: "true", c: "false", d: 1, e: "help"}, { - properties: { - a: { - type: "number", - }, - b: { - type: "boolean", - }, - c: { - type: "boolean", - } - } - }) - expect(output.a).toBe(1) - expect(output.b).toBe(true) - expect(output.c).toBe(false) - expect(output.d).toBe(1) - expect(output.e).toBe("help") - }) -}) diff --git a/packages/server/src/automations/tests/automation.spec.ts b/packages/server/src/automations/tests/automation.spec.ts new file mode 100644 index 0000000000..5dbaee492d --- /dev/null +++ b/packages/server/src/automations/tests/automation.spec.ts @@ -0,0 +1,99 @@ +jest.mock("../../threads/automation") +jest.mock("../../utilities/redis", () => ({ + init: jest.fn(), + checkTestFlag: () => { + return false + }, +})) + +jest.spyOn(global.console, "error") + +import "../../environment" +import * as automation from "../index" +import * as thread from "../../threads/automation" +import * as triggers from "../triggers" +import { basicAutomation } from "../../tests/utilities/structures" +import { wait } from "../../utilities" +import { makePartial } from "../../tests/utilities" +import { cleanInputValues } from "../automationUtils" +import * as setup from "./utilities" +import { Automation } from "@budibase/types" + +describe("Run through some parts of the automations system", () => { + let config = setup.getConfig() + + beforeAll(async () => { + await automation.init() + await config.init() + }) + + afterAll(async () => { + await automation.shutdown() + setup.afterAll() + }) + + it("should be able to init in builder", async () => { + const automation: Automation = { + ...basicAutomation(), + appId: config.appId, + } + const fields: any = { a: 1, appId: config.appId } + await triggers.externalTrigger(automation, fields) + await wait(100) + expect(thread.execute).toHaveBeenCalled() + }) + + it("should check coercion", async () => { + const table = await config.createTable() + const automation: any = basicAutomation() + automation.definition.trigger.inputs.tableId = table._id + automation.definition.trigger.stepId = "APP" + automation.definition.trigger.inputs.fields = { a: "number" } + const fields: any = { + appId: config.getAppId(), + fields: { + a: "1", + }, + } + await triggers.externalTrigger(automation, fields) + await wait(100) + expect(thread.execute).toHaveBeenCalledWith( + makePartial({ + data: { + event: { + fields: { + a: 1, + }, + }, + }, + }), + expect.any(Function) + ) + }) + + it("should be able to clean inputs with the utilities", () => { + // can't clean without a schema + let output = cleanInputValues({ a: "1" }) + expect(output.a).toBe("1") + output = cleanInputValues( + { a: "1", b: "true", c: "false", d: 1, e: "help" }, + { + properties: { + a: { + type: "number", + }, + b: { + type: "boolean", + }, + c: { + type: "boolean", + }, + }, + } + ) + expect(output.a).toBe(1) + expect(output.b).toBe(true) + expect(output.c).toBe(false) + expect(output.d).toBe(1) + }) +}) diff --git a/packages/server/src/integrations/redis.ts b/packages/server/src/integrations/redis.ts index 8b0ed96b7b..4704b8e483 100644 --- a/packages/server/src/integrations/redis.ts +++ b/packages/server/src/integrations/redis.ts @@ -92,7 +92,7 @@ class RedisIntegration { } async disconnect() { - return this.client.disconnect() + return this.client.quit() } async redisContext(query: Function) { diff --git a/packages/server/src/integrations/tests/googlesheets.spec.ts b/packages/server/src/integrations/tests/googlesheets.spec.ts index eb263bd850..0e7669a957 100644 --- a/packages/server/src/integrations/tests/googlesheets.spec.ts +++ b/packages/server/src/integrations/tests/googlesheets.spec.ts @@ -39,6 +39,10 @@ describe("Google Sheets Integration", () => { config.setGoogleAuth("test") }) + afterAll(async () => { + await config.end() + }) + beforeEach(async () => { integration = new GoogleSheetsIntegration.integration({ spreadsheetId: "randomId", diff --git a/packages/server/src/integrations/tests/redis.spec.ts b/packages/server/src/integrations/tests/redis.spec.ts index 2ce8fe9326..9521d58a51 100644 --- a/packages/server/src/integrations/tests/redis.spec.ts +++ b/packages/server/src/integrations/tests/redis.spec.ts @@ -3,17 +3,17 @@ import { default as RedisIntegration } from "../redis" class TestConfiguration { integration: any - redis: any constructor(config: any = {}) { this.integration = new RedisIntegration.integration(config) - this.redis = new Redis({ + // have to kill the basic integration before replacing it + this.integration.client.quit() + this.integration.client = new Redis({ data: { test: "test", result: "1", }, }) - this.integration.client = this.redis } } @@ -24,13 +24,17 @@ describe("Redis Integration", () => { config = new TestConfiguration() }) + afterAll(() => { + config.integration.disconnect() + }) + it("calls the create method with the correct params", async () => { const body = { key: "key", value: "value", } await config.integration.create(body) - expect(await config.redis.get("key")).toEqual("value") + expect(await config.integration.client.get("key")).toEqual("value") }) it("calls the read method with the correct params", async () => { @@ -46,7 +50,7 @@ describe("Redis Integration", () => { key: "test", } await config.integration.delete(body) - expect(await config.redis.get(body.key)).toEqual(null) + expect(await config.integration.client.get(body.key)).toEqual(null) }) it("calls the pipeline method with the correct params", async () => { diff --git a/packages/server/src/tests/jestSetup.ts b/packages/server/src/tests/jestSetup.ts index b052d9941b..9f3b85cac4 100644 --- a/packages/server/src/tests/jestSetup.ts +++ b/packages/server/src/tests/jestSetup.ts @@ -1,6 +1,6 @@ import "./logging" import env from "../environment" -import { env as coreEnv } from "@budibase/backend-core" +import { env as coreEnv, timers } from "@budibase/backend-core" import { testContainerUtils } from "@budibase/backend-core/tests" if (!process.env.DEBUG) { @@ -17,3 +17,7 @@ if (!process.env.CI) { } testContainerUtils.setupEnv(env, coreEnv) + +afterAll(() => { + timers.cleanup() +}) diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index 80f804d219..cf337c689f 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -165,6 +165,8 @@ class TestConfiguration { } if (this.server) { this.server.close() + } else { + require("../../app").default.close() } if (this.allApps) { cleanup(this.allApps.map(app => app.appId)) diff --git a/packages/server/src/tests/utilities/structures.ts b/packages/server/src/tests/utilities/structures.ts index e38c0a5275..207863a6aa 100644 --- a/packages/server/src/tests/utilities/structures.ts +++ b/packages/server/src/tests/utilities/structures.ts @@ -106,7 +106,7 @@ export function newAutomation({ steps, trigger }: any = {}) { return automation } -export function basicAutomation() { +export function basicAutomation(appId?: string) { return { name: "My Automation", screenId: "kasdkfldsafkl", @@ -114,11 +114,23 @@ export function basicAutomation() { uiTree: {}, definition: { trigger: { + stepId: AutomationTriggerStepId.APP, + name: "test", + tagline: "test", + icon: "test", + description: "test", + type: "trigger", + id: "test", inputs: {}, + schema: { + inputs: {}, + outputs: {}, + }, }, steps: [], }, type: "automation", + appId, } } diff --git a/packages/worker/src/index.ts b/packages/worker/src/index.ts index 04413e8429..0b4e3be817 100644 --- a/packages/worker/src/index.ts +++ b/packages/worker/src/index.ts @@ -21,6 +21,7 @@ import { middleware, queue, env as coreEnv, + timers, } from "@budibase/backend-core" db.init() import Koa from "koa" @@ -91,6 +92,7 @@ server.on("close", async () => { } shuttingDown = true console.log("Server Closed") + timers.cleanup() await redis.shutdown() await events.shutdown() await queue.shutdown() diff --git a/packages/worker/src/tests/TestConfiguration.ts b/packages/worker/src/tests/TestConfiguration.ts index e5ed9e8141..e612742047 100644 --- a/packages/worker/src/tests/TestConfiguration.ts +++ b/packages/worker/src/tests/TestConfiguration.ts @@ -106,6 +106,8 @@ class TestConfiguration { async afterAll() { if (this.server) { await this.server.close() + } else { + await require("../index").default.close() } } diff --git a/packages/worker/src/tests/jestSetup.ts b/packages/worker/src/tests/jestSetup.ts index 31d36f00ae..7a3fb08cb9 100644 --- a/packages/worker/src/tests/jestSetup.ts +++ b/packages/worker/src/tests/jestSetup.ts @@ -2,7 +2,7 @@ import "./logging" import { mocks, testContainerUtils } from "@budibase/backend-core/tests" import env from "../environment" -import { env as coreEnv } from "@budibase/backend-core" +import { env as coreEnv, timers } from "@budibase/backend-core" // must explicitly enable fetch mock mocks.fetch.enable() @@ -21,3 +21,7 @@ if (!process.env.CI) { } testContainerUtils.setupEnv(env, coreEnv) + +afterAll(() => { + timers.cleanup() +})