--- status: pending priority: p1 issue_id: "007" tags: [testing, solr, mocking, vendors, side-effects] dependencies: [] --- # Add Missing Solr Mocking to vendors_test.clj ## Problem Statement All tests in vendors_test.clj are missing the `(with-redefs [auto-ap.solr/impl ...])` wrapper that accounts_test.clj uses consistently. This may cause: 1. Test failures if vendor operations trigger Solr indexing 2. Side effects between tests if Solr state leaks 3. Inconsistency with established testing patterns ## Findings **From kieran-rails-reviewer:** Lines 31-38, 40-53, 55-69, etc. are missing: ```clojure (with-redefs [auto-ap.solr/impl (auto-ap.solr/->InMemSolrClient (atom {}))] ...) ``` **accounts_test.clj pattern (every test):** ```clojure (deftest account-creation-success (testing "Admin should be able to create..." (with-redefs [auto-ap.solr/impl (auto-ap.solr/->InMemSolrClient (atom {}))] (let [admin-identity (admin-token) result (sut/account-save ...)] ...)))) ``` **Impact:** - Tests may fail if Solr operations are triggered during vendor save/update - Potential side effects between tests (shared Solr state) - Violates test isolation principles ## Proposed Solutions ### Option A: Add Solr Wrapper to All Tests (Recommended) **Effort:** Small (10 minutes) **Risk:** Low Wrap all test bodies with Solr mocking: ```clojure (deftest vendor-fetch-ids-returns-correct-structure (testing "fetch-ids should return properly structured results" (with-redefs [auto-ap.solr/impl (auto-ap.solr/->InMemSolrClient (atom {}))] ;; Given: Setup test data (setup-test-data [(create-vendor "Test Vendor 1") ...]) ...))) ``` **Pros:** - Ensures test isolation - Matches accounts_test.clj pattern exactly - Prevents side effects between tests **Cons:** - Slightly more verbose test code - Adds ~8 lines of boilerplate across all tests ### Option B: Verify Solr Not Needed **Effort:** Small (10 minutes) **Risk:** Medium Research whether vendor operations actually use Solr: 1. Check if `sut/fetch-page`, `sut/fetch-ids`, `sut/hydrate-results`, or `sut/merge-submit` trigger Solr 2. Check vendors.clj for `solr/index-documents-raw` calls 3. Only add mocking if Solr is actually used **Pros:** - Avoids unnecessary code if Solr not used - More targeted fix **Cons:** - Requires investigation - Risk of missing edge cases - Future changes could break tests ### Option C: Extract Solr Fixture **Effort:** Medium (20 minutes) **Risk:** Low Create a test fixture for Solr mocking: ```clojure (defn with-solr-mock [f] (with-redefs [auto-ap.solr/impl (auto-ap.solr/->InMemSolrClient (atom {}))] (f))) (use-fixtures :each wrap-setup with-solr-mock) ``` **Pros:** - Cleaner test code (no inline with-redefs) - Reusable across test files - Automatically applied to all tests **Cons:** - Adds global fixture (may affect tests that don't need it) - New pattern not used in accounts_test.clj ## Recommended Action **Go with Option A** - add explicit Solr mocking to each test. This: 1. Matches the established accounts_test.clj pattern 2. Makes dependencies explicit (clear which tests need Solr) 3. Provides immediate protection without global changes ## Technical Details **Affected Tests:** 1. `vendor-grid-loads-with-empty-database` (lines 31-38) 2. `vendor-fetch-ids-returns-correct-structure` (lines 40-53) 3. `vendor-fetch-page-returns-vendors` (lines 55-69) 4. `vendor-hydrate-results-works` (lines 71-86) 5. `vendor-merge-transfers-references` (lines 92-117) 6. `vendor-merge-same-vendor-rejected` (lines 119-134) 7. `vendor-merge-invalid-vendor-handled` (lines 136-152) 8. `vendor-hydration-includes-all-fields` (lines 158-177) **Verification:** ```bash lein test auto-ap.ssr.admin.vendors-test # Ensure all tests pass ``` ## Acceptance Criteria - [ ] All 8 tests wrapped with Solr mocking - [ ] Pattern matches accounts_test.clj exactly - [ ] Tests continue to pass - [ ] Code formatted with lein cljfmt ## Work Log ### 2026-02-07 - Initial Creation **By:** Code Review Agent **Actions:** - Identified missing Solr mocking during test pattern review - Compared with accounts_test.clj implementation - Created todo for tracking **Learnings:** - accounts_test.clj wraps every test with Solr mocking - Pattern recognition specialist flagged this as inconsistency - Important for test isolation and preventing side effects