From de55fdb625542148bee0eb6f41682495693226bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 30 Jun 2022 13:43:50 +0200 Subject: [PATCH 1/7] :test_tube: Truncate mapping tables --- .../integration/publicApi/workflows.test.ts | 4 +- .../cli/test/integration/shared/constants.ts | 18 ++++ .../cli/test/integration/shared/testDb.ts | 97 +++++++++++++++---- .../cli/test/integration/shared/types.d.ts | 3 + 4 files changed, 103 insertions(+), 19 deletions(-) diff --git a/packages/cli/test/integration/publicApi/workflows.test.ts b/packages/cli/test/integration/publicApi/workflows.test.ts index 17736af83..98875c138 100644 --- a/packages/cli/test/integration/publicApi/workflows.test.ts +++ b/packages/cli/test/integration/publicApi/workflows.test.ts @@ -35,14 +35,14 @@ beforeAll(async () => { utils.initTestTelemetry(); utils.initTestLogger(); + utils.initConfigFile(); await utils.initNodeTypes(); - await utils.initConfigFile(); workflowRunner = await utils.initActiveWorkflowRunner(); }); beforeEach(async () => { await testDb.truncate( - ['SharedCredentials', 'SharedWorkflow', 'User', 'Workflow', 'Credentials'], + ['SharedCredentials', 'SharedWorkflow', 'User', 'Workflow', 'Credentials', 'Tag'], testDbName, ); diff --git a/packages/cli/test/integration/shared/constants.ts b/packages/cli/test/integration/shared/constants.ts index d7e165ab4..33189089b 100644 --- a/packages/cli/test/integration/shared/constants.ts +++ b/packages/cli/test/integration/shared/constants.ts @@ -48,6 +48,17 @@ export const ROUTES_REQUIRING_AUTHORIZATION: Readonly = [ 'POST /owner/skip-setup', ]; +/** + * Mapping tables link entities but, unlike `SharedWorkflow` and `SharedCredentials`, + * have no entity representation. Therefore, mapping tables must be cleared + * on truncation of any of the collections they link. + */ +export const MAPPING_TABLES_TO_CLEAR: Record = { + Workflow: ['workflows_tags'], + Tag: ['workflows_tags'], +}; + + /** * Name of the connection used for creating and dropping a Postgres DB * for each suite test run. @@ -64,3 +75,10 @@ export const BOOTSTRAP_MYSQL_CONNECTION_NAME: Readonly = 'n8n_bs_mysql'; * Timeout (in milliseconds) to account for fake SMTP service being slow to respond. */ export const SMTP_TEST_TIMEOUT = 30_000; + +/** + * Mapping tables having no entity representation. + */ +export const MAPPING_TABLES = { + WorkflowsTags: 'workflows_tags', +} as const; diff --git a/packages/cli/test/integration/shared/testDb.ts b/packages/cli/test/integration/shared/testDb.ts index e34496123..16d4b4f91 100644 --- a/packages/cli/test/integration/shared/testDb.ts +++ b/packages/cli/test/integration/shared/testDb.ts @@ -5,8 +5,13 @@ import { createConnection, getConnection, ConnectionOptions, Connection } from ' import { Credentials, UserSettings } from 'n8n-core'; import config from '../../../config'; -import { BOOTSTRAP_MYSQL_CONNECTION_NAME, BOOTSTRAP_POSTGRES_CONNECTION_NAME } from './constants'; -import { Db, ICredentialsDb, IDatabaseCollections } from '../../../src'; +import { + BOOTSTRAP_MYSQL_CONNECTION_NAME, + BOOTSTRAP_POSTGRES_CONNECTION_NAME, + MAPPING_TABLES, + MAPPING_TABLES_TO_CLEAR, +} from './constants'; +import { DatabaseType, Db, ICredentialsDb } from '../../../src'; import { randomApiKey, randomEmail, randomName, randomString, randomValidPassword } from './random'; import { CredentialsEntity } from '../../../src/databases/entities/CredentialsEntity'; import { hashPassword } from '../../../src/UserManagement/UserManagementHelper'; @@ -19,7 +24,7 @@ import { createCredentiasFromCredentialsEntity } from '../../../src/CredentialsH import type { Role } from '../../../src/databases/entities/Role'; import { User } from '../../../src/databases/entities/User'; -import type { CollectionName, CredentialPayload } from './types'; +import type { CollectionName, CredentialPayload, MappingName } from './types'; import { WorkflowEntity } from '../../../src/databases/entities/WorkflowEntity'; import { ExecutionEntity } from '../../../src/databases/entities/ExecutionEntity'; import { TagEntity } from '../../../src/databases/entities/TagEntity'; @@ -127,32 +132,84 @@ export async function terminate(testDbName: string) { } } +async function truncateMappingTables( + dbType: DatabaseType, + collections: Array, + testDb: Connection, +) { + const mappingTables = collections.reduce((acc, collection) => { + const found = MAPPING_TABLES_TO_CLEAR[collection]; + + if (found) acc.push(...found); + + return acc; + }, []); + + if (dbType === 'sqlite') { + const promises = mappingTables.map((tableName) => + testDb.query(`DELETE FROM ${tableName}; DELETE FROM sqlite_sequence WHERE name=${tableName};`), + ); + + return Promise.all(promises); + } + + if (dbType === 'postgresdb') { + const promises = mappingTables.map((tableName) => { + const schema = config.getEnv('database.postgresdb.schema'); + const fullTableName = `${schema}.${tableName}`; + testDb.query(`TRUNCATE TABLE ${fullTableName} RESTART IDENTITY CASCADE;`); + }); + + return Promise.all(promises); + } + + // mysqldb, mariadb + + const promises = mappingTables.map((tableName) => + testDb.query(`DELETE FROM ${tableName}; ALTER TABLE ${tableName} AUTO_INCREMENT = 1;`), + ); + + return Promise.all(promises); +} + /** - * Truncate DB tables for collections. + * Truncate specific DB tables in a test DB. * * @param collections Array of entity names whose tables to truncate. * @param testDbName Name of the test DB to truncate tables in. */ -export async function truncate(collections: CollectionName[], testDbName: string) { +export async function truncate(collections: Array, testDbName: string) { const dbType = config.getEnv('database.type'); - const testDb = getConnection(testDbName); if (dbType === 'sqlite') { await testDb.query('PRAGMA foreign_keys=OFF'); - await Promise.all(collections.map((collection) => Db.collections[collection].clear())); + + const truncationPromises = collections.map((collection) => { + const tableName = toTableName(collection); + return testDb.query( + `DELETE FROM ${tableName}; DELETE FROM sqlite_sequence WHERE name=${tableName};`, + ); + }); + + truncationPromises.push(truncateMappingTables(dbType, collections, testDb)); + + await Promise.all(truncationPromises); + return testDb.query('PRAGMA foreign_keys=ON'); } if (dbType === 'postgresdb') { - return Promise.all( - collections.map((collection) => { - const schema = config.getEnv('database.postgresdb.schema'); - const fullTableName = `${schema}.${toTableName(collection)}`; + const truncationPromises = collections.map((collection) => { + const schema = config.getEnv('database.postgresdb.schema'); + const fullTableName = `${schema}.${toTableName(collection)}`; - testDb.query(`TRUNCATE TABLE ${fullTableName} RESTART IDENTITY CASCADE;`); - }), - ); + return testDb.query(`TRUNCATE TABLE ${fullTableName} RESTART IDENTITY CASCADE;`); + }); + + truncationPromises.push(truncateMappingTables(dbType, collections, testDb)); + + return Promise.all(truncationPromises); } /** @@ -167,11 +224,17 @@ export async function truncate(collections: CollectionName[], testDbName: string ); await truncateMySql(testDb, isShared); + await truncateMappingTables(dbType, collections, testDb); await truncateMySql(testDb, isNotShared); } } -function toTableName(collectionName: CollectionName) { +const isMapping = (collection: string): collection is MappingName => + Object.keys(MAPPING_TABLES).includes(collection); + +function toTableName(sourceName: CollectionName | MappingName) { + if (isMapping(sourceName)) return MAPPING_TABLES[sourceName]; + return { Credentials: 'credentials_entity', Workflow: 'workflow_entity', @@ -183,10 +246,10 @@ function toTableName(collectionName: CollectionName) { SharedCredentials: 'shared_credentials', SharedWorkflow: 'shared_workflow', Settings: 'settings', - }[collectionName]; + }[sourceName]; } -function truncateMySql(connection: Connection, collections: Array) { +function truncateMySql(connection: Connection, collections: CollectionName[]) { return Promise.all( collections.map(async (collection) => { const tableName = toTableName(collection); diff --git a/packages/cli/test/integration/shared/types.d.ts b/packages/cli/test/integration/shared/types.d.ts index c16a44d29..a711c4b03 100644 --- a/packages/cli/test/integration/shared/types.d.ts +++ b/packages/cli/test/integration/shared/types.d.ts @@ -2,9 +2,12 @@ import type { ICredentialDataDecryptedObject, ICredentialNodeAccess } from 'n8n- import type { ICredentialsDb, IDatabaseCollections } from '../../../src'; import type { CredentialsEntity } from '../../../src/databases/entities/CredentialsEntity'; import type { User } from '../../../src/databases/entities/User'; +import { MAPPING_TABLES } from './constants'; export type CollectionName = keyof IDatabaseCollections; +export type MappingName = keyof typeof MAPPING_TABLES; + export type SmtpTestAccount = { user: string; pass: string; From e6347e34caefad45bdf6f3941ecf993396e95414 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 30 Jun 2022 14:06:16 +0200 Subject: [PATCH 2/7] :test_tube: Add DB init timeout --- packages/cli/test/integration/shared/constants.ts | 5 +++++ packages/cli/test/integration/shared/testDb.ts | 11 ++++++++++- packages/cli/test/setup.ts | 7 +++++-- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/packages/cli/test/integration/shared/constants.ts b/packages/cli/test/integration/shared/constants.ts index 33189089b..d68ef00e1 100644 --- a/packages/cli/test/integration/shared/constants.ts +++ b/packages/cli/test/integration/shared/constants.ts @@ -76,6 +76,11 @@ export const BOOTSTRAP_MYSQL_CONNECTION_NAME: Readonly = 'n8n_bs_mysql'; */ export const SMTP_TEST_TIMEOUT = 30_000; +/** + * Timeout (in milliseconds) to account for DB being slow to initialize. + */ +export const DB_INITIALIZATION_TIMEOUT = 30_000; + /** * Mapping tables having no entity representation. */ diff --git a/packages/cli/test/integration/shared/testDb.ts b/packages/cli/test/integration/shared/testDb.ts index 16d4b4f91..552aca823 100644 --- a/packages/cli/test/integration/shared/testDb.ts +++ b/packages/cli/test/integration/shared/testDb.ts @@ -8,6 +8,7 @@ import config from '../../../config'; import { BOOTSTRAP_MYSQL_CONNECTION_NAME, BOOTSTRAP_POSTGRES_CONNECTION_NAME, + DB_INITIALIZATION_TIMEOUT, MAPPING_TABLES, MAPPING_TABLES_TO_CLEAR, } from './constants'; @@ -38,6 +39,8 @@ export async function init() { const dbType = config.getEnv('database.type'); if (dbType === 'sqlite') { + jest.setTimeout(DB_INITIALIZATION_TIMEOUT); + // no bootstrap connection required const testDbName = `n8n_test_sqlite_${randomString(6, 10)}_${Date.now()}`; await Db.init(getSqliteOptions({ name: testDbName })); @@ -47,6 +50,8 @@ export async function init() { } if (dbType === 'postgresdb') { + jest.setTimeout(DB_INITIALIZATION_TIMEOUT); + let bootstrapPostgres; const pgOptions = getBootstrapPostgresOptions(); @@ -92,6 +97,8 @@ export async function init() { } if (dbType === 'mysqldb') { + // initialization timeout in test/setup.ts + const bootstrapMysql = await createConnection(getBootstrapMySqlOptions()); const testDbName = `mysql_${randomString(6, 10)}_${Date.now()}_n8n_test`; @@ -147,7 +154,9 @@ async function truncateMappingTables( if (dbType === 'sqlite') { const promises = mappingTables.map((tableName) => - testDb.query(`DELETE FROM ${tableName}; DELETE FROM sqlite_sequence WHERE name=${tableName};`), + testDb.query( + `DELETE FROM ${tableName}; DELETE FROM sqlite_sequence WHERE name=${tableName};`, + ), ); return Promise.all(promises); diff --git a/packages/cli/test/setup.ts b/packages/cli/test/setup.ts index 1180a082c..e0af9209d 100644 --- a/packages/cli/test/setup.ts +++ b/packages/cli/test/setup.ts @@ -2,7 +2,10 @@ import { exec as callbackExec } from 'child_process'; import { promisify } from 'util'; import config from '../config'; -import { BOOTSTRAP_MYSQL_CONNECTION_NAME } from './integration/shared/constants'; +import { + BOOTSTRAP_MYSQL_CONNECTION_NAME, + DB_INITIALIZATION_TIMEOUT, +} from './integration/shared/constants'; const exec = promisify(callbackExec); @@ -17,7 +20,7 @@ if (dbType === 'mysqldb') { (async () => { try { - jest.setTimeout(30000); // 30 seconds for DB initialization + jest.setTimeout(DB_INITIALIZATION_TIMEOUT); await exec( `echo "CREATE DATABASE IF NOT EXISTS ${BOOTSTRAP_MYSQL_CONNECTION_NAME}" | mysql -h ${host} -u ${username} ${passwordSegment}; USE ${BOOTSTRAP_MYSQL_CONNECTION_NAME};`, ); From 945eb63878ffaceb7b0624858edda1ae51beab70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 30 Jun 2022 14:12:47 +0200 Subject: [PATCH 3/7] :test_tube: Separate statements when clearing mappings in MySQL --- packages/cli/test/integration/shared/testDb.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/cli/test/integration/shared/testDb.ts b/packages/cli/test/integration/shared/testDb.ts index 552aca823..86171bacc 100644 --- a/packages/cli/test/integration/shared/testDb.ts +++ b/packages/cli/test/integration/shared/testDb.ts @@ -174,9 +174,10 @@ async function truncateMappingTables( // mysqldb, mariadb - const promises = mappingTables.map((tableName) => - testDb.query(`DELETE FROM ${tableName}; ALTER TABLE ${tableName} AUTO_INCREMENT = 1;`), - ); + const promises = mappingTables.flatMap((tableName) => [ + testDb.query(`DELETE FROM ${tableName};`), + testDb.query(`ALTER TABLE ${tableName} AUTO_INCREMENT = 1;`), + ]); return Promise.all(promises); } From 4ae8b54dc26e8e82e6344a23e71bd9d8e5c8139e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 30 Jun 2022 15:05:23 +0200 Subject: [PATCH 4/7] :test_tube: Hide logging during tests --- packages/cli/config/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/cli/config/index.ts b/packages/cli/config/index.ts index 01cb5e784..a994cd742 100644 --- a/packages/cli/config/index.ts +++ b/packages/cli/config/index.ts @@ -14,7 +14,9 @@ config.getEnv = config.get; // optional configuration files if (process.env.N8N_CONFIG_FILES !== undefined) { const configFiles = process.env.N8N_CONFIG_FILES.split(','); - console.log(`\nLoading configuration overwrites from:\n - ${configFiles.join('\n - ')}\n`); + if (process.env.NODE_ENV !== 'test') { + console.log(`\nLoading configuration overwrites from:\n - ${configFiles.join('\n - ')}\n`); + } config.loadFile(configFiles); } From a2c599d98d40d9076b48d116f97f70185d3ab04e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 30 Jun 2022 15:53:13 +0200 Subject: [PATCH 5/7] :test_tube: Make PG truncation sequential --- .../cli/test/integration/shared/testDb.ts | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/cli/test/integration/shared/testDb.ts b/packages/cli/test/integration/shared/testDb.ts index 86171bacc..3d7cc6140 100644 --- a/packages/cli/test/integration/shared/testDb.ts +++ b/packages/cli/test/integration/shared/testDb.ts @@ -2,7 +2,7 @@ import { exec as callbackExec } from 'child_process'; import { promisify } from 'util'; import { createConnection, getConnection, ConnectionOptions, Connection } from 'typeorm'; -import { Credentials, UserSettings } from 'n8n-core'; +import { UserSettings } from 'n8n-core'; import config from '../../../config'; import { @@ -163,13 +163,15 @@ async function truncateMappingTables( } if (dbType === 'postgresdb') { - const promises = mappingTables.map((tableName) => { - const schema = config.getEnv('database.postgresdb.schema'); - const fullTableName = `${schema}.${tableName}`; - testDb.query(`TRUNCATE TABLE ${fullTableName} RESTART IDENTITY CASCADE;`); - }); + const schema = config.getEnv('database.postgresdb.schema'); - return Promise.all(promises); + // `TRUNCATE` in postgres cannot be parallelized + for (const tableName of mappingTables) { + const fullTableName = `${schema}.${tableName}`; + await testDb.query(`TRUNCATE TABLE ${fullTableName} RESTART IDENTITY CASCADE;`); + } + + return Promise.resolve([]); } // mysqldb, mariadb @@ -210,16 +212,16 @@ export async function truncate(collections: Array, testDbName: s } if (dbType === 'postgresdb') { - const truncationPromises = collections.map((collection) => { - const schema = config.getEnv('database.postgresdb.schema'); + const schema = config.getEnv('database.postgresdb.schema'); + + // `TRUNCATE` in postgres cannot be parallelized + for (const collection of collections) { const fullTableName = `${schema}.${toTableName(collection)}`; + await testDb.query(`TRUNCATE TABLE ${fullTableName} RESTART IDENTITY CASCADE;`); + } - return testDb.query(`TRUNCATE TABLE ${fullTableName} RESTART IDENTITY CASCADE;`); - }); - - truncationPromises.push(truncateMappingTables(dbType, collections, testDb)); - - return Promise.all(truncationPromises); + return await truncateMappingTables(dbType, collections, testDb); + // return Promise.resolve([]) } /** From f8eb5aa52ae5910b687bca09fc4feb6c60891915 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 30 Jun 2022 15:53:43 +0200 Subject: [PATCH 6/7] :test_tube: Add missing SMTP timeout --- packages/cli/test/integration/passwordReset.api.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/test/integration/passwordReset.api.test.ts b/packages/cli/test/integration/passwordReset.api.test.ts index deaf2f04f..4b6a4256c 100644 --- a/packages/cli/test/integration/passwordReset.api.test.ts +++ b/packages/cli/test/integration/passwordReset.api.test.ts @@ -35,7 +35,7 @@ beforeAll(async () => { utils.initTestLogger(); isSmtpAvailable = await utils.isTestSmtpServiceAvailable(); -}); +}, SMTP_TEST_TIMEOUT); beforeEach(async () => { await testDb.truncate(['User'], testDbName); From 70a1aa53f7a41f187d55375eb663b2c4fffd76cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 30 Jun 2022 15:53:56 +0200 Subject: [PATCH 7/7] :art: Formatting fixes --- .../integration/publicApi/workflows.test.ts | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/packages/cli/test/integration/publicApi/workflows.test.ts b/packages/cli/test/integration/publicApi/workflows.test.ts index 98875c138..e676b9c16 100644 --- a/packages/cli/test/integration/publicApi/workflows.test.ts +++ b/packages/cli/test/integration/publicApi/workflows.test.ts @@ -23,11 +23,8 @@ beforeAll(async () => { const initResult = await testDb.init(); testDbName = initResult.testDbName; - const [ - fetchedGlobalOwnerRole, - fetchedGlobalMemberRole, - fetchedWorkflowOwnerRole, - ] = await testDb.getAllRoles(); + const [fetchedGlobalOwnerRole, fetchedGlobalMemberRole, fetchedWorkflowOwnerRole] = + await testDb.getAllRoles(); globalOwnerRole = fetchedGlobalOwnerRole; globalMemberRole = fetchedGlobalMemberRole; @@ -41,10 +38,7 @@ beforeAll(async () => { }); beforeEach(async () => { - await testDb.truncate( - ['SharedCredentials', 'SharedWorkflow', 'User', 'Workflow', 'Credentials', 'Tag'], - testDbName, - ); + await testDb.truncate(['SharedWorkflow', 'User', 'Workflow'], testDbName); config.set('userManagement.disabled', false); config.set('userManagement.isInstanceOwnerSetUp', true); @@ -384,7 +378,7 @@ test('GET /workflows/:id should fail due to invalid API Key', async () => { expect(response.statusCode).toBe(401); }); -test('GET /workflows/:id should fail due to non existing workflow', async () => { +test('GET /workflows/:id should fail due to non-existing workflow', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const authOwnerAgent = utils.createAgent(app, { @@ -495,7 +489,7 @@ test('DELETE /workflows/:id should fail due to invalid API Key', async () => { expect(response.statusCode).toBe(401); }); -test('DELETE /workflows/:id should fail due to non existing workflow', async () => { +test('DELETE /workflows/:id should fail due to non-existing workflow', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const authOwnerAgent = utils.createAgent(app, { @@ -619,7 +613,7 @@ test('POST /workflows/:id/activate should fail due to invalid API Key', async () expect(response.statusCode).toBe(401); }); -test('POST /workflows/:id/activate should fail due to non existing workflow', async () => { +test('POST /workflows/:id/activate should fail due to non-existing workflow', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const authOwnerAgent = utils.createAgent(app, { @@ -781,7 +775,7 @@ test('POST /workflows/:id/deactivate should fail due to invalid API Key', async expect(response.statusCode).toBe(401); }); -test('POST /workflows/:id/deactivate should fail due to non existing workflow', async () => { +test('POST /workflows/:id/deactivate should fail due to non-existing workflow', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const authOwnerAgent = utils.createAgent(app, { @@ -1036,7 +1030,7 @@ test('PUT /workflows/:id should fail due to invalid API Key', async () => { expect(response.statusCode).toBe(401); }); -test('PUT /workflows/:id should fail due to non existing workflow', async () => { +test('PUT /workflows/:id should fail due to non-existing workflow', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole, apiKey: randomApiKey() }); const authOwnerAgent = utils.createAgent(app, {