|
| 1 | +# Plan: Update CookieCutter Test Template to Use For Loop Instead of Parametrize |
| 2 | + |
| 3 | +## Overview |
| 4 | + |
| 5 | +Update the cookiecutter template for `test_solution.py` to use a for loop approach for test cases instead of the current `@pytest.mark.parametrize` decorator. This will make the test structure more explicit and easier to read. |
| 6 | + |
| 7 | +## Current State Analysis |
| 8 | + |
| 9 | +### Current Template Structure |
| 10 | + |
| 11 | +- Uses `@pytest.mark.parametrize` decorator with `method.parametrize` and `method.test_cases` |
| 12 | +- Test cases are stored as a single string in JSON: `"[([1, 2, 3], [1, 2, 3], True), ...]"` |
| 13 | +- Generated test method signature includes all parameters: `(self, p_list: list[int | None], q_list: list[int | None], expected: bool)` |
| 14 | + |
| 15 | +### Current JSON Structure |
| 16 | + |
| 17 | +```json |
| 18 | +"_test_methods": { |
| 19 | + "list": [ |
| 20 | + { |
| 21 | + "name": "test_is_same_tree", |
| 22 | + "signature": "(self, p_list: list[int | None], q_list: list[int | None], expected: bool)", |
| 23 | + "parametrize": "p_list, q_list, expected", |
| 24 | + "test_cases": "[([1, 2, 3], [1, 2, 3], True), ([1, 2], [1, None, 2], False), ...]", |
| 25 | + "body": "result = run_is_same_tree(Solution, p_list, q_list)\nassert_is_same_tree(result, expected)" |
| 26 | + } |
| 27 | + ] |
| 28 | +} |
| 29 | +``` |
| 30 | + |
| 31 | +## Target State |
| 32 | + |
| 33 | +### New Template Structure |
| 34 | + |
| 35 | +- Keep `@pytest.mark.parametrize` decorator |
| 36 | +- Use a for loop to iterate through individual test cases from the list |
| 37 | +- Test method signature remains the same |
| 38 | +- Only change: `test_cases` becomes a list instead of a string |
| 39 | + |
| 40 | +### New JSON Structure |
| 41 | + |
| 42 | +```json |
| 43 | +"_test_methods": { |
| 44 | + "list": [ |
| 45 | + { |
| 46 | + "name": "test_is_same_tree", |
| 47 | + "signature": "(self, p_list: list[int | None], q_list: list[int | None], expected: bool)", |
| 48 | + "parametrize": "p_list, q_list, expected", |
| 49 | + "test_cases": { |
| 50 | + "list": [ |
| 51 | + "([1, 2, 3], [1, 2, 3], True)", |
| 52 | + "([1, 2], [1, None, 2], False)", |
| 53 | + "([1, 2, 1], [1, 1, 2], False)" |
| 54 | + ] |
| 55 | + }, |
| 56 | + "body": "result = run_is_same_tree(Solution, p_list, q_list)\nassert_is_same_tree(result, expected)" |
| 57 | + } |
| 58 | + ] |
| 59 | +} |
| 60 | +``` |
| 61 | + |
| 62 | +## Implementation Plan |
| 63 | + |
| 64 | +### Phase 1: Update CookieCutter Template |
| 65 | + |
| 66 | +1. **Update `test_solution.py` template** |
| 67 | + - Change from `{{method.test_cases}}` to `{{method.test_cases.list | join(', ')}}` |
| 68 | + - This follows the existing pattern used by other list fields in the template |
| 69 | + - Template line 30: `@pytest.mark.parametrize("{{method.parametrize}}", [{{method.test_cases.list | join(', ')}}])` |
| 70 | + - **✅ Validated:** Correctly follows existing "list" pattern used by `_tags`, `_readme_examples`, etc. |
| 71 | + |
| 72 | +### Phase 2: Create JSON Migration Script |
| 73 | + |
| 74 | +1. **Create `migrate_test_cases.py` script** |
| 75 | + - Parse existing JSON files in `leetcode_py/cli/resources/leetcode/json/problems/` |
| 76 | + - Convert `test_cases` string to `{"list": ["test_case1", "test_case2", ...]}` format |
| 77 | + - Keep all other fields exactly the same |
| 78 | + - Update only the `test_cases` field structure |
| 79 | + |
| 80 | +### Phase 3: Update JSON Files |
| 81 | + |
| 82 | +1. **Run migration script on all existing problem JSON files** |
| 83 | + - Process all 107 JSON files in the problems directory |
| 84 | + - Create backup of original files |
| 85 | + - Validate migrated JSON structure |
| 86 | + |
| 87 | +### Phase 4: Testing and Validation |
| 88 | + |
| 89 | +1. **Test generated templates** |
| 90 | + - Generate a test problem using updated template |
| 91 | + - Verify test cases run correctly with parametrize approach |
| 92 | + - Ensure JSON list format works with pytest parametrize |
| 93 | + |
| 94 | +2. **Comprehensive validation with all problems** |
| 95 | + - Copy additional LeetCode problems to `.cache/leetcode` (if not already present) |
| 96 | + - Regenerate ALL problems from new design using updated template |
| 97 | + - Copy existing solutions from cache to regenerated problems |
| 98 | + - Run tests on all regenerated problems to ensure they still pass |
| 99 | + - Verify no regressions in test functionality |
| 100 | + - Document any issues found and fix them |
| 101 | + |
| 102 | +### Phase 5: Update Documentation |
| 103 | + |
| 104 | +1. **Update problem creation documentation** |
| 105 | + - Update `.cursor/commands/problem-creation.md` to reflect new JSON structure |
| 106 | + - Change `test_cases` format from string to `{"list": [...]}` in examples |
| 107 | + - Update template examples to show new format |
| 108 | + - Add note about migration for existing problems |
| 109 | +2. **Update any other related documentation** |
| 110 | + - Check for other docs that reference `test_cases` format |
| 111 | + - Update examples in README or other guides if needed |
| 112 | + |
| 113 | +## Benefits of New Approach |
| 114 | + |
| 115 | +1. **Cleaner JSON Structure**: Test cases as `{"list": [...]}` object instead of single string |
| 116 | +2. **Better Maintainability**: Easier to edit individual test cases in JSON files |
| 117 | +3. **No Parsing Issues**: Avoids tuple conversion and complex JSON parsing |
| 118 | +4. **Consistency**: Aligns with other JSON list fields in the structure |
| 119 | +5. **Template Consistency**: Uses same pattern as other list fields (`| join(', ')`) |
| 120 | + |
| 121 | +## Files to Modify |
| 122 | + |
| 123 | +### Template Files |
| 124 | + |
| 125 | +- `leetcode_py/cli/resources/leetcode/{{cookiecutter.problem_name}}/test_solution.py` |
| 126 | + |
| 127 | +### Migration Script |
| 128 | + |
| 129 | +- `migrate_test_cases.py` (new file) |
| 130 | + |
| 131 | +### JSON Files |
| 132 | + |
| 133 | +- All files in `leetcode_py/cli/resources/leetcode/json/problems/` (107 files) |
| 134 | + |
| 135 | +### Generation Code |
| 136 | + |
| 137 | +- **No changes needed** - The `test_cases` field is created manually, not by automated code |
| 138 | +- The existing "list" pattern is already established and working correctly |
| 139 | +- Phase 4 removed after investigation showed no automated generation code exists |
| 140 | + |
| 141 | +## Implementation Steps |
| 142 | + |
| 143 | +1. ✅ Create this plan document |
| 144 | +2. ✅ Update cookiecutter template |
| 145 | +3. ✅ Create migration script |
| 146 | +4. ✅ Run migration on all JSON files |
| 147 | +5. ✅ Test updated template generation |
| 148 | +6. ✅ Validate all tests still work |
| 149 | +7. 🔄 Update documentation (separate step - do not combine with implementation) |
| 150 | + |
| 151 | +## Risk Mitigation |
| 152 | + |
| 153 | +1. **Backup Strategy**: Create backups of all JSON files before migration |
| 154 | +2. **Incremental Testing**: Test migration on a few files first |
| 155 | +3. **Rollback Plan**: Keep original template and migration script for rollback |
| 156 | +4. **Validation**: Ensure all existing tests still pass after migration |
| 157 | + |
| 158 | +## Success Criteria |
| 159 | + |
| 160 | +1. All existing JSON files successfully migrated to new list format |
| 161 | +2. Generated test templates work with JSON array format for test_cases |
| 162 | +3. All existing tests continue to pass with parametrize approach |
| 163 | +4. JSON structure is cleaner and more maintainable |
| 164 | +5. Template generation works correctly for new problems |
| 165 | +6. **All regenerated problems pass their tests** (comprehensive validation) |
| 166 | +7. No regressions in test functionality across the entire problem set |
0 commit comments