From 6847cd6225d6ebbab2c5e12470049de9883f4fca Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 17 Sep 2023 15:02:02 +0200 Subject: [PATCH 1/3] backend-common: avoid starting keepalive loop in tests Signed-off-by: Patrik Oldsberg --- .changeset/four-parents-visit.md | 5 +++++ .github/vale/Vocab/Backstage/accept.txt | 1 + packages/backend-common/src/database/DatabaseManager.ts | 4 +++- 3 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 .changeset/four-parents-visit.md diff --git a/.changeset/four-parents-visit.md b/.changeset/four-parents-visit.md new file mode 100644 index 0000000000..041380ce1e --- /dev/null +++ b/.changeset/four-parents-visit.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Avoid starting database keepalive loop in tests. diff --git a/.github/vale/Vocab/Backstage/accept.txt b/.github/vale/Vocab/Backstage/accept.txt index 0229ed8e12..e120198ba9 100644 --- a/.github/vale/Vocab/Backstage/accept.txt +++ b/.github/vale/Vocab/Backstage/accept.txt @@ -180,6 +180,7 @@ jsx JWTs Kaewkasi Kaswell +keepalive Keyv Knex Koyeb diff --git a/packages/backend-common/src/database/DatabaseManager.ts b/packages/backend-common/src/database/DatabaseManager.ts index 4b655512bb..c1c9e56257 100644 --- a/packages/backend-common/src/database/DatabaseManager.ts +++ b/packages/backend-common/src/database/DatabaseManager.ts @@ -377,7 +377,9 @@ export class DatabaseManager { databaseClientOverrides, deps, ); - this.startKeepaliveLoop(pluginId, client); + if (process.env.NODE_ENV !== 'test') { + this.startKeepaliveLoop(pluginId, client); + } return client; }); From 74604806aae8aa6388852a02eabd69037ca25189 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 17 Sep 2023 15:02:57 +0200 Subject: [PATCH 2/3] backend-tasks: avoid starting task janitor in tests Signed-off-by: Patrik Oldsberg --- .changeset/rich-fishes-deny.md | 5 +++++ packages/backend-tasks/src/tasks/TaskScheduler.ts | 14 ++++++++------ 2 files changed, 13 insertions(+), 6 deletions(-) create mode 100644 .changeset/rich-fishes-deny.md diff --git a/.changeset/rich-fishes-deny.md b/.changeset/rich-fishes-deny.md new file mode 100644 index 0000000000..c90d76f20d --- /dev/null +++ b/.changeset/rich-fishes-deny.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-tasks': patch +--- + +Avoid starting task janitor in tests. diff --git a/packages/backend-tasks/src/tasks/TaskScheduler.ts b/packages/backend-tasks/src/tasks/TaskScheduler.ts index fe81a0054e..6b6436e52a 100644 --- a/packages/backend-tasks/src/tasks/TaskScheduler.ts +++ b/packages/backend-tasks/src/tasks/TaskScheduler.ts @@ -80,12 +80,14 @@ export class TaskScheduler { await migrateBackendTasks(knex); } - const janitor = new PluginTaskSchedulerJanitor({ - knex, - waitBetweenRuns: Duration.fromObject({ minutes: 1 }), - logger: opts.logger, - }); - janitor.start(); + if (process.env.NODE_ENV !== 'test') { + const janitor = new PluginTaskSchedulerJanitor({ + knex, + waitBetweenRuns: Duration.fromObject({ minutes: 1 }), + logger: opts.logger, + }); + janitor.start(); + } return knex; }); From 6440e758c0fbec5d4f1a1867f555bad528ad857f Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 17 Sep 2023 15:03:24 +0200 Subject: [PATCH 3/3] backend-taske: make sure all handles are cleaned up in tests Signed-off-by: Patrik Oldsberg --- .../src/tasks/LocalTaskWorker.test.ts | 17 +++++++---- .../tasks/PluginTaskSchedulerJanitor.test.ts | 7 ++--- .../src/tasks/TaskScheduler.test.ts | 4 +++ .../src/tasks/TaskWorker.test.ts | 8 ++++-- .../__testUtils__/createTestScopedSignal.ts | 28 +++++++++++++++++++ 5 files changed, 51 insertions(+), 13 deletions(-) create mode 100644 packages/backend-tasks/src/tasks/__testUtils__/createTestScopedSignal.ts diff --git a/packages/backend-tasks/src/tasks/LocalTaskWorker.test.ts b/packages/backend-tasks/src/tasks/LocalTaskWorker.test.ts index c46743ae59..4fd4949478 100644 --- a/packages/backend-tasks/src/tasks/LocalTaskWorker.test.ts +++ b/packages/backend-tasks/src/tasks/LocalTaskWorker.test.ts @@ -82,14 +82,18 @@ describe('LocalTaskWorker', () => { it('can trigger to abort wait', async () => { const fn = jest.fn(); + const controller = new AbortController(); const worker = new LocalTaskWorker('a', fn, logger); - worker.start({ - version: 2, - initialDelayDuration: 'PT0.2S', - cadence: 'PT0.2S', - timeoutAfterDuration: 'PT1S', - }); + worker.start( + { + version: 2, + initialDelayDuration: 'PT0.2S', + cadence: 'PT0.2S', + timeoutAfterDuration: 'PT1S', + }, + { signal: controller.signal }, + ); // TODO(freben): Rewrite to fake timers - tried, but it wouldn't work expect(fn).toHaveBeenCalledTimes(0); @@ -100,5 +104,6 @@ describe('LocalTaskWorker', () => { worker.trigger(); await new Promise(r => setTimeout(r, 10)); expect(fn).toHaveBeenCalledTimes(2); + controller.abort(); }); }); diff --git a/packages/backend-tasks/src/tasks/PluginTaskSchedulerJanitor.test.ts b/packages/backend-tasks/src/tasks/PluginTaskSchedulerJanitor.test.ts index efbad2e8b6..50bea27d27 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskSchedulerJanitor.test.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskSchedulerJanitor.test.ts @@ -22,6 +22,7 @@ import waitForExpect from 'wait-for-expect'; import { migrateBackendTasks } from '../database/migrateBackendTasks'; import { DbTasksRow, DB_TASKS_TABLE } from '../database/tables'; import { PluginTaskSchedulerJanitor } from './PluginTaskSchedulerJanitor'; +import { createTestScopedSignal } from './__testUtils__/createTestScopedSignal'; const insertTask = async (knex: Knex, task: DbTasksRow) => { return knex(DB_TASKS_TABLE) @@ -45,6 +46,7 @@ describe('PluginTaskSchedulerJanitor', () => { 'MYSQL_8', ], }); + const testScopedSignal = createTestScopedSignal(); jest.setTimeout(60_000); @@ -77,8 +79,7 @@ describe('PluginTaskSchedulerJanitor', () => { logger, }); - const abortController = new AbortController(); - worker.start(abortController.signal); + worker.start(testScopedSignal()); await waitForExpect(async () => { await expect(getTask(knex)).resolves.toEqual( @@ -90,8 +91,6 @@ describe('PluginTaskSchedulerJanitor', () => { }), ); }); - - abortController.abort(); }, ); }); diff --git a/packages/backend-tasks/src/tasks/TaskScheduler.test.ts b/packages/backend-tasks/src/tasks/TaskScheduler.test.ts index 6aa77dd7fe..8e419437bc 100644 --- a/packages/backend-tasks/src/tasks/TaskScheduler.test.ts +++ b/packages/backend-tasks/src/tasks/TaskScheduler.test.ts @@ -19,6 +19,7 @@ import { TestDatabaseId, TestDatabases } from '@backstage/backend-test-utils'; import { Duration } from 'luxon'; import waitForExpect from 'wait-for-expect'; import { TaskScheduler } from './TaskScheduler'; +import { createTestScopedSignal } from './__testUtils__/createTestScopedSignal'; jest.setTimeout(60_000); @@ -27,6 +28,7 @@ describe('TaskScheduler', () => { const databases = TestDatabases.create({ ids: ['POSTGRES_13', 'POSTGRES_9', 'SQLITE_3', 'MYSQL_8'], }); + const testScopedSignal = createTestScopedSignal(); async function createDatabase( databaseId: TestDatabaseId, @@ -51,6 +53,7 @@ describe('TaskScheduler', () => { id: 'task1', timeout: Duration.fromMillis(5000), frequency: Duration.fromMillis(5000), + signal: testScopedSignal(), fn, }); @@ -71,6 +74,7 @@ describe('TaskScheduler', () => { id: 'task2', timeout: Duration.fromMillis(5000), frequency: { cron: '* * * * * *' }, + signal: testScopedSignal(), fn, }); diff --git a/packages/backend-tasks/src/tasks/TaskWorker.test.ts b/packages/backend-tasks/src/tasks/TaskWorker.test.ts index 71d02274e2..3022f59060 100644 --- a/packages/backend-tasks/src/tasks/TaskWorker.test.ts +++ b/packages/backend-tasks/src/tasks/TaskWorker.test.ts @@ -22,6 +22,7 @@ import { migrateBackendTasks } from '../database/migrateBackendTasks'; import { DbTasksRow, DB_TASKS_TABLE } from '../database/tables'; import { TaskWorker } from './TaskWorker'; import { TaskSettingsV2 } from './types'; +import { createTestScopedSignal } from './__testUtils__/createTestScopedSignal'; jest.setTimeout(60_000); @@ -30,6 +31,7 @@ describe('TaskWorker', () => { const databases = TestDatabases.create({ ids: ['POSTGRES_13', 'POSTGRES_9', 'SQLITE_3', 'MYSQL_8'], }); + const testScopedSignal = createTestScopedSignal(); beforeEach(() => { jest.resetAllMocks(); @@ -135,7 +137,7 @@ describe('TaskWorker', () => { }; const checkFrequency = Duration.fromObject({ milliseconds: 100 }); const worker = new TaskWorker('task1', fn, knex, logger, checkFrequency); - worker.start(settings); + worker.start(settings, { signal: testScopedSignal() }); await waitForExpect(() => { expect(logger.error).toHaveBeenCalled(); @@ -158,7 +160,7 @@ describe('TaskWorker', () => { }; const checkFrequency = Duration.fromObject({ milliseconds: 100 }); const worker = new TaskWorker('task1', fn, knex, logger, checkFrequency); - worker.start(settings); + worker.start(settings, { signal: testScopedSignal() }); await waitForExpect(() => { expect(fn).toHaveBeenCalledTimes(3); @@ -321,7 +323,7 @@ describe('TaskWorker', () => { logger, Duration.fromMillis(10), ); - await worker2.start(settings); + await worker2.start(settings, { signal: testScopedSignal() }); // We eventually abort the first worker just to make sure that the second // one for sure will get a go at running the task diff --git a/packages/backend-tasks/src/tasks/__testUtils__/createTestScopedSignal.ts b/packages/backend-tasks/src/tasks/__testUtils__/createTestScopedSignal.ts new file mode 100644 index 0000000000..6a497ea266 --- /dev/null +++ b/packages/backend-tasks/src/tasks/__testUtils__/createTestScopedSignal.ts @@ -0,0 +1,28 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export function createTestScopedSignal(): () => AbortSignal { + let testAbortController = new AbortController(); + + beforeEach(() => { + testAbortController = new AbortController(); + }); + afterEach(() => { + testAbortController.abort(); + }); + + return () => testAbortController.signal; +}