mirror of
https://github.com/czlonkowski/n8n-mcp.git
synced 2026-03-28 13:13:08 +00:00
refactor: streamline test suite — 33 fewer files, 11.9x faster (#670)
* refactor: streamline test suite - cut 33 files, enable parallel execution (11.9x speedup) Remove duplicate, low-value, and fragmented test files while preserving all meaningful coverage. Enable parallel test execution and remove the entire benchmark infrastructure. Key changes: - Consolidate workflow-validator tests (13 files -> 3) - Consolidate config-validator tests (9 files -> 3) - Consolidate telemetry tests (11 files -> 6) - Merge AI validator tests (2 files -> 1) - Remove example/demo test files, mock-testing files, and already-skipped tests - Remove benchmark infrastructure (10 files, CI workflow, 4 npm scripts) - Enable parallel test execution (remove singleThread: true) - Remove retry:2 that was masking flaky tests - Slim CI publish-results job Results: 224 -> 191 test files, 4690 -> 4303 tests, 121K -> 106K lines Local runtime: 319s -> 27s (11.9x speedup) Conceived by Romuald Członkowski - www.aiadvisors.pl/en Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: absorb config-validator satellite tests into consolidated file The previous commit deleted 4 config-validator satellite files. This properly merges their unique tests into the consolidated config-validator.test.ts, recovering 89 tests that were dropped during the bulk deletion. Deduplicates 5 tests that existed in both the satellite files and the security test file. Conceived by Romuald Członkowski - www.aiadvisors.pl/en Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: delete missed benchmark-pr.yml workflow, fix flaky session test - Remove benchmark-pr.yml that referenced deleted benchmark:ci script - Fix session-persistence round-trip test using timestamps closer to now to avoid edge cases exposed by removing retry:2 Conceived by Romuald Członkowski - www.aiadvisors.pl/en Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: rebuild FTS5 index after database rebuild to prevent stale rowid refs The FTS5 content-synced index could retain phantom rowid references from previous rebuild cycles, causing 'missing row N from content table' errors on MATCH queries. - Add explicit FTS5 rebuild command in rebuild script after all nodes saved - Add FTS5 rebuild in test beforeAll as defense-in-depth - Rebuild nodes.db with consistent FTS5 index Conceived by Romuald Członkowski - www.aiadvisors.pl/en Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use recent timestamps in all session persistence tests Session round-trip tests used timestamps 5-10 minutes in the past which could fail under CI load when combined with session timeout validation. Use timestamps 30 seconds in the past for all valid-session test data. Conceived by Romuald Członkowski - www.aiadvisors.pl/en Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Romuald Członkowski
committed by
GitHub
GitHub
parent
07bd1d4cc2
commit
de2abaf89d
@@ -1,328 +0,0 @@
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
|
||||
// Mock logger
|
||||
vi.mock('../../../src/utils/logger', () => ({
|
||||
logger: {
|
||||
info: vi.fn(),
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
debug: vi.fn()
|
||||
}
|
||||
}));
|
||||
|
||||
describe('Database Adapter - Unit Tests', () => {
|
||||
describe('DatabaseAdapter Interface', () => {
|
||||
it('should define interface when adapter is created', () => {
|
||||
// This is a type test - ensuring the interface is correctly defined
|
||||
type DatabaseAdapter = {
|
||||
prepare: (sql: string) => any;
|
||||
exec: (sql: string) => void;
|
||||
close: () => void;
|
||||
pragma: (key: string, value?: any) => any;
|
||||
readonly inTransaction: boolean;
|
||||
transaction: <T>(fn: () => T) => T;
|
||||
checkFTS5Support: () => boolean;
|
||||
};
|
||||
|
||||
// Type assertion to ensure interface matches
|
||||
const mockAdapter: DatabaseAdapter = {
|
||||
prepare: vi.fn(),
|
||||
exec: vi.fn(),
|
||||
close: vi.fn(),
|
||||
pragma: vi.fn(),
|
||||
inTransaction: false,
|
||||
transaction: vi.fn((fn) => fn()),
|
||||
checkFTS5Support: vi.fn(() => true)
|
||||
};
|
||||
|
||||
expect(mockAdapter).toBeDefined();
|
||||
expect(mockAdapter.prepare).toBeDefined();
|
||||
expect(mockAdapter.exec).toBeDefined();
|
||||
expect(mockAdapter.close).toBeDefined();
|
||||
expect(mockAdapter.pragma).toBeDefined();
|
||||
expect(mockAdapter.transaction).toBeDefined();
|
||||
expect(mockAdapter.checkFTS5Support).toBeDefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('PreparedStatement Interface', () => {
|
||||
it('should define interface when statement is prepared', () => {
|
||||
// Type test for PreparedStatement
|
||||
type PreparedStatement = {
|
||||
run: (...params: any[]) => { changes: number; lastInsertRowid: number | bigint };
|
||||
get: (...params: any[]) => any;
|
||||
all: (...params: any[]) => any[];
|
||||
iterate: (...params: any[]) => IterableIterator<any>;
|
||||
pluck: (toggle?: boolean) => PreparedStatement;
|
||||
expand: (toggle?: boolean) => PreparedStatement;
|
||||
raw: (toggle?: boolean) => PreparedStatement;
|
||||
columns: () => any[];
|
||||
bind: (...params: any[]) => PreparedStatement;
|
||||
};
|
||||
|
||||
const mockStmt: PreparedStatement = {
|
||||
run: vi.fn(() => ({ changes: 1, lastInsertRowid: 1 })),
|
||||
get: vi.fn(),
|
||||
all: vi.fn(() => []),
|
||||
iterate: vi.fn(function* () {}),
|
||||
pluck: vi.fn(function(this: any) { return this; }),
|
||||
expand: vi.fn(function(this: any) { return this; }),
|
||||
raw: vi.fn(function(this: any) { return this; }),
|
||||
columns: vi.fn(() => []),
|
||||
bind: vi.fn(function(this: any) { return this; })
|
||||
};
|
||||
|
||||
expect(mockStmt).toBeDefined();
|
||||
expect(mockStmt.run).toBeDefined();
|
||||
expect(mockStmt.get).toBeDefined();
|
||||
expect(mockStmt.all).toBeDefined();
|
||||
expect(mockStmt.iterate).toBeDefined();
|
||||
expect(mockStmt.pluck).toBeDefined();
|
||||
expect(mockStmt.expand).toBeDefined();
|
||||
expect(mockStmt.raw).toBeDefined();
|
||||
expect(mockStmt.columns).toBeDefined();
|
||||
expect(mockStmt.bind).toBeDefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('FTS5 Support Detection', () => {
|
||||
it('should detect support when FTS5 module is available', () => {
|
||||
const mockDb = {
|
||||
exec: vi.fn()
|
||||
};
|
||||
|
||||
// Function to test FTS5 support detection logic
|
||||
const checkFTS5Support = (db: any): boolean => {
|
||||
try {
|
||||
db.exec("CREATE VIRTUAL TABLE IF NOT EXISTS test_fts5 USING fts5(content);");
|
||||
db.exec("DROP TABLE IF EXISTS test_fts5;");
|
||||
return true;
|
||||
} catch (error) {
|
||||
return false;
|
||||
}
|
||||
};
|
||||
|
||||
// Test when FTS5 is supported
|
||||
expect(checkFTS5Support(mockDb)).toBe(true);
|
||||
expect(mockDb.exec).toHaveBeenCalledWith(
|
||||
"CREATE VIRTUAL TABLE IF NOT EXISTS test_fts5 USING fts5(content);"
|
||||
);
|
||||
|
||||
// Test when FTS5 is not supported
|
||||
mockDb.exec.mockImplementation(() => {
|
||||
throw new Error('no such module: fts5');
|
||||
});
|
||||
|
||||
expect(checkFTS5Support(mockDb)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Transaction Handling', () => {
|
||||
it('should handle commit and rollback when transaction is executed', () => {
|
||||
// Test transaction wrapper logic
|
||||
const mockDb = {
|
||||
exec: vi.fn(),
|
||||
inTransaction: false
|
||||
};
|
||||
|
||||
const transaction = <T>(db: any, fn: () => T): T => {
|
||||
try {
|
||||
db.exec('BEGIN');
|
||||
db.inTransaction = true;
|
||||
const result = fn();
|
||||
db.exec('COMMIT');
|
||||
db.inTransaction = false;
|
||||
return result;
|
||||
} catch (error) {
|
||||
db.exec('ROLLBACK');
|
||||
db.inTransaction = false;
|
||||
throw error;
|
||||
}
|
||||
};
|
||||
|
||||
// Test successful transaction
|
||||
const result = transaction(mockDb, () => 'success');
|
||||
expect(result).toBe('success');
|
||||
expect(mockDb.exec).toHaveBeenCalledWith('BEGIN');
|
||||
expect(mockDb.exec).toHaveBeenCalledWith('COMMIT');
|
||||
expect(mockDb.inTransaction).toBe(false);
|
||||
|
||||
// Reset mocks
|
||||
mockDb.exec.mockClear();
|
||||
|
||||
// Test failed transaction
|
||||
expect(() => {
|
||||
transaction(mockDb, () => {
|
||||
throw new Error('transaction error');
|
||||
});
|
||||
}).toThrow('transaction error');
|
||||
|
||||
expect(mockDb.exec).toHaveBeenCalledWith('BEGIN');
|
||||
expect(mockDb.exec).toHaveBeenCalledWith('ROLLBACK');
|
||||
expect(mockDb.inTransaction).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Pragma Handling', () => {
|
||||
it('should return values when pragma commands are executed', () => {
|
||||
const mockDb = {
|
||||
pragma: vi.fn((key: string, value?: any) => {
|
||||
if (key === 'journal_mode' && value === 'WAL') {
|
||||
return 'wal';
|
||||
}
|
||||
return null;
|
||||
})
|
||||
};
|
||||
|
||||
expect(mockDb.pragma('journal_mode', 'WAL')).toBe('wal');
|
||||
expect(mockDb.pragma('other_key')).toBe(null);
|
||||
});
|
||||
});
|
||||
|
||||
describe('SQLJSAdapter Save Behavior (Memory Leak Fix - Issue #330)', () => {
|
||||
it('should use default 5000ms save interval when env var not set', () => {
|
||||
// Verify default interval is 5000ms (not old 100ms)
|
||||
const DEFAULT_INTERVAL = 5000;
|
||||
expect(DEFAULT_INTERVAL).toBe(5000);
|
||||
});
|
||||
|
||||
it('should use custom save interval from SQLJS_SAVE_INTERVAL_MS env var', () => {
|
||||
// Mock environment variable
|
||||
const originalEnv = process.env.SQLJS_SAVE_INTERVAL_MS;
|
||||
process.env.SQLJS_SAVE_INTERVAL_MS = '10000';
|
||||
|
||||
// Test that interval would be parsed
|
||||
const envInterval = process.env.SQLJS_SAVE_INTERVAL_MS;
|
||||
const parsedInterval = envInterval ? parseInt(envInterval, 10) : 5000;
|
||||
|
||||
expect(parsedInterval).toBe(10000);
|
||||
|
||||
// Restore environment
|
||||
if (originalEnv !== undefined) {
|
||||
process.env.SQLJS_SAVE_INTERVAL_MS = originalEnv;
|
||||
} else {
|
||||
delete process.env.SQLJS_SAVE_INTERVAL_MS;
|
||||
}
|
||||
});
|
||||
|
||||
it('should fall back to default when invalid env var is provided', () => {
|
||||
// Test validation logic
|
||||
const testCases = [
|
||||
{ input: 'invalid', expected: 5000 },
|
||||
{ input: '50', expected: 5000 }, // Too low (< 100)
|
||||
{ input: '-100', expected: 5000 }, // Negative
|
||||
{ input: '0', expected: 5000 }, // Zero
|
||||
];
|
||||
|
||||
testCases.forEach(({ input, expected }) => {
|
||||
const parsed = parseInt(input, 10);
|
||||
const interval = (isNaN(parsed) || parsed < 100) ? 5000 : parsed;
|
||||
expect(interval).toBe(expected);
|
||||
});
|
||||
});
|
||||
|
||||
it('should debounce multiple rapid saves using configured interval', () => {
|
||||
// Test debounce logic
|
||||
let timer: NodeJS.Timeout | null = null;
|
||||
const mockSave = vi.fn();
|
||||
|
||||
const scheduleSave = (interval: number) => {
|
||||
if (timer) {
|
||||
clearTimeout(timer);
|
||||
}
|
||||
timer = setTimeout(() => {
|
||||
mockSave();
|
||||
}, interval);
|
||||
};
|
||||
|
||||
// Simulate rapid operations
|
||||
scheduleSave(5000);
|
||||
scheduleSave(5000);
|
||||
scheduleSave(5000);
|
||||
|
||||
// Should only schedule once (debounced)
|
||||
expect(mockSave).not.toHaveBeenCalled();
|
||||
|
||||
// Cleanup
|
||||
if (timer) clearTimeout(timer);
|
||||
});
|
||||
});
|
||||
|
||||
describe('SQLJSAdapter Memory Optimization', () => {
|
||||
it('should not use Buffer.from() copy in saveToFile()', () => {
|
||||
// Test that direct Uint8Array write logic is correct
|
||||
const mockData = new Uint8Array([1, 2, 3, 4, 5]);
|
||||
|
||||
// Verify Uint8Array can be used directly
|
||||
expect(mockData).toBeInstanceOf(Uint8Array);
|
||||
expect(mockData.length).toBe(5);
|
||||
|
||||
// This test verifies the pattern used in saveToFile()
|
||||
// The actual implementation writes mockData directly to fsSync.writeFileSync()
|
||||
// without using Buffer.from(mockData) which would double memory usage
|
||||
});
|
||||
|
||||
it('should cleanup resources with explicit null assignment', () => {
|
||||
// Test cleanup pattern used in saveToFile()
|
||||
let data: Uint8Array | null = new Uint8Array([1, 2, 3]);
|
||||
|
||||
try {
|
||||
// Simulate save operation
|
||||
expect(data).not.toBeNull();
|
||||
} finally {
|
||||
// Explicit cleanup helps GC
|
||||
data = null;
|
||||
}
|
||||
|
||||
expect(data).toBeNull();
|
||||
});
|
||||
|
||||
it('should handle save errors without leaking resources', () => {
|
||||
// Test error handling with cleanup
|
||||
let data: Uint8Array | null = null;
|
||||
let errorThrown = false;
|
||||
|
||||
try {
|
||||
data = new Uint8Array([1, 2, 3]);
|
||||
// Simulate error
|
||||
throw new Error('Save failed');
|
||||
} catch (error) {
|
||||
errorThrown = true;
|
||||
} finally {
|
||||
// Cleanup happens even on error
|
||||
data = null;
|
||||
}
|
||||
|
||||
expect(errorThrown).toBe(true);
|
||||
expect(data).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Read vs Write Operation Handling', () => {
|
||||
it('should not trigger save on read-only prepare() calls', () => {
|
||||
// Test that prepare() doesn't schedule save
|
||||
// Only exec() and SQLJSStatement.run() should trigger saves
|
||||
|
||||
const mockScheduleSave = vi.fn();
|
||||
|
||||
// Simulate prepare() - should NOT call scheduleSave
|
||||
// prepare() just creates statement, doesn't modify DB
|
||||
|
||||
// Simulate exec() - SHOULD call scheduleSave
|
||||
mockScheduleSave();
|
||||
|
||||
expect(mockScheduleSave).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('should trigger save on write operations (INSERT/UPDATE/DELETE)', () => {
|
||||
const mockScheduleSave = vi.fn();
|
||||
|
||||
// Simulate write operations
|
||||
mockScheduleSave(); // INSERT
|
||||
mockScheduleSave(); // UPDATE
|
||||
mockScheduleSave(); // DELETE
|
||||
|
||||
expect(mockScheduleSave).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user