Files
integreat/todos/012-pending-p3-improve-error-testing-pattern.md
Bryce a7daf839ec feat(tests): Add comprehensive tests for SSR admin vendors module
Add 8 BDD-style tests for the vendors module covering grid/list
operations and vendor merge functionality. Tests follow established
patterns from accounts_test.clj and include proper database
verification.

Tests Implemented:
- vendor-grid-loads-with-empty-database
- vendor-fetch-ids-returns-correct-structure
- vendor-fetch-page-returns-vendors
- vendor-hydrate-results-works
- vendor-merge-transfers-references
- vendor-merge-same-vendor-rejected
- vendor-merge-invalid-vendor-handled
- vendor-hydration-includes-all-fields

Key Implementation Details:
- Uses setup-test-data helper with unique temp IDs
- Tests focus on public interface (fetch-page, merge-submit)
- Follows BDD Given/When/Then pattern
- All 8 tests passing (26 assertions)

Documentation:
- Created implementation plan in docs/plans/
- Documented solution patterns in docs/solutions/
- Created code review todos for future improvements

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2026-02-06 23:53:31 -08:00

4.1 KiB

status, priority, issue_id, tags, dependencies
status priority issue_id tags dependencies
pending p3 012
testing
error-handling
patterns
vendors
assertions

Improve Error Testing Pattern for Validation Errors

Problem Statement

The vendor merge same-vendor test uses thrown-with-msg? which only checks the error message string. The accounts_test.clj pattern uses try/catch with ex-data assertions, which is more robust and tests the structured error data, not just the message.

Findings

From kieran-rails-reviewer:

Current vendors_test.clj pattern (lines 129-130):

(is (thrown-with-msg? clojure.lang.ExceptionInfo
                      #"Please select two different vendors"
                      (sut/merge-submit ...)))

accounts_test.clj pattern (lines 45-58):

(try
  (sut/account-save {...})
  (is false "Should have thrown validation error")
  (catch clojure.lang.ExceptionInfo e
    (let [data (ex-data e)]
      (is (= :field-validation (:type data)))
      (is (contains? (:form-errors data) :account/numeric-code))
      (is (str/includes? (get-in (:form-errors data) [:account/numeric-code]) 
                         "already in use")))))

Benefits of accounts_test.clj pattern:

  • Tests structured error data, not just message
  • More specific about what failed
  • Can assert on error type and field-specific errors
  • Less brittle (message strings can change)

Proposed Solutions

Effort: Small (10 minutes) Risk: Low

Refactor same-vendor merge test to use try/catch:

(deftest vendor-merge-same-vendor-rejected
  (testing "Vendor merge should reject when source and target are the same"
    (let [admin-identity (admin-token)
          vendor-temp-id (str "vendor-solo-" (rand-int 100000))
          tempids (setup-test-data [(assoc (create-vendor "Solo Vendor")
                                           :db/id vendor-temp-id)])
          vendor-id (get tempids vendor-temp-id)]
      (try
        (sut/merge-submit {:form-params {:source-vendor vendor-id
                                         :target-vendor vendor-id}
                           :request-method :put
                           :identity admin-identity})
        (is false "Should have thrown validation error for same vendor merge")
        (catch clojure.lang.ExceptionInfo e
          (let [data (ex-data e)]
            (is (= :form-validation (:type data)))
            (is (str/includes? (:form-validation-errors data) 
                               "Please select two different vendors"))))))))

Pros:

  • More robust error testing
  • Matches accounts_test.clj pattern
  • Tests structured error data

Cons:

  • More verbose
  • Requires checking source code for error structure

Option B: Keep Current Pattern

Effort: None Risk: Low

Leave as-is with thrown-with-msg?.

Pros:

  • Already working
  • Simpler code

Cons:

  • Less robust
  • Only tests message string
  • Different from accounts_test.clj

Go with Option A - use try/catch pattern with ex-data assertions. This provides more robust testing and matches the accounts_test.clj convention.

Technical Details

Affected Test:

  • Lines 119-134: vendor-merge-same-vendor-rejected

Source Investigation: Need to check vendors.clj or utils.clj to see the actual error structure:

;; Check what ex-data contains:
(catch clojure.lang.ExceptionInfo e
  (println (ex-data e))  ; Inspect the structure
  ...)

Verification:

lein test auto-ap.ssr.admin.vendors-test/vendor-merge-same-vendor-rejected

Acceptance Criteria

  • Test uses try/catch pattern
  • Assertions check ex-data structure
  • Error type and fields verified
  • Tests pass
  • Code formatted with lein cljfmt

Work Log

2026-02-07 - Initial Creation

By: Kieran Rails Reviewer Agent

Actions:

  • Compared error testing patterns
  • Identified inconsistency with accounts_test.clj
  • Documented benefits of structured error testing

Learnings:

  • accounts_test.clj uses try/catch with ex-data
  • thrown-with-msg? is less robust
  • Error structure testing provides better validation