docs: consolidate test performance docs
This commit is contained in:
parent
9b314a9806
commit
709cf010c6
3 changed files with 183 additions and 1411 deletions
|
|
@ -1,868 +0,0 @@
|
||||||
# Policy Tests - Deep Optimization Analysis
|
|
||||||
|
|
||||||
**Date:** 2026-01-28
|
|
||||||
**Status:** 📋 Analysis Complete - Ready for Decision
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Executive Summary
|
|
||||||
|
|
||||||
Analysis of policy tests identified significant optimization opportunities:
|
|
||||||
- **Redundant fixture creation:** Roles and users created repeatedly across tests
|
|
||||||
- **Framework functionality over-testing:** Many tests verify Ash policy framework behavior
|
|
||||||
- **Test duplication:** Similar tests across different permission sets
|
|
||||||
- **Estimated time savings:** 5-8 seconds (from current ~66s for top 20 tests)
|
|
||||||
|
|
||||||
**Key Finding:** All policy tests use `async: false`, which prevents `setup_all` usage. However, roles can be shared within describe blocks, and some tests verify framework functionality rather than business logic.
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Current Performance Metrics
|
|
||||||
|
|
||||||
### Policy Test Files Performance
|
|
||||||
|
|
||||||
| File | Tests | Total Time | Avg per Test | Top Tests |
|
|
||||||
|------|-------|------------|--------------|-----------|
|
|
||||||
| `member_policies_test.exs` | 24 tests | ~66s (top 20) | ~2.7s | 4.8s (slowest) |
|
|
||||||
| `user_policies_test.exs` | 30 tests | ~66s (top 20) | ~2.2s | 3.0s (slowest) |
|
|
||||||
| `custom_field_value_policies_test.exs` | 20 tests | ~66s (top 20) | ~3.3s | ~3.0s (estimated) |
|
|
||||||
| **Total** | **74 tests** | **~152s** | **~2.1s** | **4.8s** |
|
|
||||||
|
|
||||||
### Top 20 Slowest Policy Tests
|
|
||||||
|
|
||||||
| Rank | Test | File | Time | Permission Set |
|
|
||||||
|------|------|------|------|----------------|
|
|
||||||
| 1 | `cannot destroy member (returns forbidden)` | `member_policies_test.exs` | 4.8s | own_data |
|
|
||||||
| 2 | `cannot update unlinked member (returns forbidden)` | `member_policies_test.exs` | 4.6s | own_data |
|
|
||||||
| 3 | `can read all members` | `member_policies_test.exs` | 4.5s | read_only |
|
|
||||||
| 4 | `cannot update any member (returns forbidden)` | `member_policies_test.exs` | 4.2s | read_only |
|
|
||||||
| 5 | `can update linked member` | `member_policies_test.exs` | 3.7s | own_data |
|
|
||||||
| 6 | `can update any member` | `member_policies_test.exs` | 3.6s | admin |
|
|
||||||
| 7 | `can read individual member` | `member_policies_test.exs` | 3.4s | read_only |
|
|
||||||
| 8 | `can create member` | `member_policies_test.exs` | 3.1s | normal_user |
|
|
||||||
| 9 | `cannot create member (returns forbidden)` | `member_policies_test.exs` | 3.1s | own_data |
|
|
||||||
| 10 | `cannot create user (returns forbidden)` | `user_policies_test.exs` | 3.0s | read_only |
|
|
||||||
|
|
||||||
**Total Top 20:** ~66 seconds
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Current Situation Analysis
|
|
||||||
|
|
||||||
### Test File Configuration
|
|
||||||
|
|
||||||
| File | `async` Setting | Reason | Can Use `setup_all`? |
|
|
||||||
|------|----------------|--------|---------------------|
|
|
||||||
| `member_policies_test.exs` | `false` | Database commits visible across queries | ❌ No (sandbox limitation) |
|
|
||||||
| `user_policies_test.exs` | `false` | Database commits visible across queries | ❌ No (sandbox limitation) |
|
|
||||||
| `custom_field_value_policies_test.exs` | `false` | Database commits visible across queries | ❌ No (sandbox limitation) |
|
|
||||||
|
|
||||||
**Note:** All policy tests use `async: false` to ensure database commits are visible across queries within the same test. This is necessary for testing unlinked members and other scenarios where data needs to persist across multiple queries.
|
|
||||||
|
|
||||||
### Current Fixture Creation Patterns
|
|
||||||
|
|
||||||
#### Pattern 1: Role Creation (Most Redundant)
|
|
||||||
|
|
||||||
**Current Implementation:**
|
|
||||||
```elixir
|
|
||||||
# Helper function called in EVERY test
|
|
||||||
defp create_role_with_permission_set(permission_set_name, actor) do
|
|
||||||
role_name = "Test Role #{permission_set_name} #{System.unique_integer([:positive])}"
|
|
||||||
|
|
||||||
Authorization.create_role(%{
|
|
||||||
name: role_name,
|
|
||||||
description: "Test role for #{permission_set_name}",
|
|
||||||
permission_set_name: permission_set_name
|
|
||||||
}, actor: actor)
|
|
||||||
end
|
|
||||||
```
|
|
||||||
|
|
||||||
**Usage:**
|
|
||||||
- Called in `create_user_with_permission_set` helper
|
|
||||||
- Called for every user created in every test
|
|
||||||
- **Total calls:** ~74+ (one per user, multiple users per test)
|
|
||||||
|
|
||||||
**Problem:**
|
|
||||||
- Roles are **identical** across tests with the same permission set
|
|
||||||
- `own_data` role created ~20+ times (once per test)
|
|
||||||
- `read_only` role created ~20+ times
|
|
||||||
- `normal_user` role created ~20+ times
|
|
||||||
- `admin` role created ~20+ times
|
|
||||||
|
|
||||||
**Optimization Opportunity:**
|
|
||||||
- Create roles once per permission set in describe-level `setup`
|
|
||||||
- Reuse roles across tests in the same describe block
|
|
||||||
|
|
||||||
#### Pattern 2: User Creation (Redundant)
|
|
||||||
|
|
||||||
**Current Implementation:**
|
|
||||||
```elixir
|
|
||||||
# Helper function creates user + role + assigns role
|
|
||||||
defp create_user_with_permission_set(permission_set_name, actor) do
|
|
||||||
role = create_role_with_permission_set(permission_set_name, actor) # Creates role
|
|
||||||
user = Accounts.User |> Ash.Changeset.for_create(:register_with_password, ...) |> Ash.create(...)
|
|
||||||
# Assign role
|
|
||||||
# Reload with role
|
|
||||||
end
|
|
||||||
```
|
|
||||||
|
|
||||||
**Usage:**
|
|
||||||
- Called in every test's `setup` block
|
|
||||||
- Creates: 1 role + 1 user + role assignment + reload
|
|
||||||
- **Cost:** ~4-5 database operations per user
|
|
||||||
|
|
||||||
**Problem:**
|
|
||||||
- Users are created fresh for each test
|
|
||||||
- But: Users need to be unique (different emails)
|
|
||||||
- **Cannot share users** across tests (they're actors, need isolation)
|
|
||||||
|
|
||||||
**Optimization Opportunity:**
|
|
||||||
- **Limited** - Users must be unique per test
|
|
||||||
- But: Can optimize role creation (see Pattern 1)
|
|
||||||
|
|
||||||
#### Pattern 3: Member Creation (Redundant)
|
|
||||||
|
|
||||||
**Current Implementation:**
|
|
||||||
```elixir
|
|
||||||
# Helper creates admin user, then creates member
|
|
||||||
defp create_linked_member_for_user(user, actor) do
|
|
||||||
admin = create_admin_user(actor) # Creates admin user + role
|
|
||||||
member = Membership.Member |> Ash.Changeset.for_create(:create_member, ...) |> Ash.create(...)
|
|
||||||
# Link member to user
|
|
||||||
end
|
|
||||||
|
|
||||||
defp create_unlinked_member(actor) do
|
|
||||||
admin = create_admin_user(actor) # Creates admin user + role AGAIN
|
|
||||||
member = Membership.Member |> Ash.Changeset.for_create(:create_member, ...) |> Ash.create(...)
|
|
||||||
end
|
|
||||||
```
|
|
||||||
|
|
||||||
**Usage:**
|
|
||||||
- Called in every describe block's `setup`
|
|
||||||
- Creates: 1 admin user + 1 admin role + 1-2 members
|
|
||||||
- **Cost:** ~5-6 database operations per describe block
|
|
||||||
|
|
||||||
**Problem:**
|
|
||||||
- Admin user created repeatedly (once per describe block)
|
|
||||||
- Admin role created repeatedly (once per admin user)
|
|
||||||
- Members are test-specific (need isolation)
|
|
||||||
|
|
||||||
**Optimization Opportunity:**
|
|
||||||
- Create admin user once per test file (module-level)
|
|
||||||
- Reuse admin user for member creation
|
|
||||||
- **But:** `async: false` + sandbox prevents `setup_all`
|
|
||||||
|
|
||||||
#### Pattern 4: Test Structure (Highly Repetitive)
|
|
||||||
|
|
||||||
**Current Structure:**
|
|
||||||
```elixir
|
|
||||||
describe "own_data permission set" do
|
|
||||||
setup %{actor: actor} do
|
|
||||||
user = create_user_with_permission_set("own_data", actor) # Creates role + user
|
|
||||||
linked_member = create_linked_member_for_user(user, actor) # Creates admin + member
|
|
||||||
unlinked_member = create_unlinked_member(actor) # Creates admin + member
|
|
||||||
%{user: user, linked_member: linked_member, unlinked_member: unlinked_member}
|
|
||||||
end
|
|
||||||
|
|
||||||
test "can read linked member" do
|
|
||||||
# Test business logic
|
|
||||||
end
|
|
||||||
|
|
||||||
test "cannot read unlinked member" do
|
|
||||||
# Test business logic
|
|
||||||
end
|
|
||||||
|
|
||||||
# ... 5-7 more tests
|
|
||||||
end
|
|
||||||
|
|
||||||
describe "read_only permission set" do
|
|
||||||
setup %{actor: actor} do
|
|
||||||
# SAME PATTERN - creates role + user + members
|
|
||||||
end
|
|
||||||
|
|
||||||
test "can read all members" do
|
|
||||||
# Similar to own_data tests
|
|
||||||
end
|
|
||||||
|
|
||||||
# ... similar tests
|
|
||||||
end
|
|
||||||
```
|
|
||||||
|
|
||||||
**Problem:**
|
|
||||||
- Same setup pattern repeated 4 times (one per permission set)
|
|
||||||
- Same test patterns repeated across permission sets
|
|
||||||
- Tests verify **framework behavior** (Ash policies) not business logic
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Framework Functionality Analysis
|
|
||||||
|
|
||||||
### What Are We Testing?
|
|
||||||
|
|
||||||
According to `CODE_GUIDELINES.md` Section 4.7, we should **not** test framework functionality. Let's analyze what policy tests actually verify:
|
|
||||||
|
|
||||||
#### Framework Functionality (Should NOT Test)
|
|
||||||
|
|
||||||
**Ash Policy Framework:**
|
|
||||||
- ✅ **Policy evaluation** - How Ash evaluates policies (framework)
|
|
||||||
- ✅ **Permission lookup** - How Ash looks up permissions (framework)
|
|
||||||
- ✅ **Scope filtering** - How Ash applies scope filters (framework)
|
|
||||||
- ✅ **Auto-filter behavior** - How Ash auto-filters queries (framework)
|
|
||||||
- ✅ **Forbidden vs NotFound** - How Ash returns errors (framework)
|
|
||||||
|
|
||||||
**Examples:**
|
|
||||||
```elixir
|
|
||||||
# Tests Ash framework behavior:
|
|
||||||
test "cannot read other users (returns not found due to auto_filter)" do
|
|
||||||
# This tests how Ash's auto_filter works (framework)
|
|
||||||
assert_raise Ash.Error.Invalid, fn ->
|
|
||||||
Ash.get!(Accounts.User, other_user.id, actor: user, domain: Mv.Accounts)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
test "cannot update other users (returns forbidden)" do
|
|
||||||
# This tests how Ash returns Forbidden errors (framework)
|
|
||||||
assert_raise Ash.Error.Forbidden, fn ->
|
|
||||||
other_user |> Ash.Changeset.for_update(...) |> Ash.update!(actor: user)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
```
|
|
||||||
|
|
||||||
#### Business Logic (Should Test)
|
|
||||||
|
|
||||||
**Our Authorization Rules:**
|
|
||||||
- ✅ **Permission set definitions** - What permissions each role has (business logic)
|
|
||||||
- ✅ **Scope definitions** - What scopes each permission set uses (business logic)
|
|
||||||
- ✅ **Special cases** - Custom business rules (e.g., "user can always read linked member")
|
|
||||||
- ✅ **Permission set behavior** - How our permission sets differ (business logic)
|
|
||||||
|
|
||||||
**Examples:**
|
|
||||||
```elixir
|
|
||||||
# Tests our business logic:
|
|
||||||
test "own_data can read linked member" do
|
|
||||||
# Tests our business rule: own_data can read linked members
|
|
||||||
{:ok, member} = Ash.get(Membership.Member, linked_member.id, actor: user)
|
|
||||||
assert member.id == linked_member.id
|
|
||||||
end
|
|
||||||
|
|
||||||
test "read_only can read all members" do
|
|
||||||
# Tests our business rule: read_only can read all members
|
|
||||||
{:ok, members} = Ash.read(Membership.Member, actor: user)
|
|
||||||
assert length(members) >= 2 # Should see all members
|
|
||||||
end
|
|
||||||
```
|
|
||||||
|
|
||||||
### Test Classification
|
|
||||||
|
|
||||||
#### Tests That Verify Framework Functionality
|
|
||||||
|
|
||||||
**Error Handling Tests (Framework):**
|
|
||||||
- `cannot read other users (returns not found due to auto_filter)` - Tests Ash auto-filter
|
|
||||||
- `cannot update other users (returns forbidden)` - Tests Ash error types
|
|
||||||
- `cannot create user (returns forbidden)` - Tests Ash error types
|
|
||||||
- `cannot destroy user (returns forbidden)` - Tests Ash error types
|
|
||||||
|
|
||||||
**Permission Evaluation Tests (Framework):**
|
|
||||||
- Most "cannot" tests verify Ash's policy evaluation (framework)
|
|
||||||
- Tests that check error types (Forbidden vs NotFound) verify framework behavior
|
|
||||||
|
|
||||||
**Recommendation:** Remove or consolidate these tests - they test Ash framework, not our business logic.
|
|
||||||
|
|
||||||
#### Tests That Verify Business Logic
|
|
||||||
|
|
||||||
**Permission Set Behavior Tests (Business Logic):**
|
|
||||||
- `can read linked member` - Tests our business rule
|
|
||||||
- `can read all members` - Tests our permission set definition
|
|
||||||
- `can update linked member` - Tests our scope definition
|
|
||||||
- `list members returns only linked member` - Tests our scope filter
|
|
||||||
|
|
||||||
**Special Case Tests (Business Logic):**
|
|
||||||
- `user can always READ linked member` - Tests our custom business rule
|
|
||||||
- `own_data user can update linked member` - Tests our permission set
|
|
||||||
|
|
||||||
**Recommendation:** Keep these tests - they verify our business logic.
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Detailed Test Analysis
|
|
||||||
|
|
||||||
### `member_policies_test.exs` (24 tests)
|
|
||||||
|
|
||||||
#### Test Structure
|
|
||||||
|
|
||||||
**4 Permission Sets × ~6 Tests Each = 24 Tests**
|
|
||||||
|
|
||||||
**Permission Sets Tested:**
|
|
||||||
1. `own_data` (Mitglied) - 7 tests
|
|
||||||
2. `read_only` (Vorstand/Buchhaltung) - 6 tests
|
|
||||||
3. `normal_user` (Kassenwart) - 5 tests
|
|
||||||
4. `admin` - 4 tests
|
|
||||||
5. Special case - 3 tests
|
|
||||||
|
|
||||||
#### Redundancy Analysis
|
|
||||||
|
|
||||||
**Redundant Tests Across Permission Sets:**
|
|
||||||
|
|
||||||
| Test Pattern | own_data | read_only | normal_user | admin | Redundant? |
|
|
||||||
|--------------|----------|-----------|-------------|-------|------------|
|
|
||||||
| `can read linked/all members` | ✅ | ✅ | ✅ | ✅ | ⚠️ Partially (different scopes) |
|
|
||||||
| `can update linked/any member` | ✅ (linked) | ❌ | ✅ (all) | ✅ (all) | ⚠️ Partially |
|
|
||||||
| `cannot create member` | ✅ | ✅ | ❌ | ❌ | ⚠️ Yes (same behavior) |
|
|
||||||
| `cannot destroy member` | ✅ | ✅ | ✅ | ❌ | ⚠️ Yes (same behavior) |
|
|
||||||
| `cannot update unlinked/any` | ✅ | ✅ | ❌ | ❌ | ⚠️ Yes (same behavior) |
|
|
||||||
|
|
||||||
**Framework Tests (Should Remove):**
|
|
||||||
- `cannot create member (returns forbidden)` - Tests Ash error handling (framework)
|
|
||||||
- `cannot destroy member (returns forbidden)` - Tests Ash error handling (framework)
|
|
||||||
- `cannot update unlinked member (returns forbidden)` - Tests Ash error handling (framework)
|
|
||||||
|
|
||||||
**Business Logic Tests (Should Keep):**
|
|
||||||
- `can read linked member` - Tests our scope definition
|
|
||||||
- `can update linked member` - Tests our permission set
|
|
||||||
- `can read all members` - Tests our permission set
|
|
||||||
- `can create member` - Tests our permission set
|
|
||||||
- `list members returns only linked member` - Tests our scope filter
|
|
||||||
|
|
||||||
#### Fixture Creation Analysis
|
|
||||||
|
|
||||||
**Per Describe Block Setup:**
|
|
||||||
```elixir
|
|
||||||
describe "own_data permission set" do
|
|
||||||
setup %{actor: actor} do
|
|
||||||
user = create_user_with_permission_set("own_data", actor) # Creates: role + user + assignment + reload
|
|
||||||
linked_member = create_linked_member_for_user(user, actor) # Creates: admin user + admin role + member + link
|
|
||||||
unlinked_member = create_unlinked_member(actor) # Creates: admin user + admin role + member
|
|
||||||
# Total: 1 own_data role + 1 own_data user + 1 admin role + 1 admin user + 2 members = ~10 DB operations
|
|
||||||
end
|
|
||||||
end
|
|
||||||
```
|
|
||||||
|
|
||||||
**Total Per Test File:**
|
|
||||||
- 4 describe blocks × ~10 operations = ~40 operations per test run
|
|
||||||
- But: `setup` runs for each test, so ~40 × 6 tests = ~240 operations
|
|
||||||
- **Actually:** `setup` runs once per describe block, so ~40 operations total per file
|
|
||||||
|
|
||||||
**Optimization Opportunity:**
|
|
||||||
- Roles are identical across tests in same describe block
|
|
||||||
- Admin user is identical across describe blocks
|
|
||||||
- **Can create roles once per describe block** (already done via `setup`)
|
|
||||||
- **Can create admin user once per test file** (but `async: false` prevents `setup_all`)
|
|
||||||
|
|
||||||
### `user_policies_test.exs` (30 tests)
|
|
||||||
|
|
||||||
#### Test Structure
|
|
||||||
|
|
||||||
**4 Permission Sets × ~7 Tests Each + 3 Bypass Tests = 30 Tests**
|
|
||||||
|
|
||||||
**Permission Sets Tested:**
|
|
||||||
1. `own_data` (Mitglied) - 7 tests
|
|
||||||
2. `read_only` (Vorstand/Buchhaltung) - 7 tests
|
|
||||||
3. `normal_user` (Kassenwart) - 7 tests
|
|
||||||
4. `admin` - 5 tests
|
|
||||||
5. AshAuthentication bypass - 3 tests
|
|
||||||
|
|
||||||
#### Redundancy Analysis
|
|
||||||
|
|
||||||
**Highly Redundant Tests:**
|
|
||||||
|
|
||||||
| Test Pattern | own_data | read_only | normal_user | admin | Redundant? |
|
|
||||||
|--------------|----------|-----------|-------------|-------|------------|
|
|
||||||
| `can read own user record` | ✅ | ✅ | ✅ | ❌ | ⚠️ Yes (same behavior) |
|
|
||||||
| `can update own email` | ✅ | ✅ | ✅ | ❌ | ⚠️ Yes (same behavior) |
|
|
||||||
| `cannot read other users` | ✅ | ✅ | ✅ | ❌ | ⚠️ Yes (same behavior) |
|
|
||||||
| `cannot update other users` | ✅ | ✅ | ✅ | ❌ | ⚠️ Yes (same behavior) |
|
|
||||||
| `list users returns only own` | ✅ | ✅ | ✅ | ❌ | ⚠️ Yes (same behavior) |
|
|
||||||
| `cannot create user` | ✅ | ✅ | ✅ | ❌ | ⚠️ Yes (same behavior) |
|
|
||||||
| `cannot destroy user` | ✅ | ✅ | ✅ | ❌ | ⚠️ Yes (same behavior) |
|
|
||||||
|
|
||||||
**Framework Tests (Should Remove):**
|
|
||||||
- All "cannot" tests - Test Ash error handling (framework)
|
|
||||||
- `cannot read other users (returns not found)` - Tests Ash auto-filter (framework)
|
|
||||||
- `cannot update other users (returns forbidden)` - Tests Ash error types (framework)
|
|
||||||
|
|
||||||
**Business Logic Tests (Should Keep):**
|
|
||||||
- `can read all users` (admin) - Tests our permission set
|
|
||||||
- `can update other users` (admin) - Tests our permission set
|
|
||||||
- `can create user` (admin) - Tests our permission set
|
|
||||||
- `can destroy user` (admin) - Tests our permission set
|
|
||||||
|
|
||||||
**AshAuthentication Bypass Tests:**
|
|
||||||
- These test **our integration** with AshAuthentication (business logic)
|
|
||||||
- ✅ Keep these tests
|
|
||||||
|
|
||||||
### `custom_field_value_policies_test.exs` (20 tests)
|
|
||||||
|
|
||||||
#### Test Structure
|
|
||||||
|
|
||||||
**4 Permission Sets × ~5 Tests Each = 20 Tests**
|
|
||||||
|
|
||||||
**Similar Patterns to Member Policies:**
|
|
||||||
- Same redundancy issues
|
|
||||||
- Same framework vs. business logic concerns
|
|
||||||
- Same fixture creation patterns
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Optimization Opportunities
|
|
||||||
|
|
||||||
### Opportunity 1: Remove Framework Functionality Tests
|
|
||||||
|
|
||||||
**Tests to Remove:**
|
|
||||||
|
|
||||||
**`member_policies_test.exs`:**
|
|
||||||
- `cannot create member (returns forbidden)` - 3 tests (one per permission set)
|
|
||||||
- `cannot destroy member (returns forbidden)` - 3 tests (one per permission set)
|
|
||||||
- `cannot update unlinked member (returns forbidden)` - 1 test (own_data)
|
|
||||||
|
|
||||||
**`user_policies_test.exs`:**
|
|
||||||
- `cannot read other users (returns not found)` - 3 tests
|
|
||||||
- `cannot update other users (returns forbidden)` - 3 tests
|
|
||||||
- `cannot create user (returns forbidden)` - 3 tests
|
|
||||||
- `cannot destroy user (returns forbidden)` - 3 tests
|
|
||||||
- `list users returns only own user` - 3 tests (tests framework auto-filter)
|
|
||||||
|
|
||||||
**Total Tests to Remove:** ~22 tests
|
|
||||||
|
|
||||||
**Estimated Time Saved:** 3-4 seconds
|
|
||||||
|
|
||||||
**Risk:** ⚠️ **Very Low** - Framework functionality is tested by Ash maintainers
|
|
||||||
|
|
||||||
### Opportunity 2: Consolidate Redundant Tests
|
|
||||||
|
|
||||||
**Tests to Consolidate:**
|
|
||||||
|
|
||||||
**`user_policies_test.exs`:**
|
|
||||||
- `can read own user record` - 3 tests → 1 integration test
|
|
||||||
- `can update own email` - 3 tests → 1 integration test
|
|
||||||
|
|
||||||
**`member_policies_test.exs`:**
|
|
||||||
- Similar patterns can be consolidated
|
|
||||||
|
|
||||||
**Total Tests to Consolidate:** ~6-8 tests → 2-3 tests
|
|
||||||
|
|
||||||
**Estimated Time Saved:** 1-2 seconds
|
|
||||||
|
|
||||||
**Risk:** ⚠️ **Low** - Same coverage, fewer tests
|
|
||||||
|
|
||||||
### Opportunity 3: Share Roles Within Describe Blocks
|
|
||||||
|
|
||||||
**Current:**
|
|
||||||
```elixir
|
|
||||||
describe "own_data permission set" do
|
|
||||||
setup %{actor: actor} do
|
|
||||||
# Each test creates its own role via create_user_with_permission_set
|
|
||||||
user = create_user_with_permission_set("own_data", actor) # Creates role
|
|
||||||
# ...
|
|
||||||
end
|
|
||||||
|
|
||||||
test "test 1", %{user: user} do
|
|
||||||
# Uses user with role
|
|
||||||
end
|
|
||||||
|
|
||||||
test "test 2", %{user: user} do
|
|
||||||
# Uses same user (role already created in setup)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
```
|
|
||||||
|
|
||||||
**Actually, roles ARE already shared** - `setup` runs once per describe block, so role is created once and reused.
|
|
||||||
|
|
||||||
**Optimization:** None needed - already optimal
|
|
||||||
|
|
||||||
### Opportunity 4: Share Admin User Across Describe Blocks
|
|
||||||
|
|
||||||
**Current:**
|
|
||||||
```elixir
|
|
||||||
# Each describe block creates admin user
|
|
||||||
describe "own_data permission set" do
|
|
||||||
setup %{actor: actor} do
|
|
||||||
linked_member = create_linked_member_for_user(user, actor) # Creates admin
|
|
||||||
unlinked_member = create_unlinked_member(actor) # Creates admin AGAIN
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
describe "read_only permission set" do
|
|
||||||
setup %{actor: actor} do
|
|
||||||
linked_member = create_linked_member_for_user(user, actor) # Creates admin AGAIN
|
|
||||||
unlinked_member = create_unlinked_member(actor) # Creates admin AGAIN
|
|
||||||
end
|
|
||||||
end
|
|
||||||
```
|
|
||||||
|
|
||||||
**Problem:**
|
|
||||||
- Admin user created 2× per describe block (linked + unlinked member helpers)
|
|
||||||
- Admin user created 4× per test file (one per permission set)
|
|
||||||
- **Total:** ~8 admin users + 8 admin roles per test file
|
|
||||||
|
|
||||||
**Optimization:**
|
|
||||||
- Create admin user once in module-level `setup`
|
|
||||||
- Reuse admin user in helper functions
|
|
||||||
|
|
||||||
**Implementation:**
|
|
||||||
```elixir
|
|
||||||
# Module-level setup
|
|
||||||
setup do
|
|
||||||
system_actor = Mv.Helpers.SystemActor.get_system_actor()
|
|
||||||
admin_user = create_user_with_permission_set("admin", system_actor)
|
|
||||||
%{actor: system_actor, admin_user: admin_user}
|
|
||||||
end
|
|
||||||
|
|
||||||
# Helper functions use shared admin
|
|
||||||
defp create_linked_member_for_user(user, actor, admin_user) do
|
|
||||||
# Use admin_user instead of creating new one
|
|
||||||
end
|
|
||||||
```
|
|
||||||
|
|
||||||
**Estimated Time Saved:** 1-2 seconds
|
|
||||||
|
|
||||||
**Risk:** ⚠️ **Low** - Admin user is read-only in tests, safe to share
|
|
||||||
|
|
||||||
### Opportunity 5: Reduce Test Data Volume
|
|
||||||
|
|
||||||
**Current:**
|
|
||||||
- Each test creates multiple members/users
|
|
||||||
- Some tests create more data than needed
|
|
||||||
|
|
||||||
**Optimization:**
|
|
||||||
- Use minimum required data
|
|
||||||
- Share fixtures where possible
|
|
||||||
|
|
||||||
**Estimated Time Saved:** 0.5-1 second
|
|
||||||
|
|
||||||
**Risk:** ⚠️ **Very Low** - Still tests same functionality
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Estimated Total Impact
|
|
||||||
|
|
||||||
| Optimization | Tests Affected | Time Saved | Risk |
|
|
||||||
|--------------|----------------|------------|------|
|
|
||||||
| Remove framework tests | ~22 tests | 3-4s | ⚠️ Very Low |
|
|
||||||
| Consolidate redundant tests | ~6-8 tests | 1-2s | ⚠️ Low |
|
|
||||||
| Share admin user | All tests | 1-2s | ⚠️ Low |
|
|
||||||
| Reduce test data volume | Multiple tests | 0.5-1s | ⚠️ Very Low |
|
|
||||||
| **Total** | | **5.5-9s** | |
|
|
||||||
|
|
||||||
**Projected Final Time:** From ~66s (top 20) to **~57-60s** (8-14% reduction)
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Risk Assessment
|
|
||||||
|
|
||||||
### Overall Risk: ⚠️ **Low**
|
|
||||||
|
|
||||||
**Mitigations:**
|
|
||||||
1. Framework functionality is tested by Ash maintainers
|
|
||||||
2. Business logic tests remain intact
|
|
||||||
3. Admin user sharing maintains test isolation (read-only)
|
|
||||||
4. Consolidation preserves coverage
|
|
||||||
|
|
||||||
**What to Monitor:**
|
|
||||||
1. No increase in authorization bugs
|
|
||||||
2. Test coverage remains adequate
|
|
||||||
3. CI execution times improve as expected
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Recommended Scenarios
|
|
||||||
|
|
||||||
### Scenario 1: Remove Framework Tests (High Impact, Low Risk)
|
|
||||||
|
|
||||||
**Action:** Remove tests that verify Ash framework functionality
|
|
||||||
|
|
||||||
**Tests to Remove:**
|
|
||||||
- All "cannot" tests that verify error types (Forbidden, NotFound)
|
|
||||||
- Tests that verify auto-filter behavior (framework)
|
|
||||||
- Tests that verify permission evaluation (framework)
|
|
||||||
|
|
||||||
**Keep:**
|
|
||||||
- Tests that verify permission set behavior (business logic)
|
|
||||||
- Tests that verify scope definitions (business logic)
|
|
||||||
- Tests that verify special cases (business logic)
|
|
||||||
|
|
||||||
**Estimated Savings:** 3-4 seconds
|
|
||||||
|
|
||||||
**Risk:** ⚠️ **Very Low**
|
|
||||||
|
|
||||||
### Scenario 2: Consolidate Redundant Tests (Medium Impact, Low Risk)
|
|
||||||
|
|
||||||
**Action:** Merge similar tests across permission sets
|
|
||||||
|
|
||||||
**Examples:**
|
|
||||||
- `can read own user record` (3 tests) → 1 integration test
|
|
||||||
- `can update own email` (3 tests) → 1 integration test
|
|
||||||
|
|
||||||
**Estimated Savings:** 1-2 seconds
|
|
||||||
|
|
||||||
**Risk:** ⚠️ **Low**
|
|
||||||
|
|
||||||
### Scenario 3: Share Admin User (Low Impact, Low Risk)
|
|
||||||
|
|
||||||
**Action:** Create admin user once per test file, reuse in helpers
|
|
||||||
|
|
||||||
**Implementation:**
|
|
||||||
- Module-level `setup` creates admin user
|
|
||||||
- Helper functions accept admin user as parameter
|
|
||||||
- Reuse admin user instead of creating new one
|
|
||||||
|
|
||||||
**Estimated Savings:** 1-2 seconds
|
|
||||||
|
|
||||||
**Risk:** ⚠️ **Low**
|
|
||||||
|
|
||||||
### Scenario 4: Combined Approach (Recommended)
|
|
||||||
|
|
||||||
**Action:** Implement all three scenarios
|
|
||||||
|
|
||||||
**Estimated Savings:** 5.5-9 seconds
|
|
||||||
|
|
||||||
**Risk:** ⚠️ **Low** (combination of low-risk optimizations)
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Implementation Plan
|
|
||||||
|
|
||||||
### Phase 1: Remove Framework Tests
|
|
||||||
|
|
||||||
1. Identify all "cannot" tests that verify error types
|
|
||||||
2. Remove tests that verify Ash auto-filter behavior
|
|
||||||
3. Remove tests that verify permission evaluation (framework)
|
|
||||||
4. Keep business logic tests
|
|
||||||
|
|
||||||
**Estimated Time:** 1-2 hours
|
|
||||||
**Risk:** ⚠️ Very Low
|
|
||||||
|
|
||||||
### Phase 2: Consolidate Redundant Tests
|
|
||||||
|
|
||||||
1. Identify similar tests across permission sets
|
|
||||||
2. Create integration tests that cover multiple permission sets
|
|
||||||
3. Remove redundant individual tests
|
|
||||||
|
|
||||||
**Estimated Time:** 1-2 hours
|
|
||||||
**Risk:** ⚠️ Low
|
|
||||||
|
|
||||||
### Phase 3: Share Admin User
|
|
||||||
|
|
||||||
1. Add module-level `setup` to create admin user
|
|
||||||
2. Update helper functions to accept admin user parameter
|
|
||||||
3. Update all `setup` blocks to use shared admin user
|
|
||||||
|
|
||||||
**Estimated Time:** 1-2 hours
|
|
||||||
**Risk:** ⚠️ 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
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Detailed Test-by-Test Analysis
|
|
||||||
|
|
||||||
### `member_policies_test.exs` - Test Classification
|
|
||||||
|
|
||||||
#### own_data Permission Set (7 tests)
|
|
||||||
|
|
||||||
| Test | Type | Action |
|
|
||||||
|------|------|--------|
|
|
||||||
| `can read linked member` | ✅ Business Logic | Keep |
|
|
||||||
| `can update linked member` | ✅ Business Logic | Keep |
|
|
||||||
| `cannot read unlinked member` | ⚠️ Framework (auto-filter) | Remove |
|
|
||||||
| `cannot update unlinked member` | ⚠️ Framework (error type) | Remove |
|
|
||||||
| `list members returns only linked member` | ✅ Business Logic | Keep |
|
|
||||||
| `cannot create member` | ⚠️ Framework (error type) | Remove |
|
|
||||||
| `cannot destroy member` | ⚠️ Framework (error type) | Remove |
|
|
||||||
|
|
||||||
#### read_only Permission Set (6 tests)
|
|
||||||
|
|
||||||
| Test | Type | Action |
|
|
||||||
|------|------|--------|
|
|
||||||
| `can read all members` | ✅ Business Logic | Keep |
|
|
||||||
| `can read individual member` | ✅ Business Logic | Keep |
|
|
||||||
| `cannot create member` | ⚠️ Framework (error type) | Remove |
|
|
||||||
| `cannot update any member` | ⚠️ Framework (error type) | Remove |
|
|
||||||
| `cannot destroy any member` | ⚠️ Framework (error type) | Remove |
|
|
||||||
| *(Missing: can read linked member test)* | - | - |
|
|
||||||
|
|
||||||
#### normal_user Permission Set (5 tests)
|
|
||||||
|
|
||||||
| Test | Type | Action |
|
|
||||||
|------|------|--------|
|
|
||||||
| `can read all members` | ✅ Business Logic | Keep |
|
|
||||||
| `can create member` | ✅ Business Logic | Keep |
|
|
||||||
| `can update any member` | ✅ Business Logic | Keep |
|
|
||||||
| `cannot destroy member` | ⚠️ Framework (error type) | Remove |
|
|
||||||
| *(Missing: cannot read unlinked test)* | - | - |
|
|
||||||
|
|
||||||
#### admin Permission Set (4 tests)
|
|
||||||
|
|
||||||
| Test | Type | Action |
|
|
||||||
|------|------|--------|
|
|
||||||
| `can read all members` | ✅ Business Logic | Keep |
|
|
||||||
| `can create member` | ✅ Business Logic | Keep |
|
|
||||||
| `can update any member` | ✅ Business Logic | Keep |
|
|
||||||
| `can destroy any member` | ✅ Business Logic | Keep |
|
|
||||||
|
|
||||||
#### Special Case (3 tests)
|
|
||||||
|
|
||||||
| Test | Type | Action |
|
|
||||||
|------|------|--------|
|
|
||||||
| `read_only user can read linked member` | ✅ Business Logic | Keep |
|
|
||||||
| `own_data user can read linked member` | ✅ Business Logic | Keep |
|
|
||||||
| `own_data user can update linked member` | ✅ Business Logic | Keep |
|
|
||||||
|
|
||||||
**Total Tests:** 24 → **14 tests** (remove 10 framework tests)
|
|
||||||
|
|
||||||
### `user_policies_test.exs` - Test Classification
|
|
||||||
|
|
||||||
#### own_data Permission Set (7 tests)
|
|
||||||
|
|
||||||
| Test | Type | Action |
|
|
||||||
|------|------|--------|
|
|
||||||
| `can read own user record` | ⚠️ Redundant | Consolidate |
|
|
||||||
| `can update own email` | ⚠️ Redundant | Consolidate |
|
|
||||||
| `cannot read other users` | ⚠️ Framework (auto-filter) | Remove |
|
|
||||||
| `cannot update other users` | ⚠️ Framework (error type) | Remove |
|
|
||||||
| `list users returns only own user` | ⚠️ Framework (auto-filter) | Remove |
|
|
||||||
| `cannot create user` | ⚠️ Framework (error type) | Remove |
|
|
||||||
| `cannot destroy user` | ⚠️ Framework (error type) | Remove |
|
|
||||||
|
|
||||||
#### read_only Permission Set (7 tests)
|
|
||||||
|
|
||||||
| Test | Type | Action |
|
|
||||||
|------|------|--------|
|
|
||||||
| `can read own user record` | ⚠️ Redundant | Consolidate |
|
|
||||||
| `can update own email` | ⚠️ Redundant | Consolidate |
|
|
||||||
| `cannot read other users` | ⚠️ Framework (auto-filter) | Remove |
|
|
||||||
| `cannot update other users` | ⚠️ Framework (error type) | Remove |
|
|
||||||
| `list users returns only own user` | ⚠️ Framework (auto-filter) | Remove |
|
|
||||||
| `cannot create user` | ⚠️ Framework (error type) | Remove |
|
|
||||||
| `cannot destroy user` | ⚠️ Framework (error type) | Remove |
|
|
||||||
|
|
||||||
#### normal_user Permission Set (7 tests)
|
|
||||||
|
|
||||||
| Test | Type | Action |
|
|
||||||
|------|------|--------|
|
|
||||||
| `can read own user record` | ⚠️ Redundant | Consolidate |
|
|
||||||
| `can update own email` | ⚠️ Redundant | Consolidate |
|
|
||||||
| `cannot read other users` | ⚠️ Framework (auto-filter) | Remove |
|
|
||||||
| `cannot update other users` | ⚠️ Framework (error type) | Remove |
|
|
||||||
| `list users returns only own user` | ⚠️ Framework (auto-filter) | Remove |
|
|
||||||
| `cannot create user` | ⚠️ Framework (error type) | Remove |
|
|
||||||
| `cannot destroy user` | ⚠️ Framework (error type) | Remove |
|
|
||||||
|
|
||||||
#### admin Permission Set (5 tests)
|
|
||||||
|
|
||||||
| Test | Type | Action |
|
|
||||||
|------|------|--------|
|
|
||||||
| `can read all users` | ✅ Business Logic | Keep |
|
|
||||||
| `can read other users` | ✅ Business Logic | Keep |
|
|
||||||
| `can update other users` | ✅ Business Logic | Keep |
|
|
||||||
| `can create user` | ✅ Business Logic | Keep |
|
|
||||||
| `can destroy user` | ✅ Business Logic | Keep |
|
|
||||||
|
|
||||||
#### AshAuthentication Bypass (3 tests)
|
|
||||||
|
|
||||||
| Test | Type | Action |
|
|
||||||
|------|------|--------|
|
|
||||||
| `register_with_password works without actor` | ✅ Business Logic | Keep |
|
|
||||||
| `register_with_rauthy works without actor` | ✅ Business Logic | Keep |
|
|
||||||
| `sign_in_with_rauthy works without actor` | ✅ Business Logic | Keep |
|
|
||||||
|
|
||||||
**Total Tests:** 30 → **14 tests** (remove 16 framework/redundant tests)
|
|
||||||
|
|
||||||
### `custom_field_value_policies_test.exs` - Test Classification
|
|
||||||
|
|
||||||
**Similar patterns to `member_policies_test.exs`:**
|
|
||||||
|
|
||||||
- Remove "cannot" tests (framework)
|
|
||||||
- Keep "can" tests (business logic)
|
|
||||||
- Keep special case tests (business logic)
|
|
||||||
|
|
||||||
**Estimated:** 20 → **12 tests** (remove 8 framework tests)
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Summary of Tests to Remove/Consolidate
|
|
||||||
|
|
||||||
### Remove (Framework Functionality)
|
|
||||||
|
|
||||||
**`member_policies_test.exs`:** 10 tests
|
|
||||||
- `cannot create member` (3 tests)
|
|
||||||
- `cannot destroy member` (3 tests)
|
|
||||||
- `cannot update unlinked member` (1 test)
|
|
||||||
- `cannot read unlinked member` (1 test - auto-filter)
|
|
||||||
- *(2 more framework tests)*
|
|
||||||
|
|
||||||
**`user_policies_test.exs`:** 16 tests
|
|
||||||
- `cannot read other users` (3 tests - auto-filter)
|
|
||||||
- `cannot update other users` (3 tests)
|
|
||||||
- `cannot create user` (3 tests)
|
|
||||||
- `cannot destroy user` (3 tests)
|
|
||||||
- `list users returns only own user` (3 tests - auto-filter)
|
|
||||||
- `can read own user record` (3 tests - redundant)
|
|
||||||
- `can update own email` (3 tests - redundant)
|
|
||||||
|
|
||||||
**`custom_field_value_policies_test.exs`:** 8 tests
|
|
||||||
- Similar "cannot" tests
|
|
||||||
|
|
||||||
**Total to Remove:** ~34 tests
|
|
||||||
|
|
||||||
### Consolidate (Redundant)
|
|
||||||
|
|
||||||
**`user_policies_test.exs`:** 6 tests → 2 tests
|
|
||||||
- `can read own user record` (3 tests) → 1 integration test
|
|
||||||
- `can update own email` (3 tests) → 1 integration test
|
|
||||||
|
|
||||||
**Total to Consolidate:** 6 tests → 2 tests
|
|
||||||
|
|
||||||
### Keep (Business Logic)
|
|
||||||
|
|
||||||
**All "can" tests that verify permission set behavior:**
|
|
||||||
- `can read linked/all members` - Tests our scope definitions
|
|
||||||
- `can update linked/any member` - Tests our permission sets
|
|
||||||
- `can create member/user` - Tests our permission sets
|
|
||||||
- `can destroy member/user` - Tests our permission sets
|
|
||||||
- `list members returns only linked member` - Tests our scope filters
|
|
||||||
|
|
||||||
**Special case tests:**
|
|
||||||
- `user can always READ linked member` - Tests our custom business rule
|
|
||||||
|
|
||||||
**AshAuthentication bypass tests:**
|
|
||||||
- All bypass tests - Tests our integration
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Questions for Decision
|
|
||||||
|
|
||||||
1. **Are we comfortable removing framework functionality tests?**
|
|
||||||
- Ash maintainers test the framework
|
|
||||||
- Our tests should focus on business logic
|
|
||||||
- Risk is very low
|
|
||||||
|
|
||||||
2. **Should we consolidate redundant tests?**
|
|
||||||
- Tests verify same behavior across permission sets
|
|
||||||
- Integration tests can cover multiple sets
|
|
||||||
- Risk is low
|
|
||||||
|
|
||||||
3. **Can we share admin user across describe blocks?**
|
|
||||||
- Admin user is read-only in tests
|
|
||||||
- `async: false` prevents `setup_all`, but module-level `setup` works
|
|
||||||
- Risk is low
|
|
||||||
|
|
||||||
4. **What's the minimum test coverage we need?**
|
|
||||||
- One test per permission set per action?
|
|
||||||
- Or integration tests covering all sets?
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## References
|
|
||||||
|
|
||||||
- **Coding Guidelines:** `CODE_GUIDELINES.md` - Section 4.7 (Testing Best Practices)
|
|
||||||
- **Test Performance Optimization:** `docs/test-performance-optimization.md`
|
|
||||||
- **User LiveView Analysis:** `docs/test-optimization-user-liveview-analysis.md`
|
|
||||||
- **Phase 2 Analysis:** `docs/test-optimization-phase2-shared-fixtures-analysis.md`
|
|
||||||
|
|
@ -1,524 +0,0 @@
|
||||||
# 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
|
|
||||||
|
|
@ -373,30 +373,190 @@ After implementing the full test suite via promotion, the remaining slowest test
|
||||||
|
|
||||||
### Priority 1: User LiveView Tests Optimization
|
### Priority 1: User LiveView Tests Optimization
|
||||||
|
|
||||||
**Estimated Savings:** 15-20 seconds
|
**Estimated Savings:** 14-22 seconds
|
||||||
|
**Status:** 📋 Analysis Complete - Ready for Implementation
|
||||||
|
|
||||||
**Actions:**
|
#### Analysis Summary
|
||||||
1. Move shared fixtures to `setup_all` where possible
|
|
||||||
2. Reduce test data volume (use 3-5 users instead of 10+)
|
Analysis of User LiveView tests identified significant optimization opportunities:
|
||||||
3. Analyze and optimize recurring setup patterns
|
- **Framework functionality over-testing:** ~30 tests test Phoenix/Ash/Gettext core features
|
||||||
4. Consider mocking expensive operations (if appropriate)
|
- **Redundant test data creation:** Each test creates users/members independently
|
||||||
|
- **Missing shared fixtures:** No `setup_all` usage for common data
|
||||||
|
|
||||||
|
#### Current Performance
|
||||||
|
|
||||||
|
**Top 20 Slowest Tests (User LiveView):**
|
||||||
|
- `index_test.exs`: ~10.2s for 3 tests in Top 20
|
||||||
|
- `form_test.exs`: ~15.0s for 4 tests in Top 20
|
||||||
|
- `show_test.exs`: ~10.3s for 3 tests in Top 20
|
||||||
|
- **Total:** ~35.5 seconds for User LiveView tests
|
||||||
|
|
||||||
|
#### Optimization Opportunities
|
||||||
|
|
||||||
|
**1. Remove Framework Functionality Tests (~30 tests, 8-12s saved)**
|
||||||
|
- Remove translation tests (Gettext framework)
|
||||||
|
- Remove navigation tests (Phoenix LiveView framework)
|
||||||
|
- Remove validation tests (Ash framework)
|
||||||
|
- Remove basic HTML rendering tests (consolidate into smoke test)
|
||||||
|
- Remove password storage tests (AshAuthentication framework)
|
||||||
|
|
||||||
|
**2. Implement Shared Fixtures (3-5s saved)**
|
||||||
|
- Use `setup_all` for common test data in `index_test.exs` and `show_test.exs`
|
||||||
|
- Share users for sorting/checkbox tests
|
||||||
|
- Share common users/members across tests
|
||||||
|
- **Note:** `form_test.exs` uses `async: false`, preventing `setup_all` usage
|
||||||
|
|
||||||
|
**3. Consolidate Redundant Tests (~10 tests → 3-4 tests, 2-3s saved)**
|
||||||
|
- Merge basic display tests into smoke test
|
||||||
|
- Merge navigation tests into integration test
|
||||||
|
- Reduce sorting tests to 1 integration test
|
||||||
|
|
||||||
|
**4. Optimize Test Data Volume (1-2s saved)**
|
||||||
|
- Use minimum required data (2 users for sorting, 2 for checkboxes)
|
||||||
|
- Share data across tests via `setup_all`
|
||||||
|
|
||||||
|
#### Tests to Keep (Business Logic)
|
||||||
|
|
||||||
|
**Index Tests:**
|
||||||
|
- `initially sorts by email ascending` - Tests default sort
|
||||||
|
- `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
|
||||||
|
- Edge case tests
|
||||||
|
|
||||||
|
**Form Tests:**
|
||||||
|
- `creates user without password` - Business logic
|
||||||
|
- `creates user with password when enabled` - Business logic
|
||||||
|
- `admin sets new password for user` - Business logic
|
||||||
|
- `selecting member and saving links member to user` - Business logic
|
||||||
|
- Member linking/unlinking workflow tests
|
||||||
|
|
||||||
|
**Show Tests:**
|
||||||
|
- `displays password authentication status` - Business logic
|
||||||
|
- `displays linked member when present` - Business logic
|
||||||
|
- `redirects to user list when viewing system actor user` - Business rule
|
||||||
|
|
||||||
|
#### Implementation Plan
|
||||||
|
|
||||||
|
**Phase 1: Remove Framework Tests (1-2 hours, ⚠️ Very Low Risk)**
|
||||||
|
- Remove translation, navigation, validation, and basic HTML rendering tests
|
||||||
|
- Consolidate remaining display tests into smoke test
|
||||||
|
|
||||||
|
**Phase 2: Implement Shared Fixtures (2-3 hours, ⚠️ Low Risk)**
|
||||||
|
- Add `setup_all` to `index_test.exs` and `show_test.exs`
|
||||||
|
- Update tests to use shared fixtures
|
||||||
|
- Verify test isolation maintained
|
||||||
|
|
||||||
|
**Phase 3: Consolidate Tests (1-2 hours, ⚠️ Very Low Risk)**
|
||||||
|
- Merge basic display tests into smoke test
|
||||||
|
- Merge navigation tests into integration test
|
||||||
|
- Reduce sorting tests to 1 integration test
|
||||||
|
|
||||||
**Risk Assessment:** ⚠️ **Low**
|
**Risk Assessment:** ⚠️ **Low**
|
||||||
- Requires careful testing to ensure isolation is maintained
|
- Framework functionality is tested by framework maintainers
|
||||||
- Should verify that shared fixtures don't cause test interdependencies
|
- Business logic tests remain intact
|
||||||
|
- Shared fixtures maintain test isolation
|
||||||
|
- Consolidation preserves coverage
|
||||||
|
|
||||||
### Priority 2: Policy Tests Optimization
|
### Priority 2: Policy Tests Optimization
|
||||||
|
|
||||||
**Estimated Savings:** 5-8 seconds
|
**Estimated Savings:** 5.5-9 seconds
|
||||||
|
**Status:** 📋 Analysis Complete - Ready for Decision
|
||||||
|
|
||||||
**Actions:**
|
#### Analysis Summary
|
||||||
1. Create roles in `setup_all` (they're identical across tests)
|
|
||||||
2. Reuse common fixtures (users, members)
|
Analysis of policy tests identified significant optimization opportunities:
|
||||||
3. Maintain test isolation while optimizing setup
|
- **Redundant fixture creation:** Roles and users created repeatedly across tests
|
||||||
|
- **Framework functionality over-testing:** Many tests verify Ash policy framework behavior
|
||||||
|
- **Test duplication:** Similar tests across different permission sets
|
||||||
|
|
||||||
|
#### Current Performance
|
||||||
|
|
||||||
|
**Policy Test Files Performance:**
|
||||||
|
- `member_policies_test.exs`: 24 tests, ~66s (top 20)
|
||||||
|
- `user_policies_test.exs`: 30 tests, ~66s (top 20)
|
||||||
|
- `custom_field_value_policies_test.exs`: 20 tests, ~66s (top 20)
|
||||||
|
- **Total:** 74 tests, ~152s total
|
||||||
|
|
||||||
|
**Top 20 Slowest Policy Tests:** ~66 seconds
|
||||||
|
|
||||||
|
#### Framework vs. Business Logic Analysis
|
||||||
|
|
||||||
|
**Framework Functionality (Should NOT Test):**
|
||||||
|
- Policy evaluation (how Ash evaluates policies)
|
||||||
|
- Permission lookup (how Ash looks up permissions)
|
||||||
|
- Scope filtering (how Ash applies scope filters)
|
||||||
|
- Auto-filter behavior (how Ash auto-filters queries)
|
||||||
|
- Forbidden vs NotFound (how Ash returns errors)
|
||||||
|
|
||||||
|
**Business Logic (Should Test):**
|
||||||
|
- Permission set definitions (what permissions each role has)
|
||||||
|
- Scope definitions (what scopes each permission set uses)
|
||||||
|
- Special cases (custom business rules)
|
||||||
|
- Permission set behavior (how our permission sets differ)
|
||||||
|
|
||||||
|
#### Optimization Opportunities
|
||||||
|
|
||||||
|
**1. Remove Framework Functionality Tests (~22-34 tests, 3-4s saved)**
|
||||||
|
- Remove "cannot" tests that verify error types (Forbidden, NotFound)
|
||||||
|
- Remove tests that verify auto-filter behavior (framework)
|
||||||
|
- Remove tests that verify permission evaluation (framework)
|
||||||
|
- **Risk:** ⚠️ Very Low - Framework functionality is tested by Ash maintainers
|
||||||
|
|
||||||
|
**2. Consolidate Redundant Tests (~6-8 tests → 2-3 tests, 1-2s saved)**
|
||||||
|
- Merge similar tests across permission sets
|
||||||
|
- Create integration tests that cover multiple permission sets
|
||||||
|
- **Risk:** ⚠️ Low - Same coverage, fewer tests
|
||||||
|
|
||||||
|
**3. Share Admin User Across Describe Blocks (1-2s saved)**
|
||||||
|
- Create admin user once in module-level `setup`
|
||||||
|
- Reuse admin user in helper functions
|
||||||
|
- **Note:** `async: false` prevents `setup_all`, but module-level `setup` works
|
||||||
|
- **Risk:** ⚠️ Low - Admin user is read-only in tests, safe to share
|
||||||
|
|
||||||
|
**4. Reduce Test Data Volume (0.5-1s saved)**
|
||||||
|
- Use minimum required data
|
||||||
|
- Share fixtures where possible
|
||||||
|
- **Risk:** ⚠️ Very Low - Still tests same functionality
|
||||||
|
|
||||||
|
#### Test Classification Summary
|
||||||
|
|
||||||
|
**Tests to Remove (Framework):**
|
||||||
|
- `member_policies_test.exs`: ~10 tests (cannot create/destroy/update, auto-filter tests)
|
||||||
|
- `user_policies_test.exs`: ~16 tests (cannot read/update/create/destroy, auto-filter tests)
|
||||||
|
- `custom_field_value_policies_test.exs`: ~8 tests (similar "cannot" tests)
|
||||||
|
|
||||||
|
**Tests to Consolidate (Redundant):**
|
||||||
|
- `user_policies_test.exs`: 6 tests → 2 tests (can read/update own user record)
|
||||||
|
|
||||||
|
**Tests to Keep (Business Logic):**
|
||||||
|
- All "can" tests that verify permission set behavior
|
||||||
|
- Special case tests (e.g., "user can always READ linked member")
|
||||||
|
- AshAuthentication bypass tests (our integration)
|
||||||
|
|
||||||
|
#### Implementation Plan
|
||||||
|
|
||||||
|
**Phase 1: Remove Framework Tests (1-2 hours, ⚠️ Very Low Risk)**
|
||||||
|
- Identify all "cannot" tests that verify error types
|
||||||
|
- Remove tests that verify Ash auto-filter behavior
|
||||||
|
- Remove tests that verify permission evaluation (framework)
|
||||||
|
|
||||||
|
**Phase 2: Consolidate Redundant Tests (1-2 hours, ⚠️ Low Risk)**
|
||||||
|
- Identify similar tests across permission sets
|
||||||
|
- Create integration tests that cover multiple permission sets
|
||||||
|
- Remove redundant individual tests
|
||||||
|
|
||||||
|
**Phase 3: Share Admin User (1-2 hours, ⚠️ Low Risk)**
|
||||||
|
- Add module-level `setup` to create admin user
|
||||||
|
- Update helper functions to accept admin user parameter
|
||||||
|
- Update all `setup` blocks to use shared admin user
|
||||||
|
|
||||||
**Risk Assessment:** ⚠️ **Low**
|
**Risk Assessment:** ⚠️ **Low**
|
||||||
- Roles are read-only in tests, safe to share
|
- Framework functionality is tested by Ash maintainers
|
||||||
- Need to ensure user/member fixtures don't interfere with each other
|
- Business logic tests remain intact
|
||||||
|
- Admin user sharing maintains test isolation (read-only)
|
||||||
|
- Consolidation preserves coverage
|
||||||
|
|
||||||
### Priority 3: Seeds Tests Further Optimization
|
### Priority 3: Seeds Tests Further Optimization
|
||||||
|
|
||||||
|
|
@ -431,13 +591,17 @@ After implementing the full test suite via promotion, the remaining slowest test
|
||||||
|
|
||||||
| Priority | Optimization | Estimated Savings |
|
| Priority | Optimization | Estimated Savings |
|
||||||
|----------|-------------|-------------------|
|
|----------|-------------|-------------------|
|
||||||
| 1 | User LiveView Tests | 15-20s |
|
| 1 | User LiveView Tests | 14-22s |
|
||||||
| 2 | Policy Tests | 5-8s |
|
| 2 | Policy Tests | 5.5-9s |
|
||||||
| 3 | Seeds Tests Further | 3-5s |
|
| 3 | Seeds Tests Further | 3-5s |
|
||||||
| 4 | Additional Isolation | Variable |
|
| 4 | Additional Isolation | Variable |
|
||||||
| **Total Potential** | | **23-33 seconds** |
|
| **Total Potential** | | **22.5-36 seconds** |
|
||||||
|
|
||||||
**Projected Final Time:** From ~368 seconds (fast suite) to **~340-350 seconds** (~5.7-5.8 minutes) with remaining optimizations
|
**Projected Final Time:** From ~368 seconds (fast suite) to **~332-345 seconds** (~5.5-5.8 minutes) with remaining optimizations
|
||||||
|
|
||||||
|
**Note:** Detailed analysis documents available:
|
||||||
|
- User LiveView Tests: See "Priority 1: User LiveView Tests Optimization" section above
|
||||||
|
- Policy Tests: See "Priority 2: Policy Tests Optimization" section above
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue