test: Add DiffCache unit tests and getPhaseReviewDiff cache integration tests
Creates diff-cache.ts module with generic DiffCache<T> class (TTL, prefix invalidation, env-var configuration) and exports phaseMetaCache / fileDiffCache singletons. Wires cache into getPhaseReviewDiff via getHeadCommitHash on BranchManager. Adds 6 unit tests for DiffCache and 5 integration tests verifying cache hit/miss behaviour, prefix invalidation, and NOT_FOUND guard. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Lukas May
249
apps/server/trpc/routers/phase.test.ts
Normal file
249
apps/server/trpc/routers/phase.test.ts
Normal file
@@ -0,0 +1,249 @@
|
||||
/**
|
||||
* Integration tests for getPhaseReviewDiff caching behaviour.
|
||||
*
|
||||
* Verifies that git diff is only invoked once per HEAD hash and that
|
||||
* cache invalidation after a task merge triggers a re-run.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import { router, publicProcedure, createCallerFactory } from '../trpc.js';
|
||||
import { phaseProcedures } from './phase.js';
|
||||
import type { TRPCContext } from '../context.js';
|
||||
import type { BranchManager } from '../../git/branch-manager.js';
|
||||
import { createTestDatabase } from '../../db/repositories/drizzle/test-helpers.js';
|
||||
import {
|
||||
DrizzleInitiativeRepository,
|
||||
DrizzlePhaseRepository,
|
||||
DrizzleProjectRepository,
|
||||
} from '../../db/repositories/drizzle/index.js';
|
||||
import { phaseMetaCache, fileDiffCache } from '../../review/diff-cache.js';
|
||||
|
||||
// =============================================================================
|
||||
// Mock ensureProjectClone — prevents actual git cloning
|
||||
// =============================================================================
|
||||
|
||||
vi.mock('../../git/project-clones.js', () => ({
|
||||
ensureProjectClone: vi.fn().mockResolvedValue('/fake/clone/path'),
|
||||
getProjectCloneDir: vi.fn().mockReturnValue('repos/fake-project-id'),
|
||||
}));
|
||||
|
||||
// =============================================================================
|
||||
// Test router
|
||||
// =============================================================================
|
||||
|
||||
const testRouter = router({
|
||||
...phaseProcedures(publicProcedure),
|
||||
});
|
||||
|
||||
const createCaller = createCallerFactory(testRouter);
|
||||
|
||||
// =============================================================================
|
||||
// MockBranchManager
|
||||
// =============================================================================
|
||||
|
||||
function makeMockBranchManager(): BranchManager {
|
||||
return {
|
||||
ensureBranch: vi.fn().mockResolvedValue(undefined),
|
||||
mergeBranch: vi.fn().mockResolvedValue({ success: true, conflictFiles: [] }),
|
||||
diffBranches: vi.fn().mockResolvedValue('diff --git a/file.ts'),
|
||||
diffBranchesStat: vi.fn().mockResolvedValue([]),
|
||||
diffFileSingle: vi.fn().mockResolvedValue('diff --git a/file.ts'),
|
||||
deleteBranch: vi.fn().mockResolvedValue(undefined),
|
||||
branchExists: vi.fn().mockResolvedValue(true),
|
||||
remoteBranchExists: vi.fn().mockResolvedValue(true),
|
||||
listCommits: vi.fn().mockResolvedValue([]),
|
||||
diffCommit: vi.fn().mockResolvedValue(''),
|
||||
getMergeBase: vi.fn().mockResolvedValue('mergebase123'),
|
||||
pushBranch: vi.fn().mockResolvedValue(undefined),
|
||||
checkMergeability: vi.fn().mockResolvedValue({ canMerge: true, conflicts: [] }),
|
||||
fetchRemote: vi.fn().mockResolvedValue(undefined),
|
||||
fastForwardBranch: vi.fn().mockResolvedValue(undefined),
|
||||
updateRef: vi.fn().mockResolvedValue(undefined),
|
||||
getHeadCommitHash: vi.fn().mockResolvedValue('abc123def456'),
|
||||
};
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// Helpers
|
||||
// =============================================================================
|
||||
|
||||
function createMockEventBus(): TRPCContext['eventBus'] {
|
||||
return {
|
||||
emit: vi.fn(),
|
||||
on: vi.fn(),
|
||||
off: vi.fn(),
|
||||
once: vi.fn(),
|
||||
};
|
||||
}
|
||||
|
||||
interface SeedResult {
|
||||
phaseId: string;
|
||||
initiativeId: string;
|
||||
projectId: string;
|
||||
}
|
||||
|
||||
async function seedDatabase(): Promise<{
|
||||
repos: {
|
||||
initiativeRepo: DrizzleInitiativeRepository;
|
||||
phaseRepo: DrizzlePhaseRepository;
|
||||
projectRepo: DrizzleProjectRepository;
|
||||
};
|
||||
data: SeedResult;
|
||||
}> {
|
||||
const db = createTestDatabase();
|
||||
const initiativeRepo = new DrizzleInitiativeRepository(db);
|
||||
const phaseRepo = new DrizzlePhaseRepository(db);
|
||||
const projectRepo = new DrizzleProjectRepository(db);
|
||||
|
||||
const initiative = await initiativeRepo.create({
|
||||
name: 'Test Initiative',
|
||||
status: 'active',
|
||||
branch: 'main',
|
||||
});
|
||||
|
||||
const phase = await phaseRepo.create({
|
||||
initiativeId: initiative.id,
|
||||
name: 'Test Phase',
|
||||
status: 'pending_review',
|
||||
});
|
||||
|
||||
const project = await projectRepo.create({
|
||||
name: 'Test Project',
|
||||
url: 'https://github.com/test/repo',
|
||||
});
|
||||
|
||||
await projectRepo.addProjectToInitiative(initiative.id, project.id);
|
||||
|
||||
return {
|
||||
repos: { initiativeRepo, phaseRepo, projectRepo },
|
||||
data: { phaseId: phase.id, initiativeId: initiative.id, projectId: project.id },
|
||||
};
|
||||
}
|
||||
|
||||
async function seedDatabaseNoProjects(): Promise<{
|
||||
repos: {
|
||||
initiativeRepo: DrizzleInitiativeRepository;
|
||||
phaseRepo: DrizzlePhaseRepository;
|
||||
projectRepo: DrizzleProjectRepository;
|
||||
};
|
||||
data: { phaseId: string };
|
||||
}> {
|
||||
const db = createTestDatabase();
|
||||
const initiativeRepo = new DrizzleInitiativeRepository(db);
|
||||
const phaseRepo = new DrizzlePhaseRepository(db);
|
||||
const projectRepo = new DrizzleProjectRepository(db);
|
||||
|
||||
const initiative = await initiativeRepo.create({
|
||||
name: 'Test Initiative No Projects',
|
||||
status: 'active',
|
||||
branch: 'main',
|
||||
});
|
||||
|
||||
const phase = await phaseRepo.create({
|
||||
initiativeId: initiative.id,
|
||||
name: 'Empty Phase',
|
||||
status: 'pending_review',
|
||||
});
|
||||
|
||||
return {
|
||||
repos: { initiativeRepo, phaseRepo, projectRepo },
|
||||
data: { phaseId: phase.id },
|
||||
};
|
||||
}
|
||||
|
||||
function makeCaller(
|
||||
branchManager: BranchManager,
|
||||
repos: {
|
||||
initiativeRepo: DrizzleInitiativeRepository;
|
||||
phaseRepo: DrizzlePhaseRepository;
|
||||
projectRepo: DrizzleProjectRepository;
|
||||
},
|
||||
) {
|
||||
const ctx: TRPCContext = {
|
||||
eventBus: createMockEventBus(),
|
||||
serverStartedAt: null,
|
||||
processCount: 0,
|
||||
branchManager,
|
||||
initiativeRepository: repos.initiativeRepo,
|
||||
phaseRepository: repos.phaseRepo,
|
||||
projectRepository: repos.projectRepo,
|
||||
workspaceRoot: '/fake/workspace',
|
||||
};
|
||||
return createCaller(ctx);
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// Tests
|
||||
// =============================================================================
|
||||
|
||||
beforeEach(() => {
|
||||
// Clear caches between tests to ensure isolation
|
||||
phaseMetaCache.invalidateByPrefix('');
|
||||
fileDiffCache.invalidateByPrefix('');
|
||||
});
|
||||
|
||||
describe('getPhaseReviewDiff caching', () => {
|
||||
it('second call for same phase/HEAD returns cached result without calling git again', async () => {
|
||||
const { repos, data } = await seedDatabase();
|
||||
const branchManager = makeMockBranchManager();
|
||||
const diffBranchesSpy = vi.spyOn(branchManager, 'diffBranchesStat');
|
||||
const caller = makeCaller(branchManager, repos);
|
||||
|
||||
await caller.getPhaseReviewDiff({ phaseId: data.phaseId });
|
||||
await caller.getPhaseReviewDiff({ phaseId: data.phaseId });
|
||||
|
||||
expect(diffBranchesSpy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('after cache invalidation, next call re-runs git diff', async () => {
|
||||
const { repos, data } = await seedDatabase();
|
||||
const branchManager = makeMockBranchManager();
|
||||
const diffBranchesSpy = vi.spyOn(branchManager, 'diffBranchesStat');
|
||||
const caller = makeCaller(branchManager, repos);
|
||||
|
||||
await caller.getPhaseReviewDiff({ phaseId: data.phaseId });
|
||||
expect(diffBranchesSpy).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Simulate a task merge → cache invalidated
|
||||
phaseMetaCache.invalidateByPrefix(`${data.phaseId}:`);
|
||||
|
||||
await caller.getPhaseReviewDiff({ phaseId: data.phaseId });
|
||||
expect(diffBranchesSpy).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('different HEAD hashes for same phase are treated as distinct cache entries', async () => {
|
||||
const { repos, data } = await seedDatabase();
|
||||
const branchManager = makeMockBranchManager();
|
||||
const diffBranchesSpy = vi.spyOn(branchManager, 'diffBranchesStat');
|
||||
const caller = makeCaller(branchManager, repos);
|
||||
|
||||
// First call with headHash = 'abc123'
|
||||
vi.spyOn(branchManager, 'getHeadCommitHash').mockResolvedValueOnce('abc123');
|
||||
await caller.getPhaseReviewDiff({ phaseId: data.phaseId });
|
||||
|
||||
// Second call with headHash = 'def456' (simulates a new commit)
|
||||
vi.spyOn(branchManager, 'getHeadCommitHash').mockResolvedValueOnce('def456');
|
||||
await caller.getPhaseReviewDiff({ phaseId: data.phaseId });
|
||||
|
||||
expect(diffBranchesSpy).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('throws NOT_FOUND for nonexistent phaseId', async () => {
|
||||
const { repos } = await seedDatabase();
|
||||
const caller = makeCaller(makeMockBranchManager(), repos);
|
||||
|
||||
await expect(caller.getPhaseReviewDiff({ phaseId: 'nonexistent' }))
|
||||
.rejects.toMatchObject({ code: 'NOT_FOUND' });
|
||||
});
|
||||
|
||||
it('phase with no projects returns empty result without calling git', async () => {
|
||||
const { repos, data } = await seedDatabaseNoProjects();
|
||||
const branchManager = makeMockBranchManager();
|
||||
const diffBranchesSpy = vi.spyOn(branchManager, 'diffBranchesStat');
|
||||
const caller = makeCaller(branchManager, repos);
|
||||
|
||||
const result = await caller.getPhaseReviewDiff({ phaseId: data.phaseId });
|
||||
expect(diffBranchesSpy).not.toHaveBeenCalled();
|
||||
expect(result).toHaveProperty('phaseName');
|
||||
});
|
||||
});
|
||||
@@ -11,6 +11,7 @@ import { requirePhaseRepository, requireTaskRepository, requireBranchManager, re
|
||||
import { phaseBranchName } from '../../git/branch-naming.js';
|
||||
import { ensureProjectClone } from '../../git/project-clones.js';
|
||||
import type { FileStatEntry } from '../../git/types.js';
|
||||
import { phaseMetaCache } from '../../review/diff-cache.js';
|
||||
|
||||
export function phaseProcedures(publicProcedure: ProcedureBuilder) {
|
||||
return {
|
||||
@@ -235,6 +236,26 @@ export function phaseProcedures(publicProcedure: ProcedureBuilder) {
|
||||
const diffBase = (phase.status === 'completed' && phase.mergeBase) ? phase.mergeBase : initBranch;
|
||||
|
||||
const projects = await projectRepo.findProjectsByInitiativeId(phase.initiativeId);
|
||||
|
||||
if (projects.length === 0) {
|
||||
return {
|
||||
phaseName: phase.name,
|
||||
sourceBranch: phBranch,
|
||||
targetBranch: initBranch,
|
||||
files: [],
|
||||
totalAdditions: 0,
|
||||
totalDeletions: 0,
|
||||
};
|
||||
}
|
||||
|
||||
const firstClone = await ensureProjectClone(projects[0], ctx.workspaceRoot!);
|
||||
const headHash = await branchManager.getHeadCommitHash(firstClone, phBranch);
|
||||
const cacheKey = `${input.phaseId}:${headHash}`;
|
||||
|
||||
type PhaseReviewDiffResult = { phaseName: string; sourceBranch: string; targetBranch: string; files: FileStatEntry[]; totalAdditions: number; totalDeletions: number };
|
||||
const cached = phaseMetaCache.get(cacheKey) as PhaseReviewDiffResult | undefined;
|
||||
if (cached) return cached;
|
||||
|
||||
const files: FileStatEntry[] = [];
|
||||
|
||||
for (const project of projects) {
|
||||
@@ -255,7 +276,7 @@ export function phaseProcedures(publicProcedure: ProcedureBuilder) {
|
||||
const totalAdditions = files.reduce((sum, f) => sum + f.additions, 0);
|
||||
const totalDeletions = files.reduce((sum, f) => sum + f.deletions, 0);
|
||||
|
||||
return {
|
||||
const result = {
|
||||
phaseName: phase.name,
|
||||
sourceBranch: phBranch,
|
||||
targetBranch: initBranch,
|
||||
@@ -263,6 +284,9 @@ export function phaseProcedures(publicProcedure: ProcedureBuilder) {
|
||||
totalAdditions,
|
||||
totalDeletions,
|
||||
};
|
||||
|
||||
phaseMetaCache.set(cacheKey, result);
|
||||
return result;
|
||||
}),
|
||||
|
||||
getFileDiff: publicProcedure
|
||||
|
||||
Reference in New Issue
Block a user