refactor: remove tests against basic framework functionalities

docs/test-optimization-user-liveview-analysis.md

@@ -0,0 +1,524 @@
+# User LiveView Tests - Optimization Analysis
+
+**Date:** 2026-01-28  
+**Status:** 📋 Analysis Complete - Ready for Implementation
+
+---
+
+## Executive Summary
+
+Analysis of User LiveView tests identified significant optimization opportunities:
+- **Framework functionality over-testing:** ~15-20 tests test Phoenix/Ash core features
+- **Redundant test data creation:** Each test creates users/members independently
+- **Missing shared fixtures:** No `setup_all` usage for common data
+- **Estimated time savings:** 15-20 seconds (from current ~35.5s for 10 tests in Top 20)
+
+---
+
+## Current Performance Metrics
+
+### Top 20 Slowest Tests (User LiveView)
+
+| Rank | Test | File | Time | Category |
+|------|------|------|------|----------|
+| 3 | `test sorting functionality initially sorts by email ascending` | `index_test.exs` | 4.5s | Index |
+| 5 | `test edit user form - updates admin sets new password for user` | `form_test.exs` | 4.2s | Form |
+| 6 | `test edit user form - validation shows error for duplicate email` | `form_test.exs` | 4.0s | Form |
+| 7 | `test mount and display displays linked member when present` | `show_test.exs` | 3.9s | Show |
+| 8 | `test member linking - workflow selecting member and saving links member to user` | `form_test.exs` | 3.6s | Form |
+| 10 | `test edit user form - validation shows error for invalid password` | `form_test.exs` | 3.5s | Form |
+| 13 | `test mount and display displays password authentication status when enabled` | `show_test.exs` | 3.3s | Show |
+| 14 | `test mount and display mounts successfully with valid user ID` | `show_test.exs` | 3.1s | Show |
+| 15 | `test new user form - creation creates user with password when enabled` | `form_test.exs` | 3.0s | Form |
+| 16 | `test edge cases handles very long email addresses` | `index_test.exs` | 2.9s | Index |
+| 17 | `test checkbox selection functionality can select individual users` | `index_test.exs` | 2.8s | Index |
+| 19 | `test checkbox selection functionality select all automatically checks when all individual users are selected` | `index_test.exs` | 2.7s | Index |
+
+**Total:** ~35.5 seconds for User LiveView tests in Top 20
+
+---
+
+## Analysis by Test File
+
+### 1. `test/mv_web/user_live/index_test.exs`
+
+**Total Tests:** 25 tests  
+**Performance Impact:** ~10.2s for 3 tests in Top 20
+
+#### Issues Identified
+
+**1. Framework Functionality Over-Testing**
+
+**Navigation Tests (Lines 329-355):**
+```elixir
+test "clicking on user row navigates to user show page"
+test "edit link points to correct edit page"
+test "new user button points to correct new page"
+```
+**Problem:** Tests Phoenix LiveView navigation (framework functionality)  
+**Recommendation:** Remove or consolidate into single integration test
+
+**Translation Tests (Lines 357-377):**
+```elixir
+test "shows German translations for selection"
+test "shows English translations for selection"
+```
+**Problem:** Tests Gettext (framework functionality)  
+**Recommendation:** Remove - translations are tested by framework
+
+**Basic Display Tests (Lines 6-47):**
+```elixir
+test "shows translated title in German"
+test "shows translated title in English"
+test "shows New User button"
+test "displays users in a table"
+test "shows correct action links"
+```
+**Problem:** Tests basic HTML rendering (framework functionality)  
+**Recommendation:** Consolidate into single smoke test
+
+**2. Redundant Test Data Creation**
+
+**Sorting Tests (Lines 49-133):**
+- Each test creates 3 users in `setup` block
+- Users are identical across tests
+- Could be shared in `setup_all`
+
+**Checkbox Tests (Lines 135-299):**
+- Each test creates 2 users in `setup` block
+- Users are identical across tests
+- Could be shared in `setup_all`
+
+**3. Framework Sorting Logic Testing**
+
+**Sorting Tests:**
+```elixir
+test "initially sorts by email ascending"
+test "can sort email descending by clicking sort button"
+test "toggles back to ascending when clicking sort button twice"
+test "shows sort direction icons"
+```
+**Problem:** Tests `Enum.sort_by` (Elixir standard library) and basic UI state  
+**Recommendation:** Keep one integration test, remove others
+
+**4. Checkbox State Testing**
+
+**Checkbox Tests (Lines 135-299):**
+- 7 tests testing checkbox state management
+- Most test Phoenix LiveView event handling (framework functionality)
+- Only business logic: "select all automatically checks when all individual users are selected"
+
+**Recommendation:** Keep business logic test, remove framework tests
+
+#### Optimization Opportunities
+
+1. **Consolidate Basic Display Tests:** 5 tests → 1 smoke test
+2. **Remove Translation Tests:** 2 tests → 0 (framework functionality)
+3. **Consolidate Navigation Tests:** 3 tests → 1 integration test
+4. **Share Test Data:** Move user creation to `setup_all` for sorting/checkbox tests
+5. **Reduce Sorting Tests:** 4 tests → 1 integration test
+6. **Reduce Checkbox Tests:** 7 tests → 2 tests (business logic only)
+
+**Estimated Savings:** 5-7 seconds
+
+---
+
+### 2. `test/mv_web/user_live/form_test.exs`
+
+**Total Tests:** 20 tests  
+**Performance Impact:** ~15.0s for 4 tests in Top 20  
+**Note:** Uses `async: false` to prevent PostgreSQL deadlocks
+
+#### Issues Identified
+
+**1. Framework Validation Testing**
+
+**Validation Tests (Lines 122-153, 221-258):**
+```elixir
+test "shows error for duplicate email"
+test "shows error for short password"
+test "shows error for invalid password"
+```
+**Problem:** Tests Ash validations (framework functionality)  
+**Recommendation:** Remove - validations are tested by Ash framework
+
+**2. Redundant Test Data Creation**
+
+**Form Display Tests:**
+- Each test creates users/members independently
+- `setup_live_view` creates new admin user each time
+- Members created in each test that needs them
+
+**Recommendation:** Use `setup_all` for common fixtures
+
+**3. Framework Functionality Testing**
+
+**Internationalization Tests (Lines 261-292):**
+```elixir
+test "shows German labels"
+test "shows English labels"
+test "shows different labels for edit vs new"
+```
+**Problem:** Tests Gettext (framework functionality)  
+**Recommendation:** Remove - translations are tested by framework
+
+**4. Password Storage Testing**
+
+**Password Storage Tests (Lines 92-119):**
+```elixir
+test "stores password when provided"
+```
+**Problem:** Tests password hashing (AshAuthentication framework functionality)  
+**Recommendation:** Remove - password hashing is tested by AshAuthentication
+
+**5. Member Linking Tests**
+
+**Member Linking Tests (Lines 294-422):**
+- Tests member linking workflow (business logic) ✅ Keep
+- But creates members in each test
+- Could share fixtures
+
+#### Optimization Opportunities
+
+1. **Remove Validation Tests:** 3 tests → 0 (framework functionality)
+2. **Remove Translation Tests:** 3 tests → 0 (framework functionality)
+3. **Remove Password Storage Test:** 1 test → 0 (framework functionality)
+4. **Share Test Data:** Use `setup_all` for common users/members
+5. **Consolidate Display Tests:** Multiple display tests → 1-2 smoke tests
+
+**Estimated Savings:** 6-8 seconds
+
+---
+
+### 3. `test/mv_web/live/user_live/show_test.exs`
+
+**Total Tests:** 11 tests  
+**Performance Impact:** ~10.3s for 3 tests in Top 20
+
+#### Issues Identified
+
+**1. Framework Functionality Testing**
+
+**Navigation Tests (Lines 105-133):**
+```elixir
+test "back button navigates to user list"
+test "edit button navigates to edit form"
+```
+**Problem:** Tests Phoenix LiveView navigation (framework functionality)  
+**Recommendation:** Remove or consolidate into single integration test
+
+**2. Basic Display Tests**
+
+**Display Tests (Lines 25-103):**
+- Multiple tests for basic HTML rendering
+- Some test framework functionality (Gettext)
+
+**Recommendation:** Consolidate into fewer tests
+
+**3. Redundant Test Data Creation**
+
+- Each test creates users/members independently
+- `setup` creates one user, but many tests create additional users/members
+
+**Recommendation:** Use `setup_all` for common fixtures
+
+#### Optimization Opportunities
+
+1. **Consolidate Display Tests:** 5 tests → 2-3 tests
+2. **Remove Navigation Tests:** 2 tests → 0 (framework functionality)
+3. **Share Test Data:** Use `setup_all` for common users/members
+
+**Estimated Savings:** 3-4 seconds
+
+---
+
+### 4. Additional Test Files
+
+#### `test/mv_web/user_live/form_member_selection_test.exs`
+- **Tests:** 6 tests
+- **Status:** ✅ Good - Tests business logic (member selection workflow)
+- **Optimization:** Could share member fixtures in `setup_all`
+
+#### `test/mv_web/user_live/form_member_search_test.exs`
+- **Tests:** 4 tests
+- **Status:** ✅ Good - Tests business logic (fuzzy search)
+- **Optimization:** Could share member fixtures in `setup_all`
+
+---
+
+## Framework Functionality Over-Testing
+
+### Tests That Should Be Removed
+
+According to `CODE_GUIDELINES.md` Section 4.7, we should not test framework functionality:
+
+**Phoenix LiveView Framework:**
+- Navigation (redirects, links)
+- Basic HTML rendering
+- Event handling (unless custom business logic)
+
+**Ash Framework:**
+- Standard validations (duplicate email, password length)
+- Standard CRUD operations
+- Relationship loading (unless custom logic)
+
+**Gettext Framework:**
+- Translation functionality
+
+**Elixir Standard Library:**
+- `Enum.sort_by` sorting
+- Basic list operations
+
+### Complete List of Tests to Remove/Consolidate
+
+| Test | File | Reason | Action |
+|------|------|--------|--------|
+| `shows translated title in German` | `index_test.exs` | Gettext framework | Remove |
+| `shows translated title in English` | `index_test.exs` | Gettext framework | Remove |
+| `shows New User button` | `index_test.exs` | Basic HTML rendering | Consolidate |
+| `displays users in a table` | `index_test.exs` | Basic HTML rendering | Consolidate |
+| `shows correct action links` | `index_test.exs` | Basic HTML rendering | Consolidate |
+| `shows sort direction icons` | `index_test.exs` | UI state (framework) | Remove |
+| `toggles back to ascending when clicking sort button twice` | `index_test.exs` | Framework sorting | Remove |
+| `shows select all checkbox` | `index_test.exs` | Basic HTML rendering | Remove |
+| `shows individual user checkboxes` | `index_test.exs` | Basic HTML rendering | Remove |
+| `can select individual users` | `index_test.exs` | Framework event handling | Remove |
+| `can deselect individual users` | `index_test.exs` | Framework event handling | Remove |
+| `select all functionality selects all users` | `index_test.exs` | Framework event handling | Remove |
+| `deselect all functionality deselects all users` | `index_test.exs` | Framework event handling | Remove |
+| `clicking on user row navigates to user show page` | `index_test.exs` | Framework navigation | Remove |
+| `edit link points to correct edit page` | `index_test.exs` | Framework navigation | Remove |
+| `new user button points to correct new page` | `index_test.exs` | Framework navigation | Remove |
+| `shows German translations for selection` | `index_test.exs` | Gettext framework | Remove |
+| `shows English translations for selection` | `index_test.exs` | Gettext framework | Remove |
+| `shows correct form elements` | `form_test.exs` | Basic HTML rendering | Consolidate |
+| `hides password fields initially` | `form_test.exs` | UI state (framework) | Remove |
+| `shows password fields when checkbox toggled` | `form_test.exs` | UI state (framework) | Remove |
+| `shows error for duplicate email` | `form_test.exs` | Ash validation | Remove |
+| `shows error for short password` | `form_test.exs` | Ash validation | Remove |
+| `shows error for invalid password` | `form_test.exs` | Ash validation | Remove |
+| `stores password when provided` | `form_test.exs` | AshAuthentication framework | Remove |
+| `shows German labels` | `form_test.exs` | Gettext framework | Remove |
+| `shows English labels` | `form_test.exs` | Gettext framework | Remove |
+| `shows different labels for edit vs new` | `form_test.exs` | Gettext framework | Remove |
+| `shows correct form elements for existing user` | `form_test.exs` | Basic HTML rendering | Consolidate |
+| `shows admin password fields when enabled` | `form_test.exs` | UI state (framework) | Remove |
+| `back button navigates to user list` | `show_test.exs` | Framework navigation | Remove |
+| `edit button navigates to edit form` | `show_test.exs` | Framework navigation | Remove |
+| `displays user email` | `show_test.exs` | Basic HTML rendering | Consolidate |
+| `sets correct page title` | `show_test.exs` | Basic HTML rendering | Remove |
+
+**Total Tests to Remove:** ~30 tests  
+**Estimated Time Saved:** 8-12 seconds
+
+---
+
+## Optimization Recommendations
+
+### Priority 1: Remove Framework Functionality Tests
+
+**Action:** Remove tests that verify framework functionality (Phoenix, Ash, Gettext)
+
+**Impact:**
+- **Tests Removed:** ~30 tests
+- **Time Saved:** 8-12 seconds
+- **Risk:** ⚠️ Very Low - Framework functionality is tested by framework maintainers
+
+**Implementation:**
+1. Remove tests listed in "Complete List of Tests to Remove/Consolidate"
+2. Keep only business logic tests
+3. Consolidate basic display tests into 1-2 smoke tests
+
+### Priority 2: Implement Shared Fixtures
+
+**Action:** Use `setup_all` for common test data
+
+**Current Problem:**
+- Each test creates users/members independently
+- `conn_with_oidc_user` creates new admin user each time
+- Members created repeatedly for similar tests
+
+**Solution:**
+```elixir
+# In index_test.exs
+setup_all do
+  # Create shared users for sorting tests
+  user_a = create_test_user(%{email: "alpha@example.com", oidc_id: "alpha"})
+  user_z = create_test_user(%{email: "zulu@example.com", oidc_id: "zulu"})
+  user_m = create_test_user(%{email: "mike@example.com", oidc_id: "mike"})
+  
+  # Create shared users for checkbox tests
+  user1 = create_test_user(%{email: "user1@example.com", oidc_id: "user1"})
+  user2 = create_test_user(%{email: "user2@example.com", oidc_id: "user2"})
+  
+  %{
+    sorting_users: [user_a, user_z, user_m],
+    checkbox_users: [user1, user2]
+  }
+end
+```
+
+**Impact:**
+- **Time Saved:** 3-5 seconds
+- **Risk:** ⚠️ Low - Need to ensure test isolation is maintained
+
+**Note:** `async: false` in `form_test.exs` prevents `setup_all` usage due to sandbox limitations. Consider:
+- Using `:shared` sandbox mode (adds complexity)
+- Or keeping current approach for form tests
+
+### Priority 3: Consolidate Redundant Tests
+
+**Action:** Merge similar tests into single integration tests
+
+**Examples:**
+- Basic display tests → 1 smoke test
+- Navigation tests → 1 integration test
+- Sorting tests → 1 integration test (keep business logic)
+
+**Impact:**
+- **Tests Consolidated:** ~10 tests → 3-4 tests
+- **Time Saved:** 2-3 seconds
+- **Risk:** ⚠️ Very Low - Same coverage, fewer tests
+
+### Priority 4: Optimize Test Data Volume
+
+**Action:** Reduce number of test records where possible
+
+**Current:**
+- Sorting tests create 3 users
+- Checkbox tests create 2 users
+- Some tests create multiple members
+
+**Optimization:**
+- Use minimum required data (2 users for sorting, 2 for checkboxes)
+- Share data across tests via `setup_all`
+
+**Impact:**
+- **Time Saved:** 1-2 seconds
+- **Risk:** ⚠️ Very Low - Still tests same functionality
+
+---
+
+## Estimated Total Impact
+
+| Optimization | Tests Affected | Time Saved | Risk |
+|--------------|----------------|------------|------|
+| Remove framework tests | ~30 tests | 8-12s | ⚠️ Very Low |
+| Shared fixtures | All test files | 3-5s | ⚠️ Low |
+| Consolidate tests | ~10 tests | 2-3s | ⚠️ Very Low |
+| Optimize data volume | Multiple tests | 1-2s | ⚠️ Very Low |
+| **Total** | | **14-22s** | |
+
+**Projected Final Time:** From ~35.5s to **~13-21s** (40-60% reduction)
+
+---
+
+## Risk Assessment
+
+### Overall Risk: ⚠️ Low
+
+**Mitigations:**
+1. Framework functionality is tested by framework maintainers
+2. Business logic tests remain intact
+3. Shared fixtures maintain test isolation
+4. Consolidation preserves coverage
+
+**What to Monitor:**
+1. No increase in bugs related to removed functionality
+2. Test coverage remains adequate
+3. CI execution times improve as expected
+
+---
+
+## Implementation Plan
+
+### Phase 1: Remove Framework Tests (Low Risk, High Impact)
+
+1. Remove translation tests (Gettext)
+2. Remove navigation tests (Phoenix LiveView)
+3. Remove validation tests (Ash)
+4. Remove basic HTML rendering tests (consolidate into smoke test)
+
+**Estimated Time:** 1-2 hours  
+**Risk:** ⚠️ Very Low
+
+### Phase 2: Implement Shared Fixtures (Medium Risk, Medium Impact)
+
+1. Add `setup_all` to `index_test.exs` for sorting/checkbox users
+2. Add `setup_all` to `show_test.exs` for common users/members
+3. Update tests to use shared fixtures
+4. Verify test isolation maintained
+
+**Estimated Time:** 2-3 hours  
+**Risk:** ⚠️ Low (need to verify sandbox isolation)
+
+### Phase 3: Consolidate Tests (Low Risk, Low Impact)
+
+1. Merge basic display tests into smoke test
+2. Merge navigation tests into integration test
+3. Reduce sorting tests to 1 integration test
+
+**Estimated Time:** 1-2 hours  
+**Risk:** ⚠️ Very Low
+
+### Phase 4: Verify and Benchmark
+
+1. Run full test suite
+2. Benchmark execution times
+3. Verify no regressions
+4. Update documentation
+
+**Estimated Time:** 1 hour  
+**Risk:** ⚠️ Very Low
+
+---
+
+## Tests to Keep (Business Logic)
+
+### Index Tests
+- ✅ `initially sorts by email ascending` - Tests default sort (business logic)
+- ✅ `can sort email descending by clicking sort button` - Tests sort functionality
+- ✅ `select all automatically checks when all individual users are selected` - Business logic
+- ✅ `does not show system actor user in list` - Business rule
+- ✅ `displays linked member name in user list` - Business logic
+- ✅ `handles empty user list gracefully` - Edge case
+- ✅ `handles users with missing OIDC ID` - Edge case
+
+### Form Tests
+- ✅ `creates user without password` - Business logic
+- ✅ `creates user with password when enabled` - Business logic
+- ✅ `updates email without changing password` - Business logic
+- ✅ `admin sets new password for user` - Business logic
+- ✅ `selecting member and saving links member to user` - Business logic
+- ✅ `unlinking member and saving removes member from user` - Business logic
+- ✅ `redirects to user list when editing system actor user` - Business rule
+
+### Show Tests
+- ✅ `displays password authentication status when enabled` - Business logic
+- ✅ `displays password authentication status when not enabled` - Business logic
+- ✅ `displays linked member when present` - Business logic
+- ✅ `displays 'No member linked' when no member is linked` - Business logic
+- ✅ `redirects to user list when viewing system actor user` - Business rule
+
+### Member Selection/Search Tests
+- ✅ All tests in `form_member_selection_test.exs` - Business logic
+- ✅ All tests in `form_member_search_test.exs` - Business logic (fuzzy search)
+
+---
+
+## References
+
+- **Coding Guidelines:** `CODE_GUIDELINES.md` - Section 4.7 (Testing Best Practices)
+- **Test Performance Optimization:** `docs/test-performance-optimization.md`
+- **User LiveView Implementation:** `lib/mv_web/live/user_live/`
+
+---
+
+## Next Steps
+
+1. ✅ Analysis complete
+2. ⏳ Review with team
+3. ⏳ Implement Phase 1 (Remove framework tests)
+4. ⏳ Implement Phase 2 (Shared fixtures)
+5. ⏳ Implement Phase 3 (Consolidate tests)
+6. ⏳ Benchmark and verify