ralph/chore/fix.tests (#1578)

This commit is contained in:
Ralph Khreish
2026-01-15 15:46:15 +01:00
committed by GitHub
parent d4680f446d
commit 87ba3a2a5b
14 changed files with 162 additions and 147 deletions

View File

@@ -16,8 +16,8 @@
"test": "vitest run",
"test:watch": "vitest",
"test:coverage": "vitest run --coverage",
"test:unit": "vitest run -t unit",
"test:integration": "vitest run -t integration",
"test:unit": "vitest run '**/*.spec.ts'",
"test:integration": "vitest run '**/*.test.ts'",
"test:e2e": "vitest run --dir tests/e2e",
"test:ci": "vitest run --coverage --reporter=dot"
},

View File

@@ -77,8 +77,8 @@ describe('LoopCommand', () => {
mockTmCore = {
loop: {
run: mockLoopRun,
checkSandboxAuth: vi.fn().mockReturnValue(true),
runInteractiveAuth: vi.fn(),
checkSandboxAuth: vi.fn().mockReturnValue({ ready: true }),
runInteractiveAuth: vi.fn().mockReturnValue({ success: true }),
resolveIterations: vi.fn().mockImplementation((opts) => {
// Mirror the real implementation logic for accurate testing
if (opts.userIterations !== undefined) return opts.userIterations;
@@ -400,7 +400,7 @@ describe('LoopCommand', () => {
});
it('should run interactive auth when sandbox not ready', async () => {
mockTmCore.loop.checkSandboxAuth.mockReturnValue(false);
mockTmCore.loop.checkSandboxAuth.mockReturnValue({ ready: false });
const result = createMockResult();
mockLoopRun.mockResolvedValue(result);

View File

@@ -17,8 +17,8 @@
"test": "vitest run",
"test:watch": "vitest",
"test:coverage": "vitest run --coverage",
"test:unit": "vitest run -t unit",
"test:integration": "vitest run -t integration",
"test:unit": "vitest run '**/*.spec.ts'",
"test:integration": "vitest run '**/*.test.ts'",
"test:ci": "vitest run --coverage --reporter=dot"
},
"dependencies": {

View File

@@ -158,7 +158,7 @@ describe('generate MCP tool', () => {
const response = callMCPTool('generate', { projectRoot: testDir });
expect(response.data.orphanedFilesRemoved).toBe(1);
}, 15000);
}, 30000); // Longer timeout: this test makes 2 MCP calls (generate, then regenerate)
it('should accept output parameter for custom directory', () => {
const testData = createTasksFile({

View File

@@ -20,6 +20,9 @@
"turbo:dev": "turbo dev",
"turbo:build": "turbo build",
"turbo:typecheck": "turbo typecheck",
"turbo:test": "turbo test",
"turbo:test:unit": "turbo test:unit",
"turbo:test:integration": "turbo test:integration",
"build:build-config": "npm run build -w @tm/build-config",
"test": "cross-env NODE_ENV=test node --experimental-vm-modules node_modules/.bin/jest",
"test:unit": "node --experimental-vm-modules node_modules/.bin/jest --testPathPattern=unit",

View File

@@ -21,6 +21,8 @@
},
"scripts": {
"test": "vitest run",
"test:unit": "vitest run '**/*.spec.ts'",
"test:integration": "vitest run '**/*.test.ts'",
"test:watch": "vitest",
"test:coverage": "vitest run --coverage",
"lint": "biome check --write",

View File

@@ -34,72 +34,64 @@ describe('ConfigManager', () => {
}
});
// Setup default mock behaviors
vi.mocked(ConfigLoader).mockImplementation(
() =>
({
getDefaultConfig: vi.fn().mockReturnValue({
models: { main: 'default-model', fallback: 'fallback-model' },
storage: { type: 'file' },
version: '1.0.0'
}),
loadLocalConfig: vi.fn().mockResolvedValue(null),
loadGlobalConfig: vi.fn().mockResolvedValue(null),
hasLocalConfig: vi.fn().mockResolvedValue(false),
hasGlobalConfig: vi.fn().mockResolvedValue(false)
}) as any
);
// Setup default mock behaviors using class syntax for proper constructor mocking
vi.mocked(ConfigLoader).mockImplementation(function (this: any) {
this.getDefaultConfig = vi.fn().mockReturnValue({
models: { main: 'default-model', fallback: 'fallback-model' },
storage: { type: 'file' },
version: '1.0.0'
});
this.loadLocalConfig = vi.fn().mockResolvedValue(null);
this.loadGlobalConfig = vi.fn().mockResolvedValue(null);
this.hasLocalConfig = vi.fn().mockResolvedValue(false);
this.hasGlobalConfig = vi.fn().mockResolvedValue(false);
return this;
} as any);
vi.mocked(ConfigMerger).mockImplementation(
() =>
({
addSource: vi.fn(),
clearSources: vi.fn(),
merge: vi.fn().mockReturnValue({
models: { main: 'merged-model', fallback: 'fallback-model' },
storage: { type: 'file' }
}),
getSources: vi.fn().mockReturnValue([]),
hasSource: vi.fn().mockReturnValue(false),
removeSource: vi.fn().mockReturnValue(false)
}) as any
);
vi.mocked(ConfigMerger).mockImplementation(function (this: any) {
this.addSource = vi.fn();
this.clearSources = vi.fn();
this.merge = vi.fn().mockReturnValue({
models: { main: 'merged-model', fallback: 'fallback-model' },
storage: { type: 'file' }
});
this.getSources = vi.fn().mockReturnValue([]);
this.hasSource = vi.fn().mockReturnValue(false);
this.removeSource = vi.fn().mockReturnValue(false);
return this;
} as any);
vi.mocked(RuntimeStateManager).mockImplementation(
() =>
({
loadState: vi.fn().mockResolvedValue({ activeTag: 'master' }),
saveState: vi.fn().mockResolvedValue(undefined),
getCurrentTag: vi.fn().mockReturnValue('master'),
setCurrentTag: vi.fn().mockResolvedValue(undefined),
getState: vi.fn().mockReturnValue({ activeTag: 'master' }),
updateMetadata: vi.fn().mockResolvedValue(undefined),
clearState: vi.fn().mockResolvedValue(undefined)
}) as any
);
vi.mocked(RuntimeStateManager).mockImplementation(function (this: any) {
this.loadState = vi.fn().mockResolvedValue({ activeTag: 'master' });
this.saveState = vi.fn().mockResolvedValue(undefined);
this.getCurrentTag = vi.fn().mockReturnValue('master');
this.setCurrentTag = vi.fn().mockResolvedValue(undefined);
this.getState = vi.fn().mockReturnValue({ activeTag: 'master' });
this.updateMetadata = vi.fn().mockResolvedValue(undefined);
this.clearState = vi.fn().mockResolvedValue(undefined);
return this;
} as any);
vi.mocked(ConfigPersistence).mockImplementation(
() =>
({
saveConfig: vi.fn().mockResolvedValue(undefined),
configExists: vi.fn().mockResolvedValue(false),
deleteConfig: vi.fn().mockResolvedValue(undefined),
getBackups: vi.fn().mockResolvedValue([]),
restoreFromBackup: vi.fn().mockResolvedValue(undefined)
}) as any
);
vi.mocked(ConfigPersistence).mockImplementation(function (this: any) {
this.saveConfig = vi.fn().mockResolvedValue(undefined);
this.configExists = vi.fn().mockResolvedValue(false);
this.deleteConfig = vi.fn().mockResolvedValue(undefined);
this.getBackups = vi.fn().mockResolvedValue([]);
this.restoreFromBackup = vi.fn().mockResolvedValue(undefined);
return this;
} as any);
vi.mocked(EnvironmentConfigProvider).mockImplementation(
() =>
({
loadConfig: vi.fn().mockReturnValue({}),
getRuntimeState: vi.fn().mockReturnValue({}),
hasEnvVar: vi.fn().mockReturnValue(false),
getAllTaskmasterEnvVars: vi.fn().mockReturnValue({}),
addMapping: vi.fn(),
getMappings: vi.fn().mockReturnValue([])
}) as any
);
vi.mocked(EnvironmentConfigProvider).mockImplementation(function (
this: any
) {
this.loadConfig = vi.fn().mockReturnValue({});
this.getRuntimeState = vi.fn().mockReturnValue({});
this.hasEnvVar = vi.fn().mockReturnValue(false);
this.getAllTaskmasterEnvVars = vi.fn().mockReturnValue({});
this.addMapping = vi.fn();
this.getMappings = vi.fn().mockReturnValue([]);
return this;
} as any);
// Since constructor is private, we need to use the factory method
// But for testing, we'll create a test instance using create()
@@ -178,28 +170,30 @@ describe('ConfigManager', () => {
it('should return storage configuration', () => {
const storage = manager.getStorageConfig();
expect(storage).toEqual({ type: 'file' });
expect(storage).toEqual({
type: 'file',
basePath: testProjectRoot,
apiConfigured: false
});
});
it('should return API storage configuration when configured', async () => {
// Create a new instance with API storage config
vi.mocked(ConfigMerger).mockImplementationOnce(
() =>
({
addSource: vi.fn(),
clearSources: vi.fn(),
merge: vi.fn().mockReturnValue({
storage: {
type: 'api',
apiEndpoint: 'https://api.example.com',
apiAccessToken: 'token123'
}
}),
getSources: vi.fn().mockReturnValue([]),
hasSource: vi.fn().mockReturnValue(false),
removeSource: vi.fn().mockReturnValue(false)
}) as any
);
vi.mocked(ConfigMerger).mockImplementationOnce(function (this: any) {
this.addSource = vi.fn();
this.clearSources = vi.fn();
this.merge = vi.fn().mockReturnValue({
storage: {
type: 'api',
apiEndpoint: 'https://api.example.com',
apiAccessToken: 'token123'
}
});
this.getSources = vi.fn().mockReturnValue([]);
this.hasSource = vi.fn().mockReturnValue(false);
this.removeSource = vi.fn().mockReturnValue(false);
return this;
} as any);
const apiManager = await ConfigManager.create(testProjectRoot);
@@ -207,7 +201,9 @@ describe('ConfigManager', () => {
expect(storage).toEqual({
type: 'api',
apiEndpoint: 'https://api.example.com',
apiAccessToken: 'token123'
apiAccessToken: 'token123',
basePath: testProjectRoot,
apiConfigured: true
});
});

View File

@@ -2,17 +2,12 @@
* @fileoverview Unit tests for ConfigLoader service
*/
import fs from 'node:fs/promises';
import * as fsPromises from 'node:fs/promises';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { DEFAULT_CONFIG_VALUES } from '../../../common/interfaces/configuration.interface.js';
import { ConfigLoader } from './config-loader.service.js';
vi.mock('node:fs', () => ({
promises: {
readFile: vi.fn(),
access: vi.fn()
}
}));
vi.mock('node:fs/promises');
describe('ConfigLoader', () => {
let configLoader: ConfigLoader;
@@ -56,11 +51,13 @@ describe('ConfigLoader', () => {
storage: { type: 'api' as const }
};
vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(mockConfig));
vi.mocked(fsPromises.readFile).mockResolvedValue(
JSON.stringify(mockConfig)
);
const result = await configLoader.loadLocalConfig();
expect(fs.readFile).toHaveBeenCalledWith(
expect(fsPromises.readFile).toHaveBeenCalledWith(
'/test/project/.taskmaster/config.json',
'utf-8'
);
@@ -70,7 +67,7 @@ describe('ConfigLoader', () => {
it('should return null when config file does not exist', async () => {
const error = new Error('File not found') as any;
error.code = 'ENOENT';
vi.mocked(fs.readFile).mockRejectedValue(error);
vi.mocked(fsPromises.readFile).mockRejectedValue(error);
const result = await configLoader.loadLocalConfig();
@@ -79,7 +76,7 @@ describe('ConfigLoader', () => {
it('should throw TaskMasterError for other file errors', async () => {
const error = new Error('Permission denied');
vi.mocked(fs.readFile).mockRejectedValue(error);
vi.mocked(fsPromises.readFile).mockRejectedValue(error);
await expect(configLoader.loadLocalConfig()).rejects.toThrow(
'Failed to load local configuration'
@@ -87,7 +84,7 @@ describe('ConfigLoader', () => {
});
it('should throw error for invalid JSON', async () => {
vi.mocked(fs.readFile).mockResolvedValue('invalid json');
vi.mocked(fsPromises.readFile).mockResolvedValue('invalid json');
await expect(configLoader.loadLocalConfig()).rejects.toThrow();
});
@@ -102,18 +99,18 @@ describe('ConfigLoader', () => {
describe('hasLocalConfig', () => {
it('should return true when local config exists', async () => {
vi.mocked(fs.access).mockResolvedValue(undefined);
vi.mocked(fsPromises.access).mockResolvedValue(undefined);
const result = await configLoader.hasLocalConfig();
expect(fs.access).toHaveBeenCalledWith(
expect(fsPromises.access).toHaveBeenCalledWith(
'/test/project/.taskmaster/config.json'
);
expect(result).toBe(true);
});
it('should return false when local config does not exist', async () => {
vi.mocked(fs.access).mockRejectedValue(new Error('Not found'));
vi.mocked(fsPromises.access).mockRejectedValue(new Error('Not found'));
const result = await configLoader.hasLocalConfig();
@@ -123,18 +120,18 @@ describe('ConfigLoader', () => {
describe('hasGlobalConfig', () => {
it('should check global config path', async () => {
vi.mocked(fs.access).mockResolvedValue(undefined);
vi.mocked(fsPromises.access).mockResolvedValue(undefined);
const result = await configLoader.hasGlobalConfig();
expect(fs.access).toHaveBeenCalledWith(
expect(fsPromises.access).toHaveBeenCalledWith(
expect.stringContaining('.taskmaster/config.json')
);
expect(result).toBe(true);
});
it('should return false when global config does not exist', async () => {
vi.mocked(fs.access).mockRejectedValue(new Error('Not found'));
vi.mocked(fsPromises.access).mockRejectedValue(new Error('Not found'));
const result = await configLoader.hasGlobalConfig();

View File

@@ -2,19 +2,12 @@
* @fileoverview Unit tests for RuntimeStateManager service
*/
import fs from 'node:fs/promises';
import * as fs from 'node:fs/promises';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { DEFAULT_CONFIG_VALUES } from '../../../common/interfaces/configuration.interface.js';
import { RuntimeStateManager } from './runtime-state-manager.service.js';
vi.mock('node:fs', () => ({
promises: {
readFile: vi.fn(),
writeFile: vi.fn(),
mkdir: vi.fn(),
unlink: vi.fn()
}
}));
vi.mock('node:fs/promises');
describe('RuntimeStateManager', () => {
let stateManager: RuntimeStateManager;
@@ -96,15 +89,10 @@ describe('RuntimeStateManager', () => {
it('should handle invalid JSON gracefully', async () => {
vi.mocked(fs.readFile).mockResolvedValue('invalid json');
// Mock console.warn to avoid noise in tests
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
const state = await stateManager.loadState();
// Should return default state when JSON is invalid
expect(state.currentTag).toBe(DEFAULT_CONFIG_VALUES.TAGS.DEFAULT_TAG);
expect(warnSpy).toHaveBeenCalled();
warnSpy.mockRestore();
});
});
@@ -124,7 +112,7 @@ describe('RuntimeStateManager', () => {
// Verify writeFile was called with correct data
expect(fs.writeFile).toHaveBeenCalledWith(
'/test/project/.taskmaster/state.json',
expect.stringContaining('"activeTag":"test-tag"'),
expect.stringContaining('"currentTag": "test-tag"'),
'utf-8'
);

View File

@@ -102,7 +102,7 @@ describe('LoopDomain', () => {
tag: 'feature-branch'
};
const config = (loopDomain as any).buildConfig(fullConfig);
expect(config).toEqual(fullConfig);
expect(config).toMatchObject(fullConfig);
});
});

View File

@@ -122,7 +122,7 @@ describe('LoopService', () => {
});
describe('checkSandboxAuth()', () => {
it('should return true when output contains ok', () => {
it('should return ready=true when output contains ok', () => {
mockSpawnSync.mockReturnValue({
stdout: 'OK',
stderr: '',
@@ -135,7 +135,7 @@ describe('LoopService', () => {
const service = new LoopService(defaultOptions);
const result = service.checkSandboxAuth();
expect(result).toBe(true);
expect(result.ready).toBe(true);
expect(mockSpawnSync).toHaveBeenCalledWith(
'docker',
['sandbox', 'run', 'claude', '-p', 'Say OK'],
@@ -146,7 +146,7 @@ describe('LoopService', () => {
);
});
it('should return false when output does not contain ok', () => {
it('should return ready=false when output does not contain ok', () => {
mockSpawnSync.mockReturnValue({
stdout: 'Error: not authenticated',
stderr: '',
@@ -159,7 +159,7 @@ describe('LoopService', () => {
const service = new LoopService(defaultOptions);
const result = service.checkSandboxAuth();
expect(result).toBe(false);
expect(result.ready).toBe(false);
});
it('should check stderr as well as stdout', () => {
@@ -175,7 +175,7 @@ describe('LoopService', () => {
const service = new LoopService(defaultOptions);
const result = service.checkSandboxAuth();
expect(result).toBe(true);
expect(result.ready).toBe(true);
});
});
@@ -256,7 +256,7 @@ describe('LoopService', () => {
expect(mockSpawnSync).toHaveBeenCalledTimes(3);
});
it('should call spawnSync with docker sandbox run claude -p', async () => {
it('should call spawnSync with claude -p by default (non-sandbox)', async () => {
mockSpawnSync.mockReturnValue({
stdout: 'Done',
stderr: '',
@@ -274,14 +274,8 @@ describe('LoopService', () => {
});
expect(mockSpawnSync).toHaveBeenCalledWith(
'docker',
expect.arrayContaining([
'sandbox',
'run',
'claude',
'-p',
expect.any(String)
]),
'claude',
expect.arrayContaining(['-p', expect.any(String)]),
expect.objectContaining({
cwd: '/test/project'
})
@@ -397,7 +391,8 @@ describe('LoopService', () => {
expect(fsPromises.mkdir).toHaveBeenCalledWith('/test', {
recursive: true
});
expect(fsPromises.writeFile).toHaveBeenCalledWith(
// Uses appendFile instead of writeFile to preserve existing progress
expect(fsPromises.appendFile).toHaveBeenCalledWith(
'/test/progress.txt',
expect.stringContaining('# Task Master Loop Progress'),
'utf-8'
@@ -449,8 +444,8 @@ describe('LoopService', () => {
// Verify spawn was called with prompt containing iteration info
const spawnCall = mockSpawnSync.mock.calls[0];
// Args are ['sandbox', 'run', 'claude', '-p', prompt]
const promptArg = spawnCall[1][4];
// Args are ['-p', prompt, '--dangerously-skip-permissions'] for non-sandbox
const promptArg = spawnCall[1][1];
expect(promptArg).toContain('iteration 1 of 1');
});

View File

@@ -6,14 +6,15 @@
*
* FILE STORAGE (local):
* - Main tasks: "1", "2", "15"
* - Subtasks: "1.2", "15.3" (one level only)
* - Subtasks: "1.2", "15.3"
* - Sub-subtasks: "1.2.3", "15.3.1"
*
* API STORAGE (Hamster):
* - Main tasks: "HAM-1", "ham-1", "HAM1", "ham1" (all normalized to "HAM-1")
* - Variable-length prefixes: "PROJ-456", "TAS-1", "abc-999"
* - No subtasks (API doesn't use dot notation)
*
* NOT supported:
* - Deep nesting: "1.2.3" (file storage only has one subtask level)
* - API subtasks: "HAM-1.2" (doesn't exist)
*/
@@ -24,10 +25,10 @@ import { normalizeDisplayId } from '../../../common/schemas/task-id.schema.js';
* Pattern for validating a single task ID
* Permissive input - accepts with or without hyphen for API IDs
* - Numeric: "1", "15", "999"
* - Numeric subtasks: "1.2" (one level only)
* - API display IDs: "HAM-1", "ham-1", "HAM1", "ham1"
* - Numeric subtasks: "1.2", "1.2.3" (multi-level supported)
* - API display IDs: "HAM-1", "PROJ-456", "TAS-1", "abc-999"
*/
export const TASK_ID_PATTERN = /^(\d+(\.\d+)?|[A-Za-z]{3}-?\d+)$/;
export const TASK_ID_PATTERN = /^(\d+(\.\d+)*|[A-Za-z]+-?\d+)$/;
/**
* Validates a single task ID string
@@ -39,9 +40,10 @@ export const TASK_ID_PATTERN = /^(\d+(\.\d+)?|[A-Za-z]{3}-?\d+)$/;
* ```typescript
* isValidTaskIdFormat("1"); // true
* isValidTaskIdFormat("1.2"); // true
* isValidTaskIdFormat("1.2.3"); // true (multi-level subtasks)
* isValidTaskIdFormat("HAM-1"); // true
* isValidTaskIdFormat("PROJ-456"); // true (variable-length prefix)
* isValidTaskIdFormat("ham1"); // true (permissive input)
* isValidTaskIdFormat("1.2.3"); // false (too deep)
* isValidTaskIdFormat("HAM-1.2"); // false (no API subtasks)
* isValidTaskIdFormat("abc"); // false
* ```

View File

@@ -10,6 +10,8 @@
},
"scripts": {
"test": "vitest run",
"test:unit": "vitest run '**/*.spec.ts'",
"test:integration": "vitest run '**/*.test.ts'",
"test:watch": "vitest",
"test:coverage": "vitest run --coverage",
"lint": "biome check --write",

View File

@@ -39,6 +39,36 @@
"!{packages,apps}/**/node_modules/**"
],
"outputLogs": "new-only"
},
"test": {
"dependsOn": ["^build"],
"inputs": [
"$TURBO_DEFAULT$",
"!{packages,apps}/**/dist/**",
"!{packages,apps}/**/node_modules/**"
],
"outputLogs": "new-only",
"cache": false
},
"test:unit": {
"dependsOn": ["^build"],
"inputs": [
"$TURBO_DEFAULT$",
"!{packages,apps}/**/dist/**",
"!{packages,apps}/**/node_modules/**"
],
"outputLogs": "new-only",
"cache": false
},
"test:integration": {
"dependsOn": ["^build"],
"inputs": [
"$TURBO_DEFAULT$",
"!{packages,apps}/**/dist/**",
"!{packages,apps}/**/node_modules/**"
],
"outputLogs": "new-only",
"cache": false
}
},
"globalDependencies": ["turbo.json", "tsconfig.json", ".env*"]