mirror of
https://github.com/n8n-io/n8n
synced 2026-04-21 15:47:20 +00:00
perf(core): Fix slow user mock in cli tests (no-changelog) (#27205)
This commit is contained in:
parent
d0e7ce08fb
commit
4d40db2c39
6 changed files with 51 additions and 105 deletions
|
|
@ -149,6 +149,7 @@ CSS variables and styling conventions.
|
|||
### Testing Guidelines
|
||||
- **Always work from within the package directory** when running tests
|
||||
- **Mock all external dependencies** in unit tests
|
||||
- **Prefer reusing hoisted shared `mock<T>(...)` fixtures** when a typed mock is immutable and used across tests. This rule exists to avoid massive test slowdowns from repeatedly creating nested proxy mocks while preserving the type contract. Avoid replacing these with `as unknown as T` helpers for entities like `User`.
|
||||
- **Confirm test cases with user** before writing unit tests
|
||||
- **Typecheck is critical before committing** - always run `pnpm typecheck`
|
||||
- **When modifying pinia stores**, check for unused computed properties
|
||||
|
|
|
|||
|
|
@ -32,6 +32,9 @@ import type { RoleService } from '@/services/role.service';
|
|||
|
||||
import { mockExistingCredential } from './credentials.test-data';
|
||||
|
||||
const ownerUser = mock<User>({ id: 'owner-id', role: GLOBAL_OWNER_ROLE });
|
||||
const memberUser = mock<User>({ id: 'member-id', role: GLOBAL_MEMBER_ROLE });
|
||||
|
||||
describe('CredentialsService', () => {
|
||||
const credType = mock<ICredentialType>({
|
||||
extends: [],
|
||||
|
|
@ -591,9 +594,6 @@ describe('CredentialsService', () => {
|
|||
});
|
||||
|
||||
describe('getMany', () => {
|
||||
const ownerUser = mock<User>({ id: 'owner-id', role: GLOBAL_OWNER_ROLE });
|
||||
const memberUser = mock<User>({ id: 'member-id', role: GLOBAL_MEMBER_ROLE });
|
||||
|
||||
const regularCredential = {
|
||||
id: 'cred-1',
|
||||
name: 'Regular Credential',
|
||||
|
|
@ -1479,8 +1479,6 @@ describe('CredentialsService', () => {
|
|||
|
||||
describe('findAllCredentialIdsForWorkflow', () => {
|
||||
const workflowId = 'workflow-1';
|
||||
const ownerUser = mock<User>({ id: 'owner-id', role: GLOBAL_OWNER_ROLE });
|
||||
const memberUser = mock<User>({ id: 'member-id', role: GLOBAL_MEMBER_ROLE });
|
||||
|
||||
it('should return all personal credentials when owner has global read permissions', async () => {
|
||||
// ARRANGE
|
||||
|
|
@ -1539,8 +1537,6 @@ describe('CredentialsService', () => {
|
|||
|
||||
describe('findAllCredentialIdsForProject', () => {
|
||||
const projectId = 'project-1';
|
||||
const ownerUser = mock<User>({ id: 'owner-id', role: GLOBAL_OWNER_ROLE });
|
||||
const memberUser = mock<User>({ id: 'member-id', role: GLOBAL_MEMBER_ROLE });
|
||||
|
||||
it('should return all personal credentials when project owner has global read permissions', async () => {
|
||||
// ARRANGE
|
||||
|
|
@ -1598,9 +1594,6 @@ describe('CredentialsService', () => {
|
|||
});
|
||||
|
||||
describe('createUnmanagedCredential', () => {
|
||||
const ownerUser = mock<User>({ id: 'owner-id', role: GLOBAL_OWNER_ROLE });
|
||||
const memberUser = mock<User>({ id: 'member-id', role: GLOBAL_MEMBER_ROLE });
|
||||
|
||||
const credentialData = {
|
||||
name: 'Test Credential',
|
||||
type: 'apiKey',
|
||||
|
|
@ -1845,8 +1838,6 @@ describe('CredentialsService', () => {
|
|||
});
|
||||
|
||||
describe('createManagedCredential', () => {
|
||||
const ownerUser = mock<User>({ id: 'owner-id', role: GLOBAL_OWNER_ROLE });
|
||||
|
||||
const credentialData = {
|
||||
name: 'Managed Credential',
|
||||
type: 'oauth2',
|
||||
|
|
@ -1928,7 +1919,6 @@ describe('CredentialsService', () => {
|
|||
});
|
||||
|
||||
describe('prepareUpdateData', () => {
|
||||
const ownerUser = mock<User>({ id: 'owner-id', role: GLOBAL_OWNER_ROLE });
|
||||
describe('external secrets', () => {
|
||||
beforeEach(() => {
|
||||
jest.spyOn(service, 'decrypt').mockReturnValue({});
|
||||
|
|
@ -2030,7 +2020,6 @@ describe('CredentialsService', () => {
|
|||
});
|
||||
|
||||
describe('checkCredentialData', () => {
|
||||
const ownerUser = mock<User>({ id: 'owner-id', role: GLOBAL_OWNER_ROLE });
|
||||
const testProjectId = 'test-project-id';
|
||||
|
||||
beforeEach(() => {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,4 @@
|
|||
import { GLOBAL_OWNER_ROLE, GLOBAL_MEMBER_ROLE } from '@n8n/db';
|
||||
import type { User } from '@n8n/db';
|
||||
import { GLOBAL_OWNER_ROLE, GLOBAL_MEMBER_ROLE, type User } from '@n8n/db';
|
||||
import { mock } from 'jest-mock-extended';
|
||||
|
||||
import type { SecretsProviderAccessCheckService } from '@/modules/external-secrets.ee/secret-provider-access-check.service.ee';
|
||||
|
|
@ -12,9 +11,10 @@ import {
|
|||
extractProviderKeys,
|
||||
} from '../validation';
|
||||
|
||||
const ownerUser = mock<User>({ id: 'owner-id', role: GLOBAL_OWNER_ROLE });
|
||||
const memberUser = mock<User>({ id: 'member-id', role: GLOBAL_MEMBER_ROLE });
|
||||
|
||||
describe('Credentials Validation', () => {
|
||||
const ownerUser = mock<User>({ id: 'owner-id', role: GLOBAL_OWNER_ROLE });
|
||||
const memberUser = mock<User>({ id: 'member-id', role: GLOBAL_MEMBER_ROLE });
|
||||
const projectId = 'project-id';
|
||||
const errorMessage = 'Lacking permissions to reference external secrets in credentials';
|
||||
|
||||
|
|
|
|||
|
|
@ -27,6 +27,11 @@ import type { StatusExportableCredential } from '../types/exportable-credential'
|
|||
import type { ExportableProjectWithFileName } from '../types/exportable-project';
|
||||
import type { SourceControlWorkflowVersionId } from '../types/source-control-workflow-version-id';
|
||||
|
||||
// Reuse typed user mocks at module scope to avoid performance issues related to recreating nested proxy mocks per test
|
||||
const globalAdminUser = mock<User>({ role: GLOBAL_ADMIN_ROLE });
|
||||
const globalAdminUserWithId = mock<User>({ id: '1', role: GLOBAL_ADMIN_ROLE });
|
||||
const globalMemberUser = mock<User>({ role: GLOBAL_MEMBER_ROLE });
|
||||
|
||||
describe('getStatus', () => {
|
||||
const sourceControlImportService = mock<SourceControlImportService>();
|
||||
const tagRepository = mock<TagRepository>();
|
||||
|
|
@ -106,9 +111,7 @@ describe('getStatus', () => {
|
|||
|
||||
it('ensure updatedAt field for last deleted tag', async () => {
|
||||
// ARRANGE
|
||||
const user = mock<User>({
|
||||
role: GLOBAL_ADMIN_ROLE,
|
||||
});
|
||||
const user = globalAdminUser;
|
||||
|
||||
// Define a tag that does only exist remotely.
|
||||
// Pushing this means it was deleted.
|
||||
|
|
@ -145,9 +148,7 @@ describe('getStatus', () => {
|
|||
|
||||
it('ensure updatedAt field for last deleted folder', async () => {
|
||||
// ARRANGE
|
||||
const user = mock<User>({
|
||||
role: GLOBAL_ADMIN_ROLE,
|
||||
});
|
||||
const user = globalAdminUser;
|
||||
|
||||
// Define a folder that does only exist remotely.
|
||||
// Pushing this means it was deleted.
|
||||
|
|
@ -186,9 +187,7 @@ describe('getStatus', () => {
|
|||
|
||||
it('conflict depends on the value of `direction`', async () => {
|
||||
// ARRANGE
|
||||
const user = mock<User>({
|
||||
role: GLOBAL_ADMIN_ROLE,
|
||||
});
|
||||
const user = globalAdminUser;
|
||||
|
||||
// Define a credential that does only exist locally.
|
||||
// Pulling this would delete it so it should be marked as a conflict.
|
||||
|
|
@ -334,9 +333,7 @@ describe('getStatus', () => {
|
|||
|
||||
it('should throw `ForbiddenError` if direction is pull and user is not allowed to globally pull', async () => {
|
||||
// ARRANGE
|
||||
const user = mock<User>({
|
||||
role: GLOBAL_MEMBER_ROLE,
|
||||
});
|
||||
const user = globalMemberUser;
|
||||
|
||||
// ACT
|
||||
await expect(
|
||||
|
|
@ -380,12 +377,8 @@ describe('getStatus', () => {
|
|||
};
|
||||
|
||||
const mockUsers = {
|
||||
globalAdmin: mock<User>({
|
||||
role: GLOBAL_ADMIN_ROLE,
|
||||
}),
|
||||
limitedUser: mock<User>({
|
||||
role: GLOBAL_MEMBER_ROLE,
|
||||
}),
|
||||
globalAdmin: globalAdminUser,
|
||||
limitedUser: globalMemberUser,
|
||||
};
|
||||
|
||||
const setupProjectMocks = ({
|
||||
|
|
@ -836,7 +829,7 @@ describe('getStatus', () => {
|
|||
});
|
||||
|
||||
describe('workflows', () => {
|
||||
const user = mock<User>({ role: GLOBAL_ADMIN_ROLE });
|
||||
const user = globalAdminUser;
|
||||
|
||||
const createWorkflow = (
|
||||
overrides: Partial<SourceControlWorkflowVersionId> = {},
|
||||
|
|
@ -1099,7 +1092,7 @@ describe('getStatus', () => {
|
|||
|
||||
describe('credentials', () => {
|
||||
describe('owner changes', () => {
|
||||
const user = mock<User>({ role: GLOBAL_ADMIN_ROLE });
|
||||
const user = globalAdminUser;
|
||||
|
||||
const createCredential = (
|
||||
overrides: Partial<StatusExportableCredential> = {},
|
||||
|
|
@ -1326,7 +1319,7 @@ describe('getStatus', () => {
|
|||
},
|
||||
};
|
||||
|
||||
const user = mock<User>({ role: GLOBAL_ADMIN_ROLE });
|
||||
const user = globalAdminUser;
|
||||
|
||||
it('should detect folder as modified when homeProjectId changes', async () => {
|
||||
// ARRANGE
|
||||
|
|
@ -1447,7 +1440,7 @@ describe('getStatus', () => {
|
|||
});
|
||||
|
||||
describe('owner changes', () => {
|
||||
const user = mock<User>({ role: GLOBAL_ADMIN_ROLE });
|
||||
const user = globalAdminUser;
|
||||
|
||||
const createFolder = (overrides = {}) => ({
|
||||
id: 'folder1',
|
||||
|
|
@ -1688,7 +1681,7 @@ describe('getStatus', () => {
|
|||
});
|
||||
|
||||
describe('tag mappings', () => {
|
||||
const user = mock<User>({ role: GLOBAL_ADMIN_ROLE });
|
||||
const user = globalAdminUser;
|
||||
|
||||
it('should detect when a tag mapping is removed locally but still exists remotely', async () => {
|
||||
const tag = mock<TagEntity>({ id: 'tag1', name: 'Test Tag', updatedAt: new Date() });
|
||||
|
|
@ -1726,10 +1719,7 @@ describe('getStatus', () => {
|
|||
describe('data tables', () => {
|
||||
it('should handle undefined data tables from remote (null safety)', async () => {
|
||||
// ARRANGE
|
||||
const user = mock<User>({
|
||||
id: '1',
|
||||
role: GLOBAL_ADMIN_ROLE,
|
||||
});
|
||||
const user = globalAdminUserWithId;
|
||||
|
||||
// Mock undefined data tables from remote
|
||||
sourceControlImportService.getRemoteDataTablesFromFiles.mockResolvedValue(undefined as any);
|
||||
|
|
@ -1750,10 +1740,7 @@ describe('getStatus', () => {
|
|||
|
||||
it('should handle undefined data tables from local (null safety)', async () => {
|
||||
// ARRANGE
|
||||
const user = mock<User>({
|
||||
id: '1',
|
||||
role: GLOBAL_ADMIN_ROLE,
|
||||
});
|
||||
const user = globalAdminUserWithId;
|
||||
|
||||
// Mock undefined data tables from local
|
||||
sourceControlImportService.getRemoteDataTablesFromFiles.mockResolvedValue([]);
|
||||
|
|
@ -1774,10 +1761,7 @@ describe('getStatus', () => {
|
|||
|
||||
it('should handle both data tables undefined (null safety)', async () => {
|
||||
// ARRANGE
|
||||
const user = mock<User>({
|
||||
id: '1',
|
||||
role: GLOBAL_ADMIN_ROLE,
|
||||
});
|
||||
const user = globalAdminUserWithId;
|
||||
|
||||
// Mock both undefined
|
||||
sourceControlImportService.getRemoteDataTablesFromFiles.mockResolvedValue(undefined as any);
|
||||
|
|
@ -1798,10 +1782,7 @@ describe('getStatus', () => {
|
|||
|
||||
it('should identify data tables missing in local (remote only)', async () => {
|
||||
// ARRANGE
|
||||
const user = mock<User>({
|
||||
id: '1',
|
||||
role: GLOBAL_ADMIN_ROLE,
|
||||
});
|
||||
const user = globalAdminUserWithId;
|
||||
|
||||
const remoteDataTable = {
|
||||
id: 'dt1',
|
||||
|
|
@ -1836,10 +1817,7 @@ describe('getStatus', () => {
|
|||
|
||||
it('should identify data tables missing in remote (local only)', async () => {
|
||||
// ARRANGE
|
||||
const user = mock<User>({
|
||||
id: '1',
|
||||
role: GLOBAL_ADMIN_ROLE,
|
||||
});
|
||||
const user = globalAdminUserWithId;
|
||||
|
||||
const localDataTable = {
|
||||
id: 'dt2',
|
||||
|
|
@ -1874,10 +1852,7 @@ describe('getStatus', () => {
|
|||
|
||||
it('should identify modified data tables (name changed)', async () => {
|
||||
// ARRANGE
|
||||
const user = mock<User>({
|
||||
id: '1',
|
||||
role: GLOBAL_ADMIN_ROLE,
|
||||
});
|
||||
const user = globalAdminUserWithId;
|
||||
|
||||
const localDataTable = {
|
||||
id: 'dt3',
|
||||
|
|
@ -1922,10 +1897,7 @@ describe('getStatus', () => {
|
|||
|
||||
it('should not detect modifications when data tables are identical', async () => {
|
||||
// ARRANGE
|
||||
const user = mock<User>({
|
||||
id: '1',
|
||||
role: GLOBAL_ADMIN_ROLE,
|
||||
});
|
||||
const user = globalAdminUserWithId;
|
||||
|
||||
const dataTable = {
|
||||
id: 'dt4',
|
||||
|
|
@ -1956,10 +1928,7 @@ describe('getStatus', () => {
|
|||
|
||||
it('should handle multiple data tables with mixed states', async () => {
|
||||
// ARRANGE
|
||||
const user = mock<User>({
|
||||
id: '1',
|
||||
role: GLOBAL_ADMIN_ROLE,
|
||||
});
|
||||
const user = globalAdminUserWithId;
|
||||
|
||||
const localDataTables = [
|
||||
{
|
||||
|
|
@ -2055,10 +2024,7 @@ describe('getStatus', () => {
|
|||
});
|
||||
|
||||
describe('schema change detection', () => {
|
||||
const user = mock<User>({
|
||||
id: '1',
|
||||
role: GLOBAL_ADMIN_ROLE,
|
||||
});
|
||||
const user = globalAdminUserWithId;
|
||||
|
||||
it('should detect column addition', async () => {
|
||||
// ARRANGE
|
||||
|
|
|
|||
|
|
@ -32,6 +32,12 @@ jest.mock('../source-control-helper.ee', () => ({
|
|||
sourceControlFoldersExistCheck: jest.fn(() => true),
|
||||
}));
|
||||
|
||||
// Reuse typed user mocks at module scope to avoid performance issues related to recreating nested proxy mocks per test
|
||||
const globalAdminUser = mock<User>({ role: GLOBAL_ADMIN_ROLE });
|
||||
const globalAdminUserWithId = mock<User>({ id: 'user-id', role: GLOBAL_ADMIN_ROLE });
|
||||
const globalMemberUser = mock<User>({ role: GLOBAL_MEMBER_ROLE });
|
||||
const globalMemberUserWithId = mock<User>({ id: 'user-id', role: GLOBAL_MEMBER_ROLE });
|
||||
|
||||
describe('SourceControlService', () => {
|
||||
const preferencesService = new SourceControlPreferencesService(
|
||||
Container.get(InstanceSettings),
|
||||
|
|
@ -549,9 +555,7 @@ describe('SourceControlService', () => {
|
|||
describe('getStatus', () => {
|
||||
it('ensure updatedAt field for last deleted tag', async () => {
|
||||
// ARRANGE
|
||||
const user = mock<User>({
|
||||
role: GLOBAL_ADMIN_ROLE,
|
||||
});
|
||||
const user = globalAdminUser;
|
||||
|
||||
const mockResult = [
|
||||
{
|
||||
|
|
@ -593,9 +597,7 @@ describe('SourceControlService', () => {
|
|||
|
||||
it('ensure updatedAt field for last deleted folder', async () => {
|
||||
// ARRANGE
|
||||
const user = mock<User>({
|
||||
role: GLOBAL_ADMIN_ROLE,
|
||||
});
|
||||
const user = globalAdminUser;
|
||||
|
||||
const mockResult = [
|
||||
{
|
||||
|
|
@ -637,9 +639,7 @@ describe('SourceControlService', () => {
|
|||
|
||||
it('conflict depends on the value of `direction`', async () => {
|
||||
// ARRANGE
|
||||
const user = mock<User>({
|
||||
role: GLOBAL_ADMIN_ROLE,
|
||||
});
|
||||
const user = globalAdminUser;
|
||||
|
||||
const mockPullResult = [
|
||||
{ type: 'workflow', conflict: true },
|
||||
|
|
@ -715,9 +715,7 @@ describe('SourceControlService', () => {
|
|||
|
||||
it('should throw `ForbiddenError` if direction is pull and user is not allowed to globally pull', async () => {
|
||||
// ARRANGE
|
||||
const user = mock<User>({
|
||||
role: GLOBAL_MEMBER_ROLE,
|
||||
});
|
||||
const user = globalMemberUser;
|
||||
|
||||
mockStatusService.getStatus.mockRejectedValue(
|
||||
new ForbiddenError('You do not have permission to pull from source control'),
|
||||
|
|
@ -746,7 +744,7 @@ describe('SourceControlService', () => {
|
|||
'should return file content for $type',
|
||||
async ({ type, id, content }) => {
|
||||
jest.spyOn(gitService, 'getFileContent').mockResolvedValue(content);
|
||||
const user = mock<User>({ id: 'user-id', role: GLOBAL_ADMIN_ROLE });
|
||||
const user = globalAdminUserWithId;
|
||||
|
||||
const result = await sourceControlService.getRemoteFileEntity({ user, type, id });
|
||||
|
||||
|
|
@ -757,7 +755,7 @@ describe('SourceControlService', () => {
|
|||
it.each<SourceControlledFile['type']>(['folders', 'credential', 'tags', 'variables'])(
|
||||
'should throw an error if the file type is not handled',
|
||||
async (type) => {
|
||||
const user = mock<User>({ id: 'user-id', role: GLOBAL_ADMIN_ROLE });
|
||||
const user = globalAdminUserWithId;
|
||||
await expect(
|
||||
sourceControlService.getRemoteFileEntity({ user, type, id: 'unknown' }),
|
||||
).rejects.toThrow(`Unsupported file type: ${type}`);
|
||||
|
|
@ -766,7 +764,7 @@ describe('SourceControlService', () => {
|
|||
|
||||
it('should fail if the git service fails to get the file content', async () => {
|
||||
jest.spyOn(gitService, 'getFileContent').mockRejectedValue(new Error('Git service error'));
|
||||
const user = mock<User>({ id: 'user-id', role: GLOBAL_ADMIN_ROLE });
|
||||
const user = globalAdminUserWithId;
|
||||
|
||||
await expect(
|
||||
sourceControlService.getRemoteFileEntity({ user, type: 'workflow', id: '1234' }),
|
||||
|
|
@ -774,10 +772,7 @@ describe('SourceControlService', () => {
|
|||
});
|
||||
|
||||
it('should throw an error if the user does not have access to the project', async () => {
|
||||
const user = mock<User>({
|
||||
id: 'user-id',
|
||||
role: GLOBAL_MEMBER_ROLE,
|
||||
});
|
||||
const user = globalMemberUserWithId;
|
||||
jest
|
||||
.spyOn(sourceControlScopedService, 'getWorkflowsInAdminProjectsFromContext')
|
||||
.mockResolvedValue([]);
|
||||
|
|
@ -788,7 +783,7 @@ describe('SourceControlService', () => {
|
|||
});
|
||||
|
||||
it('should return content for an authorized workflow', async () => {
|
||||
const user = mock<User>({ id: 'user-id', role: GLOBAL_MEMBER_ROLE });
|
||||
const user = globalMemberUserWithId;
|
||||
jest
|
||||
.spyOn(sourceControlScopedService, 'getWorkflowsInAdminProjectsFromContext')
|
||||
.mockResolvedValue([{ id: '1234' } as WorkflowEntity]);
|
||||
|
|
|
|||
|
|
@ -5,8 +5,8 @@ import {
|
|||
CredentialsRepository,
|
||||
SharedCredentialsRepository,
|
||||
CredentialsEntity,
|
||||
type User,
|
||||
} from '@n8n/db';
|
||||
import type { User } from '@n8n/db';
|
||||
import { Container } from '@n8n/di';
|
||||
import {
|
||||
PROJECT_ADMIN_ROLE_SLUG,
|
||||
|
|
@ -63,13 +63,8 @@ describe('CredentialsFinderService', () => {
|
|||
const credentialsId = 'cred_123';
|
||||
const sharedCredential = mock<SharedCredentials>();
|
||||
sharedCredential.credentials = mock<CredentialsEntity>({ id: credentialsId });
|
||||
const owner = mock<User>({
|
||||
role: GLOBAL_OWNER_ROLE,
|
||||
});
|
||||
const member = mock<User>({
|
||||
role: GLOBAL_MEMBER_ROLE,
|
||||
id: 'test',
|
||||
});
|
||||
const owner = mock<User>({ role: GLOBAL_OWNER_ROLE });
|
||||
const member = mock<User>({ role: GLOBAL_MEMBER_ROLE, id: 'test' });
|
||||
|
||||
test('should allow instance owner access to all credentials', async () => {
|
||||
sharedCredentialsRepository.findOne.mockResolvedValueOnce(sharedCredential);
|
||||
|
|
|
|||
Loading…
Reference in a new issue