From 8e9fbe76cf5e8f39b4a94738e31b80bdad579e7a Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 27 Jan 2026 15:22:41 +0100 Subject: [PATCH] docs: add testing philosophy to coding guideline and update groups architecture docs #371 --- CODE_GUIDELINES.md | 34 ++++++++++++++ docs/groups-architecture.md | 94 +++++++++++++++++++++++++------------ 2 files changed, 99 insertions(+), 29 deletions(-) diff --git a/CODE_GUIDELINES.md b/CODE_GUIDELINES.md index 17b03d0..4905b79 100644 --- a/CODE_GUIDELINES.md +++ b/CODE_GUIDELINES.md @@ -1515,6 +1515,40 @@ mix test test/membership/member_test.exs:42 ### 4.7 Testing Best Practices +**Testing Philosophy: Focus on Business Logic, Not Framework Functionality** + +We test our business logic and domain-specific behavior, not core framework features. Framework features (Ash validations, Ecto relationships, etc.) are already tested by their respective libraries. + +**What We Test:** +- Business rules and validations specific to our domain +- Custom business logic (slug generation, calculations, etc.) +- Integration between our resources +- Database-level constraints (unique constraints, foreign keys, CASCADE) +- Query performance (N+1 prevention) + +**What We Don't Test:** +- Framework core functionality (Ash validations work, Ecto relationships work, etc.) +- Standard CRUD operations without custom logic +- Framework-provided features that are already tested upstream +- Detailed slug generation edge cases (Umlauts, truncation, etc.) if covered by reusable change tests + +**Example:** +```elixir +# ✅ GOOD - Tests our business rule +test "slug is immutable (doesn't change when name is updated)" do + {:ok, group} = Membership.create_group(%{name: "Original"}, actor: actor) + original_slug = group.slug + + {:ok, updated} = Membership.update_group(group, %{name: "New"}, actor: actor) + assert updated.slug == original_slug # Business rule: slug doesn't change +end + +# ❌ AVOID - Tests framework functionality +test "Ash.Changeset validates required fields" do + # This is already tested by Ash framework +end +``` + **Descriptive Test Names:** ```elixir diff --git a/docs/groups-architecture.md b/docs/groups-architecture.md index 66c663d..5e31bc5 100644 --- a/docs/groups-architecture.md +++ b/docs/groups-architecture.md @@ -944,6 +944,24 @@ Each functional unit can be implemented as a **separate issue**: ## Testing Strategy +### Testing Philosophy + +**Focus on Business Logic, Not Framework Functionality** + +We test our business logic and domain-specific behavior, not core framework features. Framework features (Ash validations, Ecto relationships, etc.) are already tested by their respective libraries. + +**What We Test:** +- Business rules and validations specific to our domain +- Custom business logic (slug generation, calculations, etc.) +- Integration between our resources +- Database-level constraints (unique constraints, foreign keys, CASCADE) +- Query performance (N+1 prevention) + +**What We Don't Test:** +- Framework core functionality (Ash validations work, Ecto relationships work, etc.) +- Standard CRUD operations without custom logic +- Framework-provided features that are already tested upstream + ### Unit Tests #### Group Resource Tests @@ -952,31 +970,23 @@ Each functional unit can be implemented as a **separate issue**: **Test Cases:** - Create group with valid attributes -- Return error when name is missing -- Return error when name exceeds 100 characters -- Return error when name is not unique -- Name uniqueness is case-insensitive -- Allow description to be nil -- Trim whitespace from name -- Description max length is 500 characters -- Slug is automatically generated from name on create -- Slug is immutable (doesn't change on update) -- Slug is unique (cannot have duplicates) -- Slug handles German umlauts correctly (ä → a, ß → ss) -- Slug converts to lowercase -- Slug removes special characters -- Slug replaces spaces with hyphens -- Slug truncates to max 100 characters -- Slug cannot be empty (validation fails if slug would be empty) -- Read group by slug (via identity lookup) +- Return error when name is missing (required validation) +- Return error when name exceeds 100 characters (length validation) +- Return error when name is not unique (case-insensitive) - application level validation +- Allow description to be nil (optional field) +- Description max length is 500 characters (length validation) +- Slug is automatically generated from name on create (custom business logic) +- Slug is immutable (doesn't change when name is updated) - business rule +- Slug is unique (prevents duplicate slugs from different names) - business rule +- Slug cannot be empty (rejects name with only special characters) - business rule - Update group name and description -- Prevent duplicate name on update -- Prevent duplicate slug on create (same name → same slug → error) -- Delete group and all member associations -- Do not delete members themselves -- Member count calculation returns 0 for empty group -- Member count calculation returns correct count when members added -- Member count updates correctly when members removed +- Prevent duplicate name on update (case-insensitive) - business rule +- Delete group and all member associations (cascade behavior) +- Do not delete members themselves (cascade boundary) +- Member count calculation returns 0 for empty group (custom calculation) +- Member count calculation returns correct count when members added/removed (custom calculation) + +**Note:** Detailed slug generation tests (Umlauts, truncation, etc.) are covered by the `GenerateSlug` change tests in `custom_field_slug_test.exs`, which is reused for groups. We don't duplicate these framework-level tests. #### MemberGroup Resource Tests @@ -984,9 +994,23 @@ Each functional unit can be implemented as a **separate issue**: **Test Cases:** - Create association between member and group -- Prevent duplicate associations -- Cascade delete when member deleted -- Cascade delete when group deleted +- Prevent duplicate associations (unique constraint) +- Cascade delete when member deleted (database constraint) +- Cascade delete when group deleted (database constraint) + +#### Database Constraint Tests + +**File:** `test/membership/group_database_constraints_test.exs` + +**Test Cases:** +- Database enforces unique name constraint (case-insensitive via LOWER) - DB level +- Database enforces unique slug constraint (case-sensitive) - DB level +- Cannot create MemberGroup with non-existent member_id (foreign key constraint) +- Cannot create MemberGroup with non-existent group_id (foreign key constraint) +- Deleting member cascades to member_groups (verified at DB level) +- Deleting group cascades to member_groups (verified at DB level) + +**Note:** These tests verify that constraints are enforced at the database level, not just application level. ### Integration Tests @@ -995,8 +1019,20 @@ Each functional unit can be implemented as a **separate issue**: **File:** `test/membership/group_integration_test.exs` **Test Cases:** -- Member can belong to multiple groups -- Group can contain multiple members +- Member can belong to multiple groups (many-to-many relationship) +- Group can contain multiple members (many-to-many relationship) +- Preloading groups with members avoids N+1 queries (performance test with query count verification) + +**File:** `test/membership/member_groups_relationship_test.exs` + +**Test Cases:** +- Member has many_to_many groups relationship (load with preloading) +- Load multiple members with groups preloaded (N+1 prevention) +- Add member to group via Ash API +- Remove member from group via Ash API +- Add member to multiple groups in single operation +- Adding member to same group twice fails (duplicate prevention) +- Removing member from group they're not in (idempotent, no error) ### UI Tests