From ca4cdabfd412373a6e20036d9a0765d2ca4d8fac Mon Sep 17 00:00:00 2001 From: smithrashell Date: Sun, 21 Dec 2025 16:52:27 -0500 Subject: [PATCH] refactor(tests): shift service tests to contract-testing pattern (Issue #238) --- .../services/__tests__/problemService.test.js | 126 ++++++++++----- .../services/__tests__/sessionService.test.js | 152 ++++++++++-------- 2 files changed, 171 insertions(+), 107 deletions(-) diff --git a/chrome-extension-app/src/shared/services/__tests__/problemService.test.js b/chrome-extension-app/src/shared/services/__tests__/problemService.test.js index 4e526802..2ad01d76 100644 --- a/chrome-extension-app/src/shared/services/__tests__/problemService.test.js +++ b/chrome-extension-app/src/shared/services/__tests__/problemService.test.js @@ -93,7 +93,7 @@ const createTagMasteryEntry = (tag, successRate, totalAttempts) => ({ totalAttempts }); -const createAttemptData = (overrides = {}) => ({ +const _createAttemptData = (overrides = {}) => ({ id: "test-uuid-123", problem_id: 1, time_spent: 1200, @@ -101,31 +101,62 @@ const createAttemptData = (overrides = {}) => ({ ...overrides }); -// Common assertion helpers -const assertProblemServiceCall = (mockFn, ...expectedArgs) => { - expect(mockFn).toHaveBeenCalledWith(...expectedArgs); +// Contract assertion helpers - focus on return shapes, not implementation + +/** + * Contract assertion: Verify getProblemByDescription return matches expected contract. + * + * The `found` flag indicates whether the problem exists in the USER's problems store + * (i.e., they've attempted it before). The problem object may still be returned from + * standard_problems even when found is false. + * + * @param {Object} result - The function return value + * @param {boolean} expectedFound - Whether problem exists in user's problems store + * @param {Object|null} [expectedProblem] - null means problem doesn't exist anywhere + */ +const assertProblemLookupContract = (result, expectedFound, expectedProblem = undefined) => { + // Contract: always returns { problem, found } shape + expect(result).toHaveProperty('problem'); + expect(result).toHaveProperty('found'); + expect(result.found).toBe(expectedFound); + + if (expectedProblem === null) { + // Explicitly expect null when problem doesn't exist anywhere + expect(result.problem).toBeNull(); + } else if (expectedProblem !== undefined) { + // When expected problem provided, verify it matches + expect(result.problem).not.toBeNull(); + expect(result.problem).toHaveProperty('id'); + expect(result.problem).toHaveProperty('title'); + expect(result.problem).toHaveProperty('difficulty'); + } + // If expectedProblem is undefined, we don't verify problem content (just the found flag) }; -const assertProblemResult = (result, expectedProblem, expectedFound) => { - if (expectedProblem && typeof expectedProblem === 'object' && expectedProblem.box_level !== undefined) { - // For database problems with snake_case, check the merged camelCase result - expect(result.found).toBe(expectedFound); - expect(result.problem).toMatchObject({ - id: expectedProblem.leetcode_id || expectedProblem.id, - leetcode_id: expectedProblem.leetcode_id || expectedProblem.id, - title: expectedProblem.title, - difficulty: expectedProblem.difficulty, - attempts: expectedProblem.attempts, - boxLevel: expectedProblem.box_level, - tags: expectedProblem.tags || [] - }); - } else { - expect(result).toEqual({ problem: expectedProblem, found: expectedFound }); - } +/** + * Contract assertion: Verify problem result has merged fields from both stores. + */ +const assertMergedProblemContract = (result, expectedProblem) => { + expect(result.found).toBe(true); + expect(result.problem).toMatchObject({ + id: expectedProblem.leetcode_id || expectedProblem.id, + leetcode_id: expectedProblem.leetcode_id || expectedProblem.id, + title: expectedProblem.title, + difficulty: expectedProblem.difficulty, + boxLevel: expectedProblem.box_level, + }); }; -const assertAttemptsServiceCall = (mockFn, expectedAttempt, expectedProblem) => { - expect(mockFn).toHaveBeenCalledWith(expectedAttempt, expectedProblem); +/** + * Contract assertion: Verify addOrUpdateProblem returns expected response shape. + */ +const assertAddOrUpdateContract = (result, expectAttemptAdded) => { + if (expectAttemptAdded) { + expect(result).toHaveProperty('message'); + expect(result.message).toContain('Attempt'); + } else { + expect(result).toHaveProperty('success'); + } }; // Mock setup helpers @@ -172,6 +203,7 @@ const runGetProblemByDescriptionTests = () => { problemsDb.checkDatabaseForProblem.mockClear(); }); + // CONTRACT: Returns { problem, found: true } with merged data when found in both stores it("should return problem from problems store when found in both stores", async () => { const mockStandardProblem = createMockProblem(); const mockProblemInDb = createMockProblemInDb(); @@ -181,11 +213,11 @@ const runGetProblemByDescriptionTests = () => { const result = await ProblemService.getProblemByDescription("Two Sum", "two-sum"); - assertProblemServiceCall(standardProblems.getProblemFromStandardProblems, "two-sum"); - assertProblemServiceCall(problemsDb.checkDatabaseForProblem, 1); - assertProblemResult(result, mockProblemInDb, true); + // Verify contract: returns merged problem with found: true + assertMergedProblemContract(result, mockProblemInDb); }); + // CONTRACT: Returns { problem, found: false } when only in standard problems it("should return problem from standard problems when not found in problems store", async () => { const mockStandardProblem = createMockProblem(); @@ -194,19 +226,23 @@ const runGetProblemByDescriptionTests = () => { const result = await ProblemService.getProblemByDescription("Two Sum", "two-sum"); - assertProblemServiceCall(standardProblems.getProblemFromStandardProblems, "two-sum"); - assertProblemServiceCall(problemsDb.checkDatabaseForProblem, 1); - assertProblemResult(result, mockStandardProblem, false); + // Verify contract: returns standard problem with found: false + assertProblemLookupContract(result, false); + expect(result.problem).toMatchObject({ + title: mockStandardProblem.title, + difficulty: mockStandardProblem.difficulty, + }); }); + // CONTRACT: Returns { problem: null, found: false } when not found anywhere it("should return null when problem not found in any store", async () => { setupMockStandardProblem(null); setupMockDatabaseProblem(null); const result = await ProblemService.getProblemByDescription("Non-existent", "non-existent"); - assertProblemServiceCall(standardProblems.getProblemFromStandardProblems, "non-existent"); - expect(result).toEqual({ problem: null, found: false }); + // Verify contract: returns null problem with found: false + assertProblemLookupContract(result, false, null); }); }); }; @@ -217,23 +253,27 @@ const runGetAllProblemsTests = () => { problemsDb.fetchAllProblems.mockClear(); }); + // CONTRACT: Returns array of problems it("should return all problems from the database", async () => { const mockProblems = createMockProblemsArray(); setupMockFetchAllProblems(mockProblems); const result = await ProblemService.getAllProblems(); - expect(problemsDb.fetchAllProblems).toHaveBeenCalled(); + // Verify contract: returns array matching input + expect(Array.isArray(result)).toBe(true); expect(result).toEqual(mockProblems); }); + // CONTRACT: Returns empty array when no problems it("should return empty array when no problems exist", async () => { setupMockFetchAllProblems([]); const result = await ProblemService.getAllProblems(); - expect(problemsDb.fetchAllProblems).toHaveBeenCalled(); - expect(result).toEqual([]); + // Verify contract: returns empty array + expect(Array.isArray(result)).toBe(true); + expect(result).toHaveLength(0); }); }); }; @@ -245,30 +285,32 @@ const runAddOrUpdateProblemTests = () => { AttemptsService.addAttempt.mockClear(); }); + // CONTRACT: Returns attempt response when problem exists it("should add attempt when problem exists", async () => { const contentScriptData = createContentScriptData(); const mockProblem = createMockProblem(); - const attemptData = createAttemptData(); setupMockDatabaseProblem(mockProblem); setupMockAddAttempt(); const result = await ProblemService.addOrUpdateProblem(contentScriptData); - assertAttemptsServiceCall(AttemptsService.addAttempt, attemptData, mockProblem); - expect(result).toEqual({ message: "Attempt added successfully", sessionId: "test-session-123" }); + // Verify contract: returns attempt added response + assertAddOrUpdateContract(result, true); + expect(result.sessionId).toBeDefined(); }); + // CONTRACT: Returns success response when adding new problem it("should add new problem when problem does not exist", async () => { const contentScriptData = createContentScriptData(); - + setupMockDatabaseProblem(null); setupMockAddProblem({ success: true }); const result = await ProblemService.addOrUpdateProblem(contentScriptData); - assertProblemServiceCall(problemsDb.addProblem, contentScriptData); - expect(result).toEqual({ success: true }); + // Verify contract: returns success response + assertAddOrUpdateContract(result, false); }); }); }; @@ -283,10 +325,10 @@ const runCreateSessionTests = () => { StorageService.getSettings.mockClear(); }); + // CONTRACT: Returns array of problems (possibly empty) it("should create session with adaptive settings", async () => { const sessionSettings = createSessionSettings(); - const _expectedResult = []; - + buildAdaptiveSessionSettings.mockResolvedValue(sessionSettings); setupMockFetchAllProblems([]); setupMockFetchAdditionalProblems([]); @@ -295,7 +337,7 @@ const runCreateSessionTests = () => { const result = await ProblemService.createSession(); - expect(buildAdaptiveSessionSettings).toHaveBeenCalled(); + // Verify contract: returns array of problems expect(Array.isArray(result)).toBe(true); }); }); diff --git a/chrome-extension-app/src/shared/services/__tests__/sessionService.test.js b/chrome-extension-app/src/shared/services/__tests__/sessionService.test.js index 66676d5e..bb64a9fb 100644 --- a/chrome-extension-app/src/shared/services/__tests__/sessionService.test.js +++ b/chrome-extension-app/src/shared/services/__tests__/sessionService.test.js @@ -67,7 +67,12 @@ const createMockProblems = () => [ { id: 2, title: "Add Two Numbers", difficulty: "Medium" }, ]; -// Test helper functions +// Test helper functions - focused on contract verification, not implementation + +/** + * Setup mocks for session completion scenarios. + * Note: We mock dependencies but verify return contracts, not mock calls. + */ const setupMocksForCompletedSession = (session) => { getSessionById.mockResolvedValue(session); updateSessionInDB.mockResolvedValue(); @@ -77,13 +82,30 @@ const setupMocksForCompletedSession = (session) => { StorageService.setSessionState = jest.fn().mockResolvedValue(); }; -const expectSessionCompletion = (sessionId, _session) => { - expect(getSessionById).toHaveBeenCalledWith(sessionId); - expect(updateSessionInDB).toHaveBeenCalledWith( - expect.objectContaining({ status: "completed" }) - ); - // Note: calculateTagMastery and updateProblemRelationships may not be called directly - // in checkAndCompleteSession anymore - they may be handled elsewhere in the flow +/** + * Contract assertion: Verify checkAndCompleteSession return value matches expected contract. + * + * @param {boolean|Array} result - The function return value + * @param {'not_found'|'completed'|'unattempted'} expectedState - Expected state + * @param {number} [expectedUnattemptedCount] - For 'unattempted' state, expected count + */ +const assertCheckAndCompleteContract = (result, expectedState, expectedUnattemptedCount = 0) => { + switch (expectedState) { + case 'not_found': + expect(result).toBe(false); + break; + case 'completed': + expect(result).toEqual([]); + break; + case 'unattempted': + expect(Array.isArray(result)).toBe(true); + expect(result.length).toBe(expectedUnattemptedCount); + result.forEach(problem => { + expect(problem).toHaveProperty('id'); + expect(problem).toHaveProperty('title'); + }); + break; + } }; const setupMocksForNewSession = () => { @@ -92,21 +114,36 @@ const setupMocksForNewSession = () => { saveSessionToStorage.mockImplementation(() => Promise.resolve()); }; +/** + * Contract assertion: Verify createNewSession return matches Session contract. + */ +const assertNewSessionContract = (session) => { + expect(session).not.toBeNull(); + expect(session).toHaveProperty('id'); + expect(session).toHaveProperty('status', 'in_progress'); + expect(session).toHaveProperty('problems'); + expect(Array.isArray(session.problems)).toBe(true); + expect(session).toHaveProperty('attempts'); + expect(Array.isArray(session.attempts)).toBe(true); +}; + // Test group functions const runCheckAndCompleteSessionTests = () => { describe("checkAndCompleteSession", () => { + // CONTRACT: Returns [] when all problems are attempted (session just completed) it("should return empty array when all problems are attempted", async () => { const sessionId = "test-session-123"; const mockSession = createMockSession({ id: sessionId }); - + setupMocksForCompletedSession(mockSession); - + const result = await SessionService.checkAndCompleteSession(sessionId); - - expectSessionCompletion(sessionId, mockSession); - expect(result).toEqual([]); + + // Verify contract: returns empty array for completed session + assertCheckAndCompleteContract(result, 'completed'); }); + // CONTRACT: Returns array of unattempted problems when session incomplete it("should return unattempted problems when not all problems are attempted", async () => { const sessionId = "test-session-456"; const mockSession = createMockSession({ @@ -118,24 +155,23 @@ const runCheckAndCompleteSessionTests = () => { const result = await SessionService.checkAndCompleteSession(sessionId); - expect(getSessionById).toHaveBeenCalledWith(sessionId); - expect(updateSessionInDB).not.toHaveBeenCalled(); - expect(calculateTagMastery).not.toHaveBeenCalled(); - expect(result).toEqual([ - expect.objectContaining({ id: 2, title: "Problem 2" }) - ]); + // Verify contract: returns array of unattempted problems with required fields + assertCheckAndCompleteContract(result, 'unattempted', 1); + expect(result[0]).toMatchObject({ id: 2, title: "Problem 2" }); }); + // CONTRACT: Returns false when session not found it("should return false when session not found", async () => { const sessionId = "non-existent-session"; getSessionById.mockResolvedValue(null); const result = await SessionService.checkAndCompleteSession(sessionId); - expect(getSessionById).toHaveBeenCalledWith(sessionId); - expect(result).toBe(false); + // Verify contract: returns false for not found + assertCheckAndCompleteContract(result, 'not_found'); }); + // CONTRACT: Returns [] for already completed session (idempotent) it("should return empty array for already completed session", async () => { const sessionId = "completed-session"; const mockSession = createMockSession({ @@ -152,14 +188,15 @@ const runCheckAndCompleteSessionTests = () => { const result = await SessionService.checkAndCompleteSession(sessionId); - expect(getSessionById).toHaveBeenCalledWith(sessionId); - expect(result).toEqual([]); + // Verify contract: completed sessions return empty array + assertCheckAndCompleteContract(result, 'completed'); }); }); }; const runResumeSessionTests = () => { describe("resumeSession", () => { + // CONTRACT: Returns session object with required fields when resumable session exists it("should resume an existing in-progress session with remaining problems", async () => { const mockSession = { id: "resume-session-123", @@ -173,32 +210,32 @@ const runResumeSessionTests = () => { const result = await SessionService.resumeSession(); - expect(getLatestSessionByType).toHaveBeenCalledWith(null, "in_progress"); - expect(saveSessionToStorage).toHaveBeenCalledWith(mockSession); - expect(result).toEqual(expect.objectContaining({ - id: "resume-session-123", - status: "in_progress", - problems: [1, 2], - attempts: [{ problemId: 1 }], - currentProblemIndex: 0, - })); + // Verify contract: returns session with required fields preserved + expect(result).not.toBeNull(); + expect(result.id).toBe("resume-session-123"); + expect(result.status).toBe("in_progress"); + expect(result.problems).toEqual([1, 2]); + expect(result.attempts).toHaveLength(1); + expect(result).toHaveProperty('currentProblemIndex'); }); + // CONTRACT: Returns null when no resumable session exists it("should return null when no in-progress session exists", async () => { getLatestSessionByType.mockResolvedValue(null); const result = await SessionService.resumeSession(); - expect(getLatestSessionByType).toHaveBeenCalledWith(null, "in_progress"); + // Verify contract: null when no session expect(result).toBeNull(); }); + // CONTRACT: Returns null when session is completed (not resumable) it("should return null when session is completed", async () => { getLatestSessionByType.mockResolvedValue(null); const result = await SessionService.resumeSession(); - expect(getLatestSessionByType).toHaveBeenCalledWith(null, "in_progress"); + // Verify contract: null when completed expect(result).toBeNull(); }); }); @@ -210,49 +247,37 @@ const runCreateNewSessionTests = () => { setupMocksForNewSession(); }); + // CONTRACT: Returns Session object with required fields when problems available it("should create a new session with problems successfully", async () => { const mockProblems = createMockProblems(); ProblemService.createSession.mockResolvedValue(mockProblems); const result = await SessionService.createNewSession(); - expect(ProblemService.createSession).toHaveBeenCalled(); - expect(saveNewSessionToDB).toHaveBeenCalledWith( - expect.objectContaining({ - id: "test-uuid-123", - status: "in_progress", - problems: mockProblems, - attempts: [], - }) - ); - expect(saveSessionToStorage).toHaveBeenCalled(); - expect(result).toEqual(expect.objectContaining({ - id: "test-uuid-123", - status: "in_progress", - problems: mockProblems, - attempts: [], - })); + // Verify contract: returns session matching Session type definition + assertNewSessionContract(result); + expect(result.id).toBe("test-uuid-123"); + expect(result.problems).toEqual(mockProblems); + expect(result.attempts).toEqual([]); }); + // CONTRACT: Returns null when no problems are available it("should return null when no problems are available", async () => { ProblemService.createSession.mockResolvedValue([]); const result = await SessionService.createNewSession(); - expect(ProblemService.createSession).toHaveBeenCalled(); - expect(saveNewSessionToDB).not.toHaveBeenCalled(); - expect(saveSessionToStorage).not.toHaveBeenCalled(); + // Verify contract: null when no problems expect(result).toBeNull(); }); + // CONTRACT: Returns null when ProblemService fails it("should return null when ProblemService fails", async () => { ProblemService.createSession.mockResolvedValue(null); const result = await SessionService.createNewSession(); - expect(ProblemService.createSession).toHaveBeenCalled(); - expect(saveNewSessionToDB).not.toHaveBeenCalled(); - expect(saveSessionToStorage).not.toHaveBeenCalled(); + // Verify contract: null on failure expect(result).toBeNull(); }); }); @@ -260,6 +285,7 @@ const runCreateNewSessionTests = () => { const runSkipProblemTests = () => { describe("skipProblem", () => { + // CONTRACT: Returns session with problem removed when valid it("should remove problem from session and save to storage", async () => { const leetCodeID = "problem-123"; const mockSession = { @@ -275,24 +301,20 @@ const runSkipProblemTests = () => { const result = await SessionService.skipProblem(leetCodeID); - expect(getLatestSession).toHaveBeenCalled(); - expect(saveSessionToStorage).toHaveBeenCalledWith( - expect.objectContaining({ - problems: [{ leetcode_id: "problem-456", title: "Problem 2" }], - }), - true - ); + // Verify contract: returns session with specified problem removed + expect(result).not.toBeNull(); expect(result.problems).toHaveLength(1); expect(result.problems[0].leetcode_id).toBe("problem-456"); + expect(result.problems.find(p => p.leetcode_id === "problem-123")).toBeUndefined(); }); + // CONTRACT: Returns null when no session exists it("should return null when no session exists", async () => { getLatestSession.mockResolvedValue(null); const result = await SessionService.skipProblem("problem-123"); - expect(getLatestSession).toHaveBeenCalled(); - expect(saveSessionToStorage).not.toHaveBeenCalled(); + // Verify contract: null when no session expect(result).toBeNull(); }); });