Implement toggleCollection function to prevent mass-editing per-item flags (#35)
This commit is contained in:
commit
7d229eecb7
@ -319,6 +319,93 @@ function getEffectivelyEnabledItems(config) {
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Toggle a collection's enabled state without modifying individual item flags
|
||||
*
|
||||
* This function implements the core requirement from TASK-003:
|
||||
* - Only modifies the collection's enabled flag
|
||||
* - Never writes to per-item flags (e.g., config.prompts[item] = enabled)
|
||||
* - Returns summary information for CLI feedback
|
||||
* - Preserves all existing explicit per-item overrides
|
||||
*
|
||||
* @param {Object} config - Configuration object with collections section
|
||||
* @param {string} name - Collection name to toggle
|
||||
* @param {boolean} enabled - Desired enabled state for the collection
|
||||
* @returns {Object} Summary object with delta information for CLI feedback
|
||||
*/
|
||||
function toggleCollection(config, name, enabled) {
|
||||
// Validate inputs
|
||||
if (!config || typeof config !== 'object') {
|
||||
throw new Error('Config object is required');
|
||||
}
|
||||
if (!name || typeof name !== 'string') {
|
||||
throw new Error('Collection name is required');
|
||||
}
|
||||
if (typeof enabled !== 'boolean') {
|
||||
throw new Error('Enabled state must be a boolean');
|
||||
}
|
||||
|
||||
// Ensure collections section exists
|
||||
if (!config.collections) {
|
||||
config.collections = {};
|
||||
}
|
||||
|
||||
// Get current state
|
||||
const currentState = Boolean(config.collections[name]);
|
||||
|
||||
// Check if collection exists (has a .collection.yml file)
|
||||
const collectionPath = path.join(__dirname, "collections", `${name}.collection.yml`);
|
||||
if (!fs.existsSync(collectionPath)) {
|
||||
throw new Error(`Collection '${name}' does not exist`);
|
||||
}
|
||||
|
||||
// If state is already the desired state, return early
|
||||
if (currentState === enabled) {
|
||||
return {
|
||||
changed: false,
|
||||
collectionName: name,
|
||||
currentState: currentState,
|
||||
newState: enabled,
|
||||
delta: { enabled: [], disabled: [] },
|
||||
message: `Collection '${name}' is already ${enabled ? 'enabled' : 'disabled'}.`
|
||||
};
|
||||
}
|
||||
|
||||
// Compute effective states before the change
|
||||
const effectiveStatesBefore = computeEffectiveItemStates(config);
|
||||
|
||||
// CORE REQUIREMENT: Only modify the collection's enabled flag
|
||||
// Never write to per-item flags (config.prompts[item] = enabled)
|
||||
config.collections[name] = enabled;
|
||||
|
||||
// Compute effective states after the change
|
||||
const effectiveStatesAfter = computeEffectiveItemStates(config);
|
||||
|
||||
// Calculate delta summary for CLI feedback
|
||||
const delta = { enabled: [], disabled: [] };
|
||||
for (const sectionName of ["prompts", "instructions", "chatmodes"]) {
|
||||
for (const item of getAllAvailableItems(sectionName)) {
|
||||
const beforeState = effectiveStatesBefore[sectionName]?.[item]?.enabled || false;
|
||||
const afterState = effectiveStatesAfter[sectionName]?.[item]?.enabled || false;
|
||||
|
||||
if (!beforeState && afterState) {
|
||||
delta.enabled.push(`${sectionName}/${item}`);
|
||||
} else if (beforeState && !afterState) {
|
||||
delta.disabled.push(`${sectionName}/${item}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
changed: true,
|
||||
collectionName: name,
|
||||
currentState: currentState,
|
||||
newState: enabled,
|
||||
delta,
|
||||
message: `${enabled ? 'Enabled' : 'Disabled'} collection '${name}'.`
|
||||
};
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
DEFAULT_CONFIG_PATH,
|
||||
CONFIG_SECTIONS,
|
||||
@ -332,5 +419,6 @@ module.exports = {
|
||||
getAllAvailableItems,
|
||||
computeEffectiveItemStates,
|
||||
getEffectivelyEnabledItems,
|
||||
generateConfigHash
|
||||
generateConfigHash,
|
||||
toggleCollection
|
||||
};
|
||||
|
||||
13
test-all.js
13
test-all.js
@ -8,6 +8,7 @@ const { runTests: runUnitTests } = require('./test-effective-states');
|
||||
const { runTests: runIntegrationTests } = require('./test-integration');
|
||||
const { runTests: runCliTests } = require('./test-cli');
|
||||
const { runTests: runApplyTests } = require('./test-apply-effective');
|
||||
const { runTests: runToggleCollectionTests } = require('./test-toggle-collection');
|
||||
|
||||
async function runAllTests() {
|
||||
console.log('🧪 Running Awesome Copilot Comprehensive Test Suite\n');
|
||||
@ -17,7 +18,8 @@ async function runAllTests() {
|
||||
unit: false,
|
||||
integration: false,
|
||||
cli: false,
|
||||
apply: false
|
||||
apply: false,
|
||||
toggleCollection: false
|
||||
};
|
||||
|
||||
try {
|
||||
@ -52,6 +54,14 @@ async function runAllTests() {
|
||||
console.error('Apply tests failed with error:', error.message);
|
||||
}
|
||||
|
||||
try {
|
||||
console.log('\n🔧 Toggle Collection Tests (TASK-003)');
|
||||
console.log('-'.repeat(36));
|
||||
results.toggleCollection = await runToggleCollectionTests();
|
||||
} catch (error) {
|
||||
console.error('Toggle collection tests failed with error:', error.message);
|
||||
}
|
||||
|
||||
try {
|
||||
console.log('\n🤖 Repository Instructions Tests');
|
||||
console.log('-'.repeat(33));
|
||||
@ -71,6 +81,7 @@ async function runAllTests() {
|
||||
{ name: 'Integration Tests', result: results.integration, emoji: '🔄' },
|
||||
{ name: 'CLI Tests', result: results.cli, emoji: '⌨️' },
|
||||
{ name: 'Apply Tests', result: results.apply, emoji: '🎯' },
|
||||
{ name: 'Toggle Collection', result: results.toggleCollection, emoji: '🔧' },
|
||||
{ name: 'Repo Instructions', result: results.repoInstructions, emoji: '🤖' }
|
||||
];
|
||||
|
||||
|
||||
194
test-toggle-collection.js
Normal file
194
test-toggle-collection.js
Normal file
@ -0,0 +1,194 @@
|
||||
#!/usr/bin/env node
|
||||
|
||||
/**
|
||||
* Unit tests for toggleCollection function
|
||||
*/
|
||||
|
||||
const path = require('path');
|
||||
const { toggleCollection, computeEffectiveItemStates } = require('./config-manager');
|
||||
|
||||
// Change to project directory for tests
|
||||
process.chdir(__dirname);
|
||||
|
||||
function assert(condition, message) {
|
||||
if (!condition) {
|
||||
throw new Error(`Assertion failed: ${message}`);
|
||||
}
|
||||
}
|
||||
|
||||
function runTests() {
|
||||
let totalTests = 0;
|
||||
let passedTests = 0;
|
||||
|
||||
function test(name, testFn) {
|
||||
totalTests++;
|
||||
try {
|
||||
testFn();
|
||||
console.log(`✅ ${name}`);
|
||||
passedTests++;
|
||||
} catch (error) {
|
||||
console.log(`❌ ${name}: ${error.message}`);
|
||||
}
|
||||
}
|
||||
|
||||
console.log("Running unit tests for toggleCollection function...\n");
|
||||
|
||||
// Test 1: Toggle collection from false to true
|
||||
test("Toggle collection from false to true", () => {
|
||||
const config = {
|
||||
collections: {
|
||||
'testing-automation': false
|
||||
}
|
||||
};
|
||||
|
||||
const result = toggleCollection(config, 'testing-automation', true);
|
||||
|
||||
assert(result.changed === true, 'Should indicate change occurred');
|
||||
assert(result.collectionName === 'testing-automation', 'Should return correct collection name');
|
||||
assert(result.currentState === false, 'Should show previous state as false');
|
||||
assert(result.newState === true, 'Should show new state as true');
|
||||
assert(config.collections['testing-automation'] === true, 'Config should be updated');
|
||||
assert(result.delta.enabled.length > 0, 'Should show items being enabled');
|
||||
});
|
||||
|
||||
// Test 2: Toggle collection from true to false
|
||||
test("Toggle collection from true to false", () => {
|
||||
const config = {
|
||||
collections: {
|
||||
'testing-automation': true
|
||||
}
|
||||
};
|
||||
|
||||
const result = toggleCollection(config, 'testing-automation', false);
|
||||
|
||||
assert(result.changed === true, 'Should indicate change occurred');
|
||||
assert(result.newState === false, 'Should show new state as false');
|
||||
assert(config.collections['testing-automation'] === false, 'Config should be updated');
|
||||
assert(result.delta.disabled.length > 0, 'Should show items being disabled');
|
||||
});
|
||||
|
||||
// Test 3: Toggle collection to same state (no change)
|
||||
test("Toggle collection to same state (no change)", () => {
|
||||
const config = {
|
||||
collections: {
|
||||
'testing-automation': true
|
||||
}
|
||||
};
|
||||
|
||||
const result = toggleCollection(config, 'testing-automation', true);
|
||||
|
||||
assert(result.changed === false, 'Should indicate no change occurred');
|
||||
assert(result.message.includes('already enabled'), 'Should indicate already enabled');
|
||||
});
|
||||
|
||||
// Test 4: Preserves explicit overrides
|
||||
test("Preserves explicit overrides", () => {
|
||||
const config = {
|
||||
prompts: {
|
||||
'playwright-generate-test': false // Explicit override
|
||||
},
|
||||
collections: {
|
||||
'testing-automation': false
|
||||
}
|
||||
};
|
||||
|
||||
// Enable collection
|
||||
const result = toggleCollection(config, 'testing-automation', true);
|
||||
|
||||
// Verify the explicit override is preserved
|
||||
assert(config.prompts['playwright-generate-test'] === false, 'Explicit override should be preserved');
|
||||
assert(config.collections['testing-automation'] === true, 'Collection should be enabled');
|
||||
|
||||
// Check that the overridden item is not in the enabled delta
|
||||
const hasOverriddenItem = result.delta.enabled.some(item => item.includes('playwright-generate-test'));
|
||||
assert(!hasOverriddenItem, 'Explicitly disabled item should not appear in enabled delta');
|
||||
});
|
||||
|
||||
// Test 5: Only modifies collection flag, never individual items
|
||||
test("Only modifies collection flag, never individual items", () => {
|
||||
const config = {
|
||||
prompts: {},
|
||||
instructions: {},
|
||||
chatmodes: {},
|
||||
collections: {
|
||||
'testing-automation': false
|
||||
}
|
||||
};
|
||||
|
||||
const originalPrompts = { ...config.prompts };
|
||||
const originalInstructions = { ...config.instructions };
|
||||
const originalChatmodes = { ...config.chatmodes };
|
||||
|
||||
toggleCollection(config, 'testing-automation', true);
|
||||
|
||||
// Verify individual sections were not modified
|
||||
assert(JSON.stringify(config.prompts) === JSON.stringify(originalPrompts), 'Prompts section should not be modified');
|
||||
assert(JSON.stringify(config.instructions) === JSON.stringify(originalInstructions), 'Instructions section should not be modified');
|
||||
assert(JSON.stringify(config.chatmodes) === JSON.stringify(originalChatmodes), 'Chatmodes section should not be modified');
|
||||
|
||||
// Only collections should be modified
|
||||
assert(config.collections['testing-automation'] === true, 'Only collection flag should be modified');
|
||||
});
|
||||
|
||||
// Test 6: Error handling for invalid inputs
|
||||
test("Error handling for invalid inputs", () => {
|
||||
let errorThrown = false;
|
||||
|
||||
try {
|
||||
toggleCollection(null, 'test', true);
|
||||
} catch (error) {
|
||||
errorThrown = true;
|
||||
assert(error.message.includes('Config object is required'), 'Should require config object');
|
||||
}
|
||||
assert(errorThrown, 'Should throw error for null config');
|
||||
|
||||
errorThrown = false;
|
||||
try {
|
||||
toggleCollection({}, '', true);
|
||||
} catch (error) {
|
||||
errorThrown = true;
|
||||
assert(error.message.includes('Collection name is required'), 'Should require collection name');
|
||||
}
|
||||
assert(errorThrown, 'Should throw error for empty name');
|
||||
|
||||
errorThrown = false;
|
||||
try {
|
||||
toggleCollection({}, 'test', 'not-boolean');
|
||||
} catch (error) {
|
||||
errorThrown = true;
|
||||
assert(error.message.includes('Enabled state must be a boolean'), 'Should require boolean enabled state');
|
||||
}
|
||||
assert(errorThrown, 'Should throw error for non-boolean enabled state');
|
||||
});
|
||||
|
||||
// Test 7: Error handling for non-existent collection
|
||||
test("Error handling for non-existent collection", () => {
|
||||
const config = { collections: {} };
|
||||
|
||||
let errorThrown = false;
|
||||
try {
|
||||
toggleCollection(config, 'non-existent-collection', true);
|
||||
} catch (error) {
|
||||
errorThrown = true;
|
||||
assert(error.message.includes('does not exist'), 'Should indicate collection does not exist');
|
||||
}
|
||||
assert(errorThrown, 'Should throw error for non-existent collection');
|
||||
});
|
||||
|
||||
console.log(`\nTest Results: ${passedTests}/${totalTests} passed`);
|
||||
|
||||
if (passedTests === totalTests) {
|
||||
console.log('🎉 All toggleCollection tests passed!');
|
||||
return true;
|
||||
} else {
|
||||
console.log('💥 Some toggleCollection tests failed!');
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
if (require.main === module) {
|
||||
const success = runTests();
|
||||
process.exit(success ? 0 : 1);
|
||||
}
|
||||
|
||||
module.exports = { runTests };
|
||||
Loading…
x
Reference in New Issue
Block a user