--- status: complete priority: p1 issue_id: 002 tags: [test-fix, critical, test-structure] --- # Problem Statement **Improperly nested deftest blocks in sorting tests** Lines 129-141 contain incorrectly nested `deftest` blocks. The `account-sorting-by-numeric-code` and potentially other sorting tests are nested inside the `testing` block of `account-sorting-by-name`, which violates test organization principles. **Impact**: Test structure is broken, unclear which tests are top-level vs nested, potential confusion about test execution order and scope. ## Findings - **Location**: `test/clj/auto_ap/ssr/admin/accounts_test.clj` lines 129-151 - **Current code structure**: ```clojure (deftest account-sorting-by-name (testing "Account sorting by name should work" (with-redefs [...]) (create-account ...) (create-account ...) (let [[accounts matching-count] (sut/fetch-page ...)] ; <-- THIS IS TOP LEVEL (is (number? matching-count))))) ; ERROR: deftest appears here, NOT inside the testing block (deftest account-sorting-by-numeric-code (testing "Account sorting by numeric code should work (default)" (with-redefs [...]) ... (deftest account-sorting-by-type (testing "Account sorting by type should work" (with-redefs [...]) ... ``` - **Issue**: The `deftest account-sorting-by-numeric-code` (line 129) and `deftest account-sorting-by-type` (line 142) appear to be at the wrong indentation level. Looking at line 118, `account-sorting-by-name` starts at column 0 with `(deftest`, so the other deftests at lines 129 and 142 should also be at column 0, not nested. - **Evidence**: Code review by code-simplicity-reviewer identified improper nesting causing broken test structure ## Proposed Solutions ### Option 1: Fix indentation - make all deftests top level (RECOMMENDED) **Pros**: - Restores proper test structure - All tests are independent and can run in any order - Matches expected test organization patterns **Cons**: - None **Effort**: Small (indentation fix) **Risk**: None ### Option 2: Move deftests inside the outer testing block **Pros**: - Some tests logically belong together **Cons**: - Unconventional nesting structure - Makes tests less independent - Could cause confusion **Effort**: Medium **Risk**: Medium (breaks test independence) ### Option 3: Remove the outer testing block for sorting tests **Pros**: - Simpler structure **Cons**: - Loses descriptive testing block - Less clear test intent **Effort**: Small **Risk**: None ## Recommended Action **Fix indentation to make all deftests top-level with their own testing blocks.** Based on the pattern at line 18-19 where `(deftest account-creation-success)` starts at column 0 with its own `(testing "..."` on the next line, the sorting tests should follow the same pattern at lines 118, 129, and 142. ## Technical Details **Affected Component**: Test suite, specifically account sorting test organization **Affected Files**: - `test/clj/auto_ap/ssr/admin/accounts_test.clj` (lines 118-151) **Database Changes**: None **API Changes**: None **Related Code**: None ## Acceptance Criteria - [ ] All `deftest` blocks are at top level (column 0) - [ ] Each deftest has its own `(testing "..."` block - [ ] Test structure follows consistent pattern with other tests - [ ] All tests still pass - [ ] Tests can run independently (no nesting issues) ## Work Log - **2026-02-06**: Finding documented through code review agents (code-simplicity-reviewer, pattern-recognition-specialist) - **2026-02-06**: Fixed indentation of `account-sorting-by-numeric-code` and `account-sorting-by-type` deftests from 2-space indentation to 0-space (top-level) - **2026-02-06**: Verified all tests pass (0 failures, 0 errors) and structure is consistent with other tests ## Resources - **Review Agents**: - code-simplicity-reviewer (task ses_3c958304dffeVRAAPfMSv583Ik) - pattern-recognition-specialist (task ses_3c9578ae3ffejQg4Wl4GT1ZSAk)