From 63ef0d36caf1f38569c1dd84557b67793a48e2dd Mon Sep 17 00:00:00 2001
From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com>
Date: Tue, 9 Sep 2025 01:04:56 +0000
Subject: [PATCH] fix: apply code review fixes for dependency-manager tests

- Add isSilentMode mock to prevent runtime errors
- Fix formatTaskId mock to return numbers for top-level tasks
- Remove incorrect resolves.not.toThrow assertions
- Use consistent TEST_TASKS_PATH constant
- Add proper process.exit mocking for cross-level tests
- Fix dependency assertions to expect correct types

Co-authored-by: Ralph Khreish <Crunchyman-ralph@users.noreply.github.com>
---
 tests/unit/dependency-manager.test.js | 48 +++++++++++----------------
 1 file changed, 19 insertions(+), 29 deletions(-)

diff --git a/tests/unit/dependency-manager.test.js b/tests/unit/dependency-manager.test.js
index b90b1a9d..6d784bcb 100644
--- a/tests/unit/dependency-manager.test.js
+++ b/tests/unit/dependency-manager.test.js
@@ -41,7 +41,8 @@ jest.mock('../../scripts/modules/utils.js', () => ({
 	writeJSON: mockWriteJSON,
 	taskExists: mockTaskExists,
 	formatTaskId: mockFormatTaskId,
-	findCycles: mockFindCycles
+	findCycles: mockFindCycles,
+	isSilentMode: jest.fn(() => true)
 }));
 
 jest.mock('../../scripts/modules/ui.js', () => ({
@@ -918,7 +919,11 @@ describe('Dependency Manager Module', () => {
 	});
 
 	describe('Cross-level dependency tests (Issue #542)', () => {
+		let originalExit;
+
 		beforeEach(() => {
+			originalExit = process.exit;
+			process.exit = jest.fn();
 			// Set up test data that matches the issue report
 			mockReadJSON.mockReturnValue({
 				tasks: [
@@ -972,23 +977,20 @@ describe('Dependency Manager Module', () => {
 			});
 
 			mockFormatTaskId.mockImplementation((id) => {
-				if (typeof id === 'string' && id.includes('.')) {
-					return id;
-				}
-				if (typeof id === 'number') {
-					return id.toString();
-				}
-				return id;
+				if (typeof id === 'string' && id.includes('.')) return id; // keep dot notation
+				return parseInt(id, 10);              // normalize top-level task IDs to number
 			});
 		});
 
+		afterEach(() => {
+			process.exit = originalExit;
+		});
+
 		test('should allow subtask to depend on top-level task', async () => {
 			const { addDependency } = await import('../../scripts/modules/dependency-manager.js');
-			const testTasksPath = 'test-tasks.json';
 
 			// Test the specific scenario from Issue #542: subtask 2.2 depending on task 11
-			await expect(addDependency(testTasksPath, '2.2', 11, { projectRoot: '/test' }))
-				.resolves.not.toThrow();
+			await addDependency(TEST_TASKS_PATH, '2.2', 11, { projectRoot: '/test' });
 
 			// Verify mockWriteJSON was called (meaning the dependency was added)
 			expect(mockWriteJSON).toHaveBeenCalled();
@@ -999,16 +1001,14 @@ describe('Dependency Manager Module', () => {
 			const subtask22 = savedData.tasks[0].subtasks[1];
 			
 			// Verify the dependency was actually added to subtask 2.2
-			expect(subtask22.dependencies).toContain('11');
+			expect(subtask22.dependencies).toContain(11);
 		});
 
 		test('should allow top-level task to depend on subtask', async () => {
 			const { addDependency } = await import('../../scripts/modules/dependency-manager.js');
-			const testTasksPath = 'test-tasks.json';
 
 			// Test reverse scenario: task 11 depending on subtask 2.1
-			await expect(addDependency(testTasksPath, 11, '2.1', { projectRoot: '/test' }))
-				.resolves.not.toThrow();
+			await addDependency(TEST_TASKS_PATH, 11, '2.1', { projectRoot: '/test' });
 
 			// Verify mockWriteJSON was called
 			expect(mockWriteJSON).toHaveBeenCalled();
@@ -1045,21 +1045,11 @@ describe('Dependency Manager Module', () => {
 			});
 
 			const { addDependency } = await import('../../scripts/modules/dependency-manager.js');
-			const testTasksPath = 'test-tasks.json';
 
-			// Mock process.exit to prevent the test from actually exiting
-			const originalExit = process.exit;
-			process.exit = jest.fn();
-
-			try {
-				await addDependency(testTasksPath, '2.2', 99, { projectRoot: '/test' });
-				
-				// If we get here, the function didn't exit as expected
-				expect(process.exit).toHaveBeenCalledWith(1);
-			} finally {
-				// Restore original process.exit
-				process.exit = originalExit;
-			}
+			await addDependency(TEST_TASKS_PATH, '2.2', 99, { projectRoot: '/test' });
+			
+			// Verify that process.exit was called due to non-existent dependency target
+			expect(process.exit).toHaveBeenCalledWith(1);
 		});
 	});
 });