Bryce
791e41cf34
- Added comprehensive test suite for account creation, validation, and grid views - Documented Datomic entity reference handling patterns in test comments - Created 5 test improvement todo documents addressing common test anti-patterns - Todo items cover: removing debug statements, fixing test nesting, strengthening assertions, extracting helpers, and removing doc-only tests
121 lines
3.9 KiB
Markdown
121 lines
3.9 KiB
Markdown
---
|
|
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)
|