---
status: pending
priority: p2
issue_id: 003
tags: [test-fix, important, assertion-quality]
---

# Problem Statement

**Weak assertions in multiple tests**

Multiple tests only verify type checking `(is (number? matching-count))` instead of verifying actual functionality. This gives false confidence that tests are validating the system behavior.

**Impact**: Tests pass even when incorrect data is returned, providing minimal test coverage and risking regressions.

## Findings

- **Location**: Multiple test functions
  - Line 100 (account-grid-view-loads-accounts): `(is (number? matching-count))`
  - Line 110 (account-grid-displays-correct-columns): `(is (number? matching-count))`
  - Line 126 (account-sorting-by-name): `(is (number? matching-count))`
  - Line 137 (account-sorting-by-numeric-code): `(is (number? matching-count))`
  - Line 150 (account-sorting-by-type): `(is (number? matching-count))`
  - Line 28 (account-creation-success): `(is (= 1 (count accounts)))`
  - Line 30 (account-creation-success): `(is (= 12345 (:account/numeric-code account)))`
  - Line 41 (account-creation-success): `(is (= "B" (:account/location account)))`

- **Issue**: Some tests verify only the count of accounts returned, not that the correct accounts are returned

- **Example of weak assertion**:
  ```clojure
  ;; Line 100 - Only checks type, not functionality
  (is (number? matching-count))

  ;; Should verify:
  ;; - The correct accounts were returned
  ;; - The accounts have the expected data
  ;; - Data integrity is maintained
  ```

- **Evidence**: Code review identified that ~40% of assertions are weak type-checking assertions

## Proposed Solutions

### Option 1: Verify actual account data returned (RECOMMENDED)
**Pros**:
- Strong validation that tests verify actual behavior
- Catches regressions more effectively
- Better test confidence

**Cons**:
- Requires test setup of expected account data
- More complex test code

**Effort**: Medium

**Risk**: None

**Example implementation**:
```clojure
;; Line 100 - Account grid view
(let [[accounts matching-count] (sut/fetch-page {:query-params {:page 1 :per-page 10}})]
  (is (number? matching-count))
  (is (pos? (count accounts)))  ; Verify accounts exist
  (is (every? #(contains? % :account/name) accounts)))  ; Verify required fields
```

### Option 2: Combine with data verification from test setup
**Pros**:
- Consistent data verification
- Checks both creation and retrieval

**Cons**:
- Requires each test to create expected data
- More code duplication

**Effort**: Medium

**Risk**: None

### Option 3: Accept current weak assertions for now
**Pros**:
- Minimal changes
- Maintains current test structure

**Cons**:
- Low test confidence
- Doesn't actually verify functionality
- More likely to miss regressions

**Effort**: None

**Risk**: High (tests are not effective)

## Recommended Action

**Strengthen assertions to verify actual account data returned, not just type checking.**

For grid and sorting tests, verify that:
1. Accounts are actually returned (not just matching-count exists)
2. Returned accounts have required fields (account/name, account/numeric-code, etc.)
3. For grid tests, verify accounts match what was created

## Technical Details

**Affected Component**: Test assertions, account grid and sorting functionality

**Affected Files**:
- `test/clj/auto_ap/ssr/admin/accounts_test.clj` (lines 100, 110, 126, 137, 150)

**Database Changes**: None

**API Changes**: None

**Related Code**: None

## Acceptance Criteria

- [ ] Grid tests verify accounts are returned and have required fields
- [ ] Sorting tests verify accounts are returned
- [ ] All weak assertions strengthened to verify actual data
- [ ] All tests still pass with strengthened assertions
- [ ] Tests provide real confidence in system behavior

## Work Log

- **2026-02-06**: Finding documented through code review agents (kieran-python-reviewer, code-simplicity-reviewer)

## Resources

- **Review Agents**:
  - kieran-python-reviewer (task ses_3c958be13ffehUVu6E1NzOC3Ch)
  - code-simplicity-reviewer (task ses_3c958304dffeVRAAPfMSv583Ik)