From 80c808046ada65e4502aee6c592cb8850f4f7aac Mon Sep 17 00:00:00 2001 From: Michael Barrett Date: Thu, 23 Jan 2025 21:12:11 +0000 Subject: [PATCH] Improved performance of integration tests (#283) no refs Improved performance of integration tests by stubbing `generateKeyPair` as it is a slow operation --- .../account.service.integration.test.ts | 406 +++++++++--------- src/site/site.service.integration.test.ts | 20 +- 2 files changed, 219 insertions(+), 207 deletions(-) diff --git a/src/account/account.service.integration.test.ts b/src/account/account.service.integration.test.ts index a1c6e4f4..025b759d 100644 --- a/src/account/account.service.integration.test.ts +++ b/src/account/account.service.integration.test.ts @@ -1,4 +1,6 @@ -import { beforeEach, describe, expect, it } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import EventEmitter from 'node:events'; import { ACTOR_DEFAULT_ICON, @@ -11,11 +13,23 @@ import { TABLE_USERS, } from '../constants'; import { client as db } from '../db'; - -import EventEmitter from 'node:events'; import { AccountService } from './account.service'; import type { Account, ExternalAccountData, Site } from './types'; +vi.mock('@fedify/fedify', async () => { + // generateCryptoKeyPair is a slow operation so we generate a key pair + // upfront and re-use it for all tests + const original = await vi.importActual('@fedify/fedify'); + + // @ts-expect-error - generateCryptoKeyPair is not typed + const keyPair = await original.generateCryptoKeyPair(); + + return { + ...original, + generateCryptoKeyPair: vi.fn().mockReturnValue(keyPair), + }; +}); + describe('AccountService', () => { let service: AccountService; let events: EventEmitter; @@ -49,61 +63,54 @@ describe('AccountService', () => { }); describe('createInternalAccount', () => { - it( - 'should create an internal account', - async () => { - const username = 'foobarbaz'; - - const expectedAccount = { - name: ACTOR_DEFAULT_NAME, - username, - bio: ACTOR_DEFAULT_SUMMARY, - avatar_url: ACTOR_DEFAULT_ICON, - url: `https://${site.host}`, - custom_fields: null, - ap_id: `https://${site.host}${AP_BASE_PATH}/users/${username}`, - ap_inbox_url: `https://${site.host}${AP_BASE_PATH}/inbox/${username}`, - ap_outbox_url: `https://${site.host}${AP_BASE_PATH}/outbox/${username}`, - ap_following_url: `https://${site.host}${AP_BASE_PATH}/following/${username}`, - ap_followers_url: `https://${site.host}${AP_BASE_PATH}/followers/${username}`, - ap_liked_url: `https://${site.host}${AP_BASE_PATH}/liked/${username}`, - ap_shared_inbox_url: null, - }; - - const account = await service.createInternalAccount( - site, - username, - ); - - // Assert the created account was returned - expect(account).toMatchObject(expectedAccount); - expect(account.id).toBeGreaterThan(0); - expect(account.ap_public_key).toBeDefined(); - expect(account.ap_public_key).toContain('key_ops'); - expect(account.ap_private_key).toBeDefined(); - expect(account.ap_private_key).toContain('key_ops'); - - // Assert the account was inserted into the database - const accounts = await db(TABLE_ACCOUNTS).select('*'); - - expect(accounts).toHaveLength(1); - - const dbAccount = accounts[0]; - - expect(dbAccount).toMatchObject(expectedAccount); - - // Assert the user was inserted into the database - const users = await db(TABLE_USERS).select('*'); - - expect(users).toHaveLength(1); - - const dbUser = users[0]; - - expect(dbUser.account_id).toBe(account.id); - expect(dbUser.site_id).toBe(site.id); - }, - 1000 * 10, // Increase timeout to 10 seconds as 5 seconds seems to be too short on CI - ); + it('should create an internal account', async () => { + const username = 'foobarbaz'; + + const expectedAccount = { + name: ACTOR_DEFAULT_NAME, + username, + bio: ACTOR_DEFAULT_SUMMARY, + avatar_url: ACTOR_DEFAULT_ICON, + url: `https://${site.host}`, + custom_fields: null, + ap_id: `https://${site.host}${AP_BASE_PATH}/users/${username}`, + ap_inbox_url: `https://${site.host}${AP_BASE_PATH}/inbox/${username}`, + ap_outbox_url: `https://${site.host}${AP_BASE_PATH}/outbox/${username}`, + ap_following_url: `https://${site.host}${AP_BASE_PATH}/following/${username}`, + ap_followers_url: `https://${site.host}${AP_BASE_PATH}/followers/${username}`, + ap_liked_url: `https://${site.host}${AP_BASE_PATH}/liked/${username}`, + ap_shared_inbox_url: null, + }; + + const account = await service.createInternalAccount(site, username); + + // Assert the created account was returned + expect(account).toMatchObject(expectedAccount); + expect(account.id).toBeGreaterThan(0); + expect(account.ap_public_key).toBeDefined(); + expect(account.ap_public_key).toContain('key_ops'); + expect(account.ap_private_key).toBeDefined(); + expect(account.ap_private_key).toContain('key_ops'); + + // Assert the account was inserted into the database + const accounts = await db(TABLE_ACCOUNTS).select('*'); + + expect(accounts).toHaveLength(1); + + const dbAccount = accounts[0]; + + expect(dbAccount).toMatchObject(expectedAccount); + + // Assert the user was inserted into the database + const users = await db(TABLE_USERS).select('*'); + + expect(users).toHaveLength(1); + + const dbUser = users[0]; + + expect(dbUser.account_id).toBe(account.id); + expect(dbUser.site_id).toBe(site.id); + }); }); describe('createExternalAccount', () => { @@ -251,81 +258,79 @@ describe('AccountService', () => { }); describe('getFollowingAccounts', () => { - it( - 'should retrieve the accounts that an account follows', - async () => { - const account = await service.createInternalAccount( - site, - 'account', - ); - const following1 = await service.createInternalAccount( - site, - 'following1', - ); - const following2 = await service.createInternalAccount( - site, - 'following2', - ); - const following3 = await service.createInternalAccount( - site, - 'following3', - ); - - await service.recordAccountFollow(following1, account); - await service.recordAccountFollow(following2, account); - await service.recordAccountFollow(following3, account); - - // Get a page of following accounts and assert the requested fields are returned - const followingAccounts = await service.getFollowingAccounts( - account, - { - limit: 2, - offset: 0, - fields: ['id', 'username', 'ap_inbox_url'], - }, - ); - - expect(followingAccounts).toHaveLength(2); - expect(followingAccounts[0]).toMatchObject({ - id: following3.id, - username: following3.username, - }); - expect(followingAccounts[0].ap_inbox_url).toBeDefined(); + it('should retrieve the accounts that an account follows', async () => { + const account = await service.createInternalAccount( + site, + 'account', + ); + const following1 = await service.createInternalAccount( + site, + 'following1', + ); + const following2 = await service.createInternalAccount( + site, + 'following2', + ); + const following3 = await service.createInternalAccount( + site, + 'following3', + ); - expect(followingAccounts[1]).toMatchObject({ - id: following2.id, - username: following2.username, - }); - expect(followingAccounts[1].ap_inbox_url).toBeDefined(); - - // Get the next page of following accounts and assert the requested fields are returned - const nextFollowingAccounts = - await service.getFollowingAccounts(account, { - limit: 2, - offset: 2, - fields: ['id', 'username', 'ap_inbox_url'], - }); - - expect(nextFollowingAccounts).toHaveLength(1); - expect(nextFollowingAccounts[0]).toMatchObject({ - id: following1.id, - username: following1.username, + await service.recordAccountFollow(following1, account); + await service.recordAccountFollow(following2, account); + await service.recordAccountFollow(following3, account); + + // Get a page of following accounts and assert the requested fields are returned + const followingAccounts = await service.getFollowingAccounts( + account, + { + limit: 2, + offset: 0, + fields: ['id', 'username', 'ap_inbox_url'], + }, + ); + + expect(followingAccounts).toHaveLength(2); + expect(followingAccounts[0]).toMatchObject({ + id: following3.id, + username: following3.username, + }); + expect(followingAccounts[0].ap_inbox_url).toBeDefined(); + + expect(followingAccounts[1]).toMatchObject({ + id: following2.id, + username: following2.username, + }); + expect(followingAccounts[1].ap_inbox_url).toBeDefined(); + + // Get the next page of following accounts and assert the requested fields are returned + const nextFollowingAccounts = await service.getFollowingAccounts( + account, + { + limit: 2, + offset: 2, + fields: ['id', 'username', 'ap_inbox_url'], + }, + ); + + expect(nextFollowingAccounts).toHaveLength(1); + expect(nextFollowingAccounts[0]).toMatchObject({ + id: following1.id, + username: following1.username, + }); + expect(nextFollowingAccounts[0].ap_inbox_url).toBeDefined(); + + // Get another page that will return no results and assert the + // results are empty + const nextFollowingAccountsEmpty = + await service.getFollowingAccounts(account, { + limit: 2, + offset: 3, + fields: ['id', 'username', 'ap_inbox_url'], }); - expect(nextFollowingAccounts[0].ap_inbox_url).toBeDefined(); - - // Get another page that will return no results and assert the - // results are empty - const nextFollowingAccountsEmpty = - await service.getFollowingAccounts(account, { - limit: 2, - offset: 3, - fields: ['id', 'username', 'ap_inbox_url'], - }); - - expect(nextFollowingAccountsEmpty).toHaveLength(0); - }, - 1000 * 10, // Increase timeout to 10 seconds as 5 seconds seems to be too short on CI - ); + + expect(nextFollowingAccountsEmpty).toHaveLength(0); + }); }); describe('getFollowingAccountsCount', () => { @@ -353,82 +358,75 @@ describe('AccountService', () => { }); describe('getFollowerAccounts', () => { - it( - 'should retrieve the accounts that are following an account', - async () => { - const account = await service.createInternalAccount( - site, - 'account', - ); - const follower1 = await service.createInternalAccount( - site, - 'follower1', - ); - const follower2 = await service.createInternalAccount( - site, - 'follower2', - ); - const follower3 = await service.createInternalAccount( - site, - 'follower3', - ); - - await service.recordAccountFollow(account, follower1); - await service.recordAccountFollow(account, follower2); - await service.recordAccountFollow(account, follower3); - - // Get a page of followers and assert the requested fields are returned - const followers = await service.getFollowerAccounts(account, { + it('should retrieve the accounts that are following an account', async () => { + const account = await service.createInternalAccount( + site, + 'account', + ); + const follower1 = await service.createInternalAccount( + site, + 'follower1', + ); + const follower2 = await service.createInternalAccount( + site, + 'follower2', + ); + const follower3 = await service.createInternalAccount( + site, + 'follower3', + ); + + await service.recordAccountFollow(account, follower1); + await service.recordAccountFollow(account, follower2); + await service.recordAccountFollow(account, follower3); + + // Get a page of followers and assert the requested fields are returned + const followers = await service.getFollowerAccounts(account, { + limit: 2, + offset: 0, + fields: ['id', 'username', 'ap_inbox_url'], + }); + + expect(followers).toHaveLength(2); + expect(followers[0]).toMatchObject({ + id: follower3.id, + username: follower3.username, + }); + expect(followers[0].ap_inbox_url).toBeDefined(); + + expect(followers[1]).toMatchObject({ + id: follower2.id, + username: follower2.username, + }); + expect(followers[1].ap_inbox_url).toBeDefined(); + + // Get the next page of followers and assert the requested fields are returned + const nextFollowers = await service.getFollowerAccounts(account, { + limit: 2, + offset: 2, + fields: ['id', 'username', 'ap_inbox_url'], + }); + + expect(nextFollowers).toHaveLength(1); + expect(nextFollowers[0]).toMatchObject({ + id: follower1.id, + username: follower1.username, + }); + expect(nextFollowers[0].ap_inbox_url).toBeDefined(); + + // Get another page that will return no results and assert the + // results are empty + const nextFollowersEmpty = await service.getFollowerAccounts( + account, + { limit: 2, - offset: 0, + offset: 3, fields: ['id', 'username', 'ap_inbox_url'], - }); - - expect(followers).toHaveLength(2); - expect(followers[0]).toMatchObject({ - id: follower3.id, - username: follower3.username, - }); - expect(followers[0].ap_inbox_url).toBeDefined(); + }, + ); - expect(followers[1]).toMatchObject({ - id: follower2.id, - username: follower2.username, - }); - expect(followers[1].ap_inbox_url).toBeDefined(); - - // Get the next page of followers and assert the requested fields are returned - const nextFollowers = await service.getFollowerAccounts( - account, - { - limit: 2, - offset: 2, - fields: ['id', 'username', 'ap_inbox_url'], - }, - ); - - expect(nextFollowers).toHaveLength(1); - expect(nextFollowers[0]).toMatchObject({ - id: follower1.id, - username: follower1.username, - }); - expect(nextFollowers[0].ap_inbox_url).toBeDefined(); - - // Get another page that will return no results and assert the - // results are empty - const nextFollowersEmpty = await service.getFollowerAccounts( - account, - { - limit: 2, - offset: 3, - fields: ['id', 'username', 'ap_inbox_url'], - }, - ); - - expect(nextFollowersEmpty).toHaveLength(0); - }, - 1000 * 10, // Increase timeout to 10 seconds as 5 seconds seems to be too short on CI - ); + expect(nextFollowersEmpty).toHaveLength(0); + }); }); describe('getFollowerAccountsCount', () => { diff --git a/src/site/site.service.integration.test.ts b/src/site/site.service.integration.test.ts index a6905d0f..5777b4a4 100644 --- a/src/site/site.service.integration.test.ts +++ b/src/site/site.service.integration.test.ts @@ -1,13 +1,27 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { TABLE_ACCOUNTS, TABLE_SITES, TABLE_USERS } from '../constants'; -import { client as db } from '../db'; - import EventEmitter from 'node:events'; + import { AccountService } from '../account/account.service'; import type { Account } from '../account/types'; +import { TABLE_ACCOUNTS, TABLE_SITES, TABLE_USERS } from '../constants'; +import { client as db } from '../db'; import { type IGhostService, type Site, SiteService } from './site.service'; +vi.mock('@fedify/fedify', async () => { + // generateCryptoKeyPair is a slow operation so we generate a key pair + // upfront and re-use it for all tests + const original = await vi.importActual('@fedify/fedify'); + + // @ts-expect-error - generateCryptoKeyPair is not typed + const keyPair = await original.generateCryptoKeyPair(); + + return { + ...original, + generateCryptoKeyPair: vi.fn().mockReturnValue(keyPair), + }; +}); + describe('SiteService', () => { let service: SiteService; let accountService: AccountService;