From 940de8b6a0a6e013b95852e7d8268b205150f371 Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Tue, 21 Feb 2023 17:13:24 +0000 Subject: [PATCH] Run CI steps in parallel (#9760) * Parallel CI * Add build to integration test * Add checkout to top of each run * Revert branch update for ci job * Experiment with --runInBand for CI * Fix intermittent backend-core migration test failure * Fix hanging worker redis connection * Update naming from reset to newTenant --- .github/workflows/budibase_ci.yml | 94 ++++++++------ .husky/pre-commit | 2 - packages/backend-core/jest.config.ts | 10 +- packages/backend-core/package.json | 2 +- .../passport/sso/tests/google.spec.ts | 2 +- .../backend-core/src/migrations/migrations.ts | 116 +++++++++--------- ...x.spec.js.snap => migrations.spec.ts.snap} | 0 .../src/migrations/tests/index.spec.js | 57 --------- .../src/migrations/tests/migrations.spec.ts | 64 ++++++++++ packages/backend-core/src/redis/init.ts | 6 +- packages/backend-core/src/redis/redis.ts | 5 + .../tests/utilities/DBTestConfiguration.ts | 4 + packages/server/jest.config.ts | 21 ++-- packages/server/package.json | 2 +- packages/server/specs/resources/query.js | 2 +- packages/server/specs/resources/table.js | 2 +- .../src/api/routes/tests/static.spec.js | 12 -- .../server/src/api/routes/tests/user.spec.js | 5 +- packages/server/src/utilities/redis.ts | 2 + packages/worker/jest.config.ts | 25 ++-- packages/worker/package.json | 2 +- packages/worker/src/utilities/redis.ts | 2 + 22 files changed, 227 insertions(+), 210 deletions(-) rename packages/backend-core/src/migrations/tests/__snapshots__/{index.spec.js.snap => migrations.spec.ts.snap} (100%) delete mode 100644 packages/backend-core/src/migrations/tests/index.spec.js create mode 100644 packages/backend-core/src/migrations/tests/migrations.spec.ts diff --git a/.github/workflows/budibase_ci.yml b/.github/workflows/budibase_ci.yml index c07f9b2c28..e0263546ff 100644 --- a/.github/workflows/budibase_ci.yml +++ b/.github/workflows/budibase_ci.yml @@ -11,7 +11,6 @@ on: branches: - master - develop - - release workflow_dispatch: env: @@ -20,9 +19,53 @@ env: PERSONAL_ACCESS_TOKEN : ${{ secrets.PERSONAL_ACCESS_TOKEN }} jobs: + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Use Node.js 14.x + uses: actions/setup-node@v1 + with: + node-version: 14.x + - run: yarn + - run: yarn lint + build: runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Use Node.js 14.x + uses: actions/setup-node@v1 + with: + node-version: 14.x + - name: Install Pro + run: yarn install:pro $BRANCH $BASE_BRANCH + - run: yarn + - run: yarn bootstrap + - run: yarn build + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Use Node.js 14.x + uses: actions/setup-node@v1 + with: + node-version: 14.x + - name: Install Pro + run: yarn install:pro $BRANCH $BASE_BRANCH + - run: yarn + - run: yarn bootstrap + - run: yarn test + - uses: codecov/codecov-action@v1 + with: + token: ${{ secrets.CODECOV_TOKEN }} # not required for public repos + files: ./packages/server/coverage/clover.xml,./packages/worker/coverage/clover.xml,./packages/backend-core/coverage/clover.xml + name: codecov-umbrella + verbose: true + + integration-test: + runs-on: ubuntu-latest services: couchdb: image: ibmcom/couchdb3 @@ -31,39 +74,18 @@ jobs: COUCHDB_USER: budibase ports: - 4567:5984 - - strategy: - matrix: - node-version: [14.x] - steps: - - uses: actions/checkout@v2 - - - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v1 - with: - node-version: ${{ matrix.node-version }} - - - name: Install Pro - run: yarn install:pro $BRANCH $BASE_BRANCH - - - run: yarn - - run: yarn bootstrap - - run: yarn lint - - run: yarn build - - run: yarn test - env: - CI: true - name: Budibase CI - - uses: codecov/codecov-action@v1 - with: - token: ${{ secrets.CODECOV_TOKEN }} # not required for public repos - files: ./packages/server/coverage/clover.xml,./packages/worker/coverage/clover.xml,./packages/backend-core/coverage/clover.xml - name: codecov-umbrella - verbose: true - - - name: QA Core Integration Tests - run: | - cd qa-core - yarn - yarn api:test:ci \ No newline at end of file + - uses: actions/checkout@v2 + - name: Use Node.js 14.x + uses: actions/setup-node@v1 + with: + node-version: 14.x + - name: Install Pro + run: yarn install:pro $BRANCH $BASE_BRANCH + - run: yarn + - run: yarn bootstrap + - run: yarn build + - run: | + cd qa-core + yarn + yarn api:test:ci diff --git a/.husky/pre-commit b/.husky/pre-commit index 3b614330e0..6700f51282 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -1,4 +1,2 @@ #!/bin/sh . "$(dirname "$0")/_/husky.sh" - -yarn run lint diff --git a/packages/backend-core/jest.config.ts b/packages/backend-core/jest.config.ts index 0483fb073a..1e69797e71 100644 --- a/packages/backend-core/jest.config.ts +++ b/packages/backend-core/jest.config.ts @@ -9,15 +9,9 @@ const baseConfig: Config.InitialProjectOptions = { transform: { "^.+\\.ts?$": "@swc/jest", }, -} - -if (!process.env.CI) { - // use sources when not in CI - baseConfig.moduleNameMapper = { + moduleNameMapper: { "@budibase/types": "/../types/src", - } -} else { - console.log("Running tests with compiled dependency sources") + }, } const config: Config.InitialOptions = { diff --git a/packages/backend-core/package.json b/packages/backend-core/package.json index c03600f5da..a32f5fd4dd 100644 --- a/packages/backend-core/package.json +++ b/packages/backend-core/package.json @@ -18,7 +18,7 @@ "build:pro": "../../scripts/pro/build.sh", "postbuild": "yarn run build:pro", "build:dev": "yarn prebuild && tsc --build --watch --preserveWatchOutput", - "test": "jest --coverage", + "test": "jest --coverage --runInBand", "test:watch": "jest --watchAll" }, "dependencies": { diff --git a/packages/backend-core/src/middleware/passport/sso/tests/google.spec.ts b/packages/backend-core/src/middleware/passport/sso/tests/google.spec.ts index eb8ffc9b71..d0689a1f0a 100644 --- a/packages/backend-core/src/middleware/passport/sso/tests/google.spec.ts +++ b/packages/backend-core/src/middleware/passport/sso/tests/google.spec.ts @@ -19,7 +19,7 @@ describe("google", () => { const callbackUrl = generator.url() it("should create successfully create a google strategy", async () => { - await google.strategyFactory(googleConfig, callbackUrl) + await google.strategyFactory(googleConfig, callbackUrl, mockSaveUserFn) const expectedOptions = { clientID: googleConfig.clientID, diff --git a/packages/backend-core/src/migrations/migrations.ts b/packages/backend-core/src/migrations/migrations.ts index 79c7eb55ea..2e3524775f 100644 --- a/packages/backend-core/src/migrations/migrations.ts +++ b/packages/backend-core/src/migrations/migrations.ts @@ -4,7 +4,7 @@ import { StaticDatabases, getAllApps, getGlobalDBName, - doWithDB, + getDB, } from "../db" import environment from "../environment" import * as platform from "../platform" @@ -86,66 +86,65 @@ export const runMigration = async ( count++ const lengthStatement = length > 1 ? `[${count}/${length}]` : "" - await doWithDB(dbName, async (db: any) => { - try { - const doc = await getMigrationsDoc(db) + const db = getDB(dbName) + try { + const doc = await getMigrationsDoc(db) - // the migration has already been run - if (doc[migrationName]) { - // check for force - if ( - options.force && - options.force[migrationType] && - options.force[migrationType].includes(migrationName) - ) { - log( - `[Tenant: ${tenantId}] [Migration: ${migrationName}] [DB: ${dbName}] Forcing` - ) - } else { - // no force, exit - return - } - } - - // check if the migration is not a no-op - if (!options.noOp) { + // the migration has already been run + if (doc[migrationName]) { + // check for force + if ( + options.force && + options.force[migrationType] && + options.force[migrationType].includes(migrationName) + ) { log( - `[Tenant: ${tenantId}] [Migration: ${migrationName}] [DB: ${dbName}] Running ${lengthStatement}` - ) - - if (migration.preventRetry) { - // eagerly set the completion date - // so that we never run this migration twice even upon failure - doc[migrationName] = Date.now() - const response = await db.put(doc) - doc._rev = response.rev - } - - // run the migration - if (migrationType === MigrationType.APP) { - await context.doInAppContext(db.name, async () => { - await migration.fn(db) - }) - } else { - await migration.fn(db) - } - - log( - `[Tenant: ${tenantId}] [Migration: ${migrationName}] [DB: ${dbName}] Complete` + `[Tenant: ${tenantId}] [Migration: ${migrationName}] [DB: ${dbName}] Forcing` ) + } else { + // no force, exit + return } - - // mark as complete - doc[migrationName] = Date.now() - await db.put(doc) - } catch (err) { - console.error( - `[Tenant: ${tenantId}] [Migration: ${migrationName}] [DB: ${dbName}] Error: `, - err - ) - throw err } - }) + + // check if the migration is not a no-op + if (!options.noOp) { + log( + `[Tenant: ${tenantId}] [Migration: ${migrationName}] [DB: ${dbName}] Running ${lengthStatement}` + ) + + if (migration.preventRetry) { + // eagerly set the completion date + // so that we never run this migration twice even upon failure + doc[migrationName] = Date.now() + const response = await db.put(doc) + doc._rev = response.rev + } + + // run the migration + if (migrationType === MigrationType.APP) { + await context.doInAppContext(db.name, async () => { + await migration.fn(db) + }) + } else { + await migration.fn(db) + } + + log( + `[Tenant: ${tenantId}] [Migration: ${migrationName}] [DB: ${dbName}] Complete` + ) + } + + // mark as complete + doc[migrationName] = Date.now() + await db.put(doc) + } catch (err) { + console.error( + `[Tenant: ${tenantId}] [Migration: ${migrationName}] [DB: ${dbName}] Error: `, + err + ) + throw err + } } } @@ -185,7 +184,10 @@ export const runMigrations = async ( // for all migrations for (const migration of migrations) { // run the migration - await context.doInTenant(tenantId, () => runMigration(migration, options)) + await context.doInTenant( + tenantId, + async () => await runMigration(migration, options) + ) } } console.log("Migrations complete") diff --git a/packages/backend-core/src/migrations/tests/__snapshots__/index.spec.js.snap b/packages/backend-core/src/migrations/tests/__snapshots__/migrations.spec.ts.snap similarity index 100% rename from packages/backend-core/src/migrations/tests/__snapshots__/index.spec.js.snap rename to packages/backend-core/src/migrations/tests/__snapshots__/migrations.spec.ts.snap diff --git a/packages/backend-core/src/migrations/tests/index.spec.js b/packages/backend-core/src/migrations/tests/index.spec.js deleted file mode 100644 index c1915510c3..0000000000 --- a/packages/backend-core/src/migrations/tests/index.spec.js +++ /dev/null @@ -1,57 +0,0 @@ -require("../../../tests") -const { runMigrations, getMigrationsDoc } = require("../index") -const { getGlobalDBName, getDB } = require("../../db") - -const { structures, testEnv } = require("../../../tests") -testEnv.multiTenant() - -let db - -describe("migrations", () => { - - const migrationFunction = jest.fn() - - const MIGRATIONS = [{ - type: "global", - name: "test", - fn: migrationFunction - }] - - let tenantId - - beforeEach(() => { - tenantId = structures.tenant.id() - db = getDB(getGlobalDBName(tenantId)) - }) - - afterEach(async () => { - jest.clearAllMocks() - await db.destroy() - }) - - const migrate = () => { - return runMigrations(MIGRATIONS, { tenantIds: [tenantId]}) - } - - it("should run a new migration", async () => { - await migrate() - expect(migrationFunction).toHaveBeenCalled() - const doc = await getMigrationsDoc(db) - expect(doc.test).toBeDefined() - }) - - it("should match snapshot", async () => { - await migrate() - const doc = await getMigrationsDoc(db) - expect(doc).toMatchSnapshot() - }) - - it("should skip a previously run migration", async () => { - await migrate() - const previousMigrationTime = await getMigrationsDoc(db).test - await migrate() - const currentMigrationTime = await getMigrationsDoc(db).test - expect(migrationFunction).toHaveBeenCalledTimes(1) - expect(currentMigrationTime).toBe(previousMigrationTime) - }) -}) \ No newline at end of file diff --git a/packages/backend-core/src/migrations/tests/migrations.spec.ts b/packages/backend-core/src/migrations/tests/migrations.spec.ts new file mode 100644 index 0000000000..c74ab816c1 --- /dev/null +++ b/packages/backend-core/src/migrations/tests/migrations.spec.ts @@ -0,0 +1,64 @@ +import { testEnv, DBTestConfiguration } from "../../../tests" +import * as migrations from "../index" +import * as context from "../../context" +import { MigrationType } from "@budibase/types" + +testEnv.multiTenant() + +describe("migrations", () => { + const config = new DBTestConfiguration() + + const migrationFunction = jest.fn() + + const MIGRATIONS = [ + { + type: MigrationType.GLOBAL, + name: "test" as any, + fn: migrationFunction, + }, + ] + + beforeEach(() => { + config.newTenant() + }) + + afterEach(async () => { + jest.clearAllMocks() + }) + + const migrate = () => { + return migrations.runMigrations(MIGRATIONS, { + tenantIds: [config.tenantId], + }) + } + + it("should run a new migration", async () => { + await config.doInTenant(async () => { + await migrate() + expect(migrationFunction).toHaveBeenCalled() + const db = context.getGlobalDB() + const doc = await migrations.getMigrationsDoc(db) + expect(doc.test).toBeDefined() + }) + }) + + it("should match snapshot", async () => { + await config.doInTenant(async () => { + await migrate() + const doc = await migrations.getMigrationsDoc(context.getGlobalDB()) + expect(doc).toMatchSnapshot() + }) + }) + + it("should skip a previously run migration", async () => { + await config.doInTenant(async () => { + const db = context.getGlobalDB() + await migrate() + const previousDoc = await migrations.getMigrationsDoc(db) + await migrate() + const currentDoc = await migrations.getMigrationsDoc(db) + expect(migrationFunction).toHaveBeenCalledTimes(1) + expect(currentDoc.test).toBe(previousDoc.test) + }) + }) +}) diff --git a/packages/backend-core/src/redis/init.ts b/packages/backend-core/src/redis/init.ts index 00329ffb84..485268edad 100644 --- a/packages/backend-core/src/redis/init.ts +++ b/packages/backend-core/src/redis/init.ts @@ -20,13 +20,17 @@ async function init() { ).init() } -process.on("exit", async () => { +export async function shutdown() { if (userClient) await userClient.finish() if (sessionClient) await sessionClient.finish() if (appClient) await appClient.finish() if (cacheClient) await cacheClient.finish() if (writethroughClient) await writethroughClient.finish() if (lockClient) await lockClient.finish() +} + +process.on("exit", async () => { + await shutdown() }) export async function getUserClient() { diff --git a/packages/backend-core/src/redis/redis.ts b/packages/backend-core/src/redis/redis.ts index 2669cd816a..951369496a 100644 --- a/packages/backend-core/src/redis/redis.ts +++ b/packages/backend-core/src/redis/redis.ts @@ -91,6 +91,11 @@ function init(selectDb = DEFAULT_SELECT_DB) { } // attach handlers client.on("end", (err: Error) => { + if (env.isTest()) { + // don't try to re-connect in test env + // allow the process to exit + return + } connectionError(selectDb, timeout, err) }) client.on("error", (err: Error) => { diff --git a/packages/backend-core/tests/utilities/DBTestConfiguration.ts b/packages/backend-core/tests/utilities/DBTestConfiguration.ts index cad62e2979..e5e57a99a3 100644 --- a/packages/backend-core/tests/utilities/DBTestConfiguration.ts +++ b/packages/backend-core/tests/utilities/DBTestConfiguration.ts @@ -12,6 +12,10 @@ class DBTestConfiguration { this.tenantId = structures.tenant.id() } + newTenant() { + this.tenantId = structures.tenant.id() + } + // TENANCY doInTenant(task: any) { diff --git a/packages/server/jest.config.ts b/packages/server/jest.config.ts index 41558d4c8e..331912aa19 100644 --- a/packages/server/jest.config.ts +++ b/packages/server/jest.config.ts @@ -11,22 +11,17 @@ const baseConfig: Config.InitialProjectOptions = { transform: { "^.+\\.ts?$": "@swc/jest", }, -} - -if (!process.env.CI) { - // use sources when not in CI - baseConfig.moduleNameMapper = { + moduleNameMapper: { "@budibase/backend-core/(.*)": "/../backend-core/$1", "@budibase/backend-core": "/../backend-core/src", "@budibase/types": "/../types/src", - } - // add pro sources if they exist - if (fs.existsSync("../../../budibase-pro")) { - baseConfig.moduleNameMapper["@budibase/pro"] = - "/../../../budibase-pro/packages/pro/src" - } -} else { - console.log("Running tests with compiled dependency sources") + }, +} + +// add pro sources if they exist +if (fs.existsSync("../../../budibase-pro")) { + baseConfig.moduleNameMapper["@budibase/pro"] = + "/../../../budibase-pro/packages/pro/src" } const config: Config.InitialOptions = { diff --git a/packages/server/package.json b/packages/server/package.json index 5812a84717..3eb133e272 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -14,7 +14,7 @@ "build:dev": "yarn prebuild && tsc --build --watch --preserveWatchOutput", "debug": "yarn build && node --expose-gc --inspect=9222 dist/index.js", "postbuild": "copyfiles -u 1 src/**/*.svelte dist/ && copyfiles -u 1 src/**/*.hbs dist/ && copyfiles -u 1 src/**/*.json dist/", - "test": "jest --coverage --maxWorkers=2", + "test": "jest --coverage --runInBand", "test:watch": "jest --watch", "predocker": "copyfiles -f ../client/dist/budibase-client.js ../client/manifest.json client", "build:docker": "yarn run predocker && docker build . -t app-service --label version=$BUDIBASE_RELEASE_VERSION", diff --git a/packages/server/specs/resources/query.js b/packages/server/specs/resources/query.js index 10544ee7eb..1442e46a04 100644 --- a/packages/server/specs/resources/query.js +++ b/packages/server/specs/resources/query.js @@ -1,6 +1,6 @@ const Resource = require("./utils/Resource") const { object } = require("./utils") -const { BaseQueryVerbs } = require("../../dist/constants") +const { BaseQueryVerbs } = require("../../src/constants") const query = { _id: "query_datasource_plus_4d8be0c506b9465daf4bf84d890fdab6_454854487c574d45bc4029b1e153219e", diff --git a/packages/server/specs/resources/table.js b/packages/server/specs/resources/table.js index 9bc57daf42..523a3a9dfd 100644 --- a/packages/server/specs/resources/table.js +++ b/packages/server/specs/resources/table.js @@ -2,7 +2,7 @@ const { FieldTypes, RelationshipTypes, FormulaTypes, -} = require("../../dist/constants") +} = require("../../src/constants") const { object } = require("./utils") const Resource = require("./utils/Resource") diff --git a/packages/server/src/api/routes/tests/static.spec.js b/packages/server/src/api/routes/tests/static.spec.js index a0532f12fb..13d963d057 100644 --- a/packages/server/src/api/routes/tests/static.spec.js +++ b/packages/server/src/api/routes/tests/static.spec.js @@ -13,18 +13,6 @@ describe("/static", () => { app = await config.init() }) - describe("/builder", () => { - it("should serve the builder", async () => { - const res = await request - .get("/builder/portal") - .set(config.defaultHeaders()) - .expect("Content-Type", /text\/html/) - .expect(200) - - expect(res.text).toContain("Budibase") - }) - }) - describe("/app", () => { beforeEach(() => { jest.clearAllMocks() diff --git a/packages/server/src/api/routes/tests/user.spec.js b/packages/server/src/api/routes/tests/user.spec.js index bae784cf3d..6b674a8479 100644 --- a/packages/server/src/api/routes/tests/user.spec.js +++ b/packages/server/src/api/routes/tests/user.spec.js @@ -1,4 +1,4 @@ -const { roles, utils } = require("@budibase/backend-core") +const { roles } = require("@budibase/backend-core") const { checkPermissionsEndpoint } = require("./utilities/TestFunctions") const setup = require("./utilities") const { BUILTIN_ROLE_IDS } = roles @@ -21,8 +21,7 @@ describe("/users", () => { afterAll(setup.afterAll) - // For some reason this cannot be a beforeAll or the test "should be able to update the user" fail - beforeEach(async () => { + beforeAll(async () => { await config.init() }) diff --git a/packages/server/src/utilities/redis.ts b/packages/server/src/utilities/redis.ts index 1b7a3ce64c..dc37baae58 100644 --- a/packages/server/src/utilities/redis.ts +++ b/packages/server/src/utilities/redis.ts @@ -21,6 +21,8 @@ export async function shutdown() { if (devAppClient) await devAppClient.finish() if (debounceClient) await debounceClient.finish() if (flagClient) await flagClient.finish() + // shutdown core clients + await redis.clients.shutdown() console.log("Redis shutdown") } diff --git a/packages/worker/jest.config.ts b/packages/worker/jest.config.ts index 8b0514211b..cdacfa411a 100644 --- a/packages/worker/jest.config.ts +++ b/packages/worker/jest.config.ts @@ -12,24 +12,19 @@ const config: Config.InitialOptions = { transform: { "^.+\\.ts?$": "@swc/jest", }, -} - -if (!process.env.CI) { - // use sources when not in CI - config.moduleNameMapper = { + moduleNameMapper: { "@budibase/backend-core/(.*)": "/../backend-core/$1", "@budibase/backend-core": "/../backend-core/src", "@budibase/types": "/../types/src", - } - // add pro sources if they exist - if (fs.existsSync("../../../budibase-pro")) { - config.moduleNameMapper["@budibase/pro/(.*)"] = - "/../../../budibase-pro/packages/pro/$1" - config.moduleNameMapper["@budibase/pro"] = - "/../../../budibase-pro/packages/pro/src" - } -} else { - console.log("Running tests with compiled dependency sources") + }, +} + +// add pro sources if they exist +if (fs.existsSync("../../../budibase-pro")) { + config.moduleNameMapper["@budibase/pro/(.*)"] = + "/../../../budibase-pro/packages/pro/$1" + config.moduleNameMapper["@budibase/pro"] = + "/../../../budibase-pro/packages/pro/src" } export default config diff --git a/packages/worker/package.json b/packages/worker/package.json index 9fd2843ae4..0fb3abe53b 100644 --- a/packages/worker/package.json +++ b/packages/worker/package.json @@ -22,7 +22,7 @@ "build:docker": "docker build . -t worker-service --label version=$BUDIBASE_RELEASE_VERSION", "dev:stack:init": "node ./scripts/dev/manage.js init", "dev:builder": "npm run dev:stack:init && nodemon", - "test": "jest --coverage --maxWorkers=2", + "test": "jest --coverage --runInBand", "test:watch": "jest --watch", "env:multi:enable": "node scripts/multiTenancy.js enable", "env:multi:disable": "node scripts/multiTenancy.js disable", diff --git a/packages/worker/src/utilities/redis.ts b/packages/worker/src/utilities/redis.ts index 893ec9f0a8..9171fe97ee 100644 --- a/packages/worker/src/utilities/redis.ts +++ b/packages/worker/src/utilities/redis.ts @@ -54,6 +54,8 @@ export async function init() { export async function shutdown() { if (pwResetClient) await pwResetClient.finish() if (invitationClient) await invitationClient.finish() + // shutdown core clients + await redis.clients.shutdown() console.log("Redis shutdown") }