diff --git a/.drone.yml b/.drone.yml index a529207..129bc4c 100644 --- a/.drone.yml +++ b/.drone.yml @@ -215,7 +215,7 @@ steps: echo "Postgres did not become available, aborting." exit 1 - - name: test-slow + - name: test-all image: docker.io/library/elixir:1.18.3-otp-27 environment: MIX_ENV: test @@ -226,5 +226,5 @@ steps: - mix local.hex --force # Fetch dependencies - mix deps.get - # Run only all tests + # Run all tests (including slow/performance tests) - mix test diff --git a/CODE_GUIDELINES.md b/CODE_GUIDELINES.md index b11a369..3f43230 100644 --- a/CODE_GUIDELINES.md +++ b/CODE_GUIDELINES.md @@ -1656,7 +1656,7 @@ end **Performance Tests:** -Performance tests that explicitly validate performance characteristics should be tagged with `@tag :slow` or `@moduletag :slow` to exclude them from standard test runs. This improves developer feedback loops while maintaining comprehensive coverage. +Performance tests that explicitly validate performance characteristics should be tagged with `@tag :slow` or `@describetag :slow` to exclude them from standard test runs. This improves developer feedback loops while maintaining comprehensive coverage. **When to Tag as `:slow`:** @@ -1664,6 +1664,12 @@ Performance tests that explicitly validate performance characteristics should be - Tests that use large datasets (e.g., 50+ records) to test scalability - Tests that validate query optimization (N+1 prevention, index usage) +**Tagging Guidelines:** + +- Use `@tag :slow` for individual tests +- Use `@describetag :slow` for entire describe blocks (not `@moduletag`, as it affects all tests in the module) +- Performance tests should include measurable assertions (query counts, timing with tolerance, etc.) + **Running Tests:** ```bash @@ -1673,7 +1679,7 @@ mix test --exclude slow # Performance tests only mix test --only slow -# All tests +# All tests (including slow tests) mix test ``` --- diff --git a/docs/test-performance-optimization.md b/docs/test-performance-optimization.md index cbff69f..6002f16 100644 --- a/docs/test-performance-optimization.md +++ b/docs/test-performance-optimization.md @@ -172,7 +172,7 @@ mix test #### CI/CD Integration - **Standard CI:** Runs `mix test --exclude slow` for faster feedback loops (~6 minutes) -- **Nightly Builds:** Separate pipeline runs daily and executes `mix test --only slow` (~1.3 minutes) +- **Nightly Builds:** Separate pipeline runs daily and executes `mix test` (all tests, including slow) for comprehensive coverage (~7.4 minutes) - **Pre-Merge:** Full test suite (`mix test`) runs before merging to main - **Manual Execution:** Can be triggered via Drone CLI or Web UI @@ -535,13 +535,18 @@ mix test test/mv_web/member_live/index_member_fields_display_test.exs --slowest - Execution time: ~6 minutes - Command: `mix test --exclude slow` or `just test-fast` -**Slow/Nightly Tests:** -- Tests tagged with `@tag :slow` (25 tests) +**Slow Tests:** +- Tests tagged with `@tag :slow` or `@describetag :slow` (25 tests) - Low risk, >1 second execution time -- UI/Display tests, workflow details, edge cases +- UI/Display tests, workflow details, edge cases, performance tests - Execution time: ~1.3 minutes - Command: `mix test --only slow` or `just test-slow` -- Run in nightly CI builds +- Excluded from standard CI runs + +**Nightly CI Builds:** +- Runs all tests (`mix test`) for comprehensive coverage +- Execution time: ~7.4 minutes +- Ensures full test coverage including slow/performance tests **All Tests:** - Includes both fast and slow tests @@ -692,13 +697,13 @@ A: Monitor for 2-3 months. If no seeds-related bugs appear that would have been A: Consider running the full test suite (including slow tests) before major releases. Daily development uses the optimized suite. **Q: How do I add a new performance test?** -A: Tag it with `@tag :slow` or `@moduletag :slow`. See "Performance Test Guidelines" section above. +A: Tag it with `@tag :slow` for individual tests or `@describetag :slow` for describe blocks. Use `@describetag` instead of `@moduletag` to avoid tagging unrelated tests. Include measurable performance assertions (query counts, timing with tolerance, etc.). See "Performance Test Guidelines" section above. **Q: Can I run slow tests locally?** A: Yes, use `just test-slow` or `mix test --only slow`. They're excluded from standard runs for faster feedback. **Q: What is the "nightly suite"?** -A: The "nightly suite" refers to tests tagged with `@tag :slow`. We use a single tag (`:slow`) for both performance tests and tests suitable for nightly execution. All tests tagged with `:slow` are excluded from standard runs and can be executed in nightly CI builds. +A: The "nightly suite" refers to the nightly CI pipeline that runs **all tests** (`mix test`), including slow tests. Tests tagged with `@tag :slow` or `@describetag :slow` are excluded from standard CI runs for faster feedback, but are included in the nightly full test suite for comprehensive coverage. **Q: Which tests should I tag as `:slow`?** A: Tag tests with `@tag :slow` if they: (1) take >1 second, (2) have low risk (not critical for catching regressions), and (3) test UI/Display/Formatting or workflow details. See "Test Tagging Guidelines" section for details. diff --git a/test/mv_web/live/group_live/index_test.exs b/test/mv_web/live/group_live/index_test.exs index e844bcc..8ce3e58 100644 --- a/test/mv_web/live/group_live/index_test.exs +++ b/test/mv_web/live/group_live/index_test.exs @@ -114,17 +114,36 @@ defmodule MvWeb.GroupLive.IndexTest do end end - @moduletag :slow describe "performance" do + @describetag :slow test "page loads efficiently with many groups", %{conn: conn} do # Create multiple groups Enum.each(1..10, fn _ -> Fixtures.group_fixture() end) + # Count queries using Telemetry + query_count_agent = Agent.start_link(fn -> 0 end) |> elem(1) + + handler = fn _event, _measurements, _metadata, _config -> + Agent.update(query_count_agent, &(&1 + 1)) + end + + handler_id = "test-query-counter-#{System.unique_integer([:positive])}" + :telemetry.attach(handler_id, [:ash, :query, :start], handler, nil) + # Should load without N+1 queries {:ok, _view, html} = live(conn, "/groups") + final_count = Agent.get(query_count_agent, & &1) + :telemetry.detach(handler_id) + # Verify all groups are displayed assert html =~ gettext("Groups") + + # Verify query count is reasonable (should avoid N+1 queries) + # Expected: 1 query for groups list + possible count queries + # Allow some overhead for LiveView setup queries + assert final_count <= 5, + "Expected max 5 queries (groups list + possible counts + LiveView setup), got #{final_count}. This suggests N+1 query problem." end test "member count is loaded efficiently via calculation", %{conn: conn} do @@ -143,10 +162,29 @@ defmodule MvWeb.GroupLive.IndexTest do actor: system_actor ) + # Count queries using Telemetry + query_count_agent = Agent.start_link(fn -> 0 end) |> elem(1) + + handler = fn _event, _measurements, _metadata, _config -> + Agent.update(query_count_agent, &(&1 + 1)) + end + + handler_id = "test-query-counter-#{System.unique_integer([:positive])}" + :telemetry.attach(handler_id, [:ash, :query, :start], handler, nil) + {:ok, _view, html} = live(conn, "/groups") + final_count = Agent.get(query_count_agent, & &1) + :telemetry.detach(handler_id) + # Member count should be displayed (should be 2) assert html =~ "2" or html =~ gettext("Members") or html =~ "Mitglieder" + + # Verify query count is reasonable (member count should be calculated efficiently) + # Expected: 1 query for groups + 1 query for member counts (aggregated) + # Allow some overhead for LiveView setup queries + assert final_count <= 5, + "Expected max 5 queries (groups + member counts + LiveView setup), got #{final_count}. This suggests inefficient member count calculation." end end end diff --git a/test/mv_web/live/group_live/show_test.exs b/test/mv_web/live/group_live/show_test.exs index 177318e..50424ac 100644 --- a/test/mv_web/live/group_live/show_test.exs +++ b/test/mv_web/live/group_live/show_test.exs @@ -219,8 +219,8 @@ defmodule MvWeb.GroupLive.ShowTest do end end - @moduletag :slow describe "performance" do + @describetag :slow test "member list is loaded efficiently (no N+1 queries)", %{conn: conn} do group = Fixtures.group_fixture() @@ -236,12 +236,31 @@ defmodule MvWeb.GroupLive.ShowTest do ) end) + # Count queries using Telemetry + query_count_agent = Agent.start_link(fn -> 0 end) |> elem(1) + + handler = fn _event, _measurements, _metadata, _config -> + Agent.update(query_count_agent, &(&1 + 1)) + end + + handler_id = "test-query-counter-#{System.unique_integer([:positive])}" + :telemetry.attach(handler_id, [:ash, :query, :start], handler, nil) + {:ok, _view, html} = live(conn, "/groups/#{group.slug}") + final_count = Agent.get(query_count_agent, & &1) + :telemetry.detach(handler_id) + # All members should be displayed Enum.each(members, fn member -> assert html =~ member.first_name or html =~ member.last_name end) + + # Verify query count is reasonable (should avoid N+1 queries) + # Expected: 1 query for group lookup + 1 query for members (with preload) + # Allow some overhead for LiveView setup queries + assert final_count <= 5, + "Expected max 5 queries (group + members preload + LiveView setup), got #{final_count}. This suggests N+1 query problem." end test "slug lookup is efficient (uses unique_slug index)", %{conn: conn} do