refactor: apply review comments
All checks were successful
continuous-integration/drone/push Build is passing

This commit is contained in:
Simon 2026-01-28 14:42:16 +01:00
parent 050ca4a13c
commit ea3bdcaa65
Signed by: simon
GPG key ID: 40E7A58C4AA1EDB2
5 changed files with 81 additions and 13 deletions

View file

@ -215,7 +215,7 @@ steps:
echo "Postgres did not become available, aborting." echo "Postgres did not become available, aborting."
exit 1 exit 1
- name: test-slow - name: test-all
image: docker.io/library/elixir:1.18.3-otp-27 image: docker.io/library/elixir:1.18.3-otp-27
environment: environment:
MIX_ENV: test MIX_ENV: test
@ -226,5 +226,5 @@ steps:
- mix local.hex --force - mix local.hex --force
# Fetch dependencies # Fetch dependencies
- mix deps.get - mix deps.get
# Run only all tests # Run all tests (including slow/performance tests)
- mix test - mix test

View file

@ -1656,7 +1656,7 @@ end
**Performance Tests:** **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`:** **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 use large datasets (e.g., 50+ records) to test scalability
- Tests that validate query optimization (N+1 prevention, index usage) - 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:** **Running Tests:**
```bash ```bash
@ -1673,7 +1679,7 @@ mix test --exclude slow
# Performance tests only # Performance tests only
mix test --only slow mix test --only slow
# All tests # All tests (including slow tests)
mix test mix test
``` ```
--- ---

View file

@ -172,7 +172,7 @@ mix test
#### CI/CD Integration #### CI/CD Integration
- **Standard CI:** Runs `mix test --exclude slow` for faster feedback loops (~6 minutes) - **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 - **Pre-Merge:** Full test suite (`mix test`) runs before merging to main
- **Manual Execution:** Can be triggered via Drone CLI or Web UI - **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 - Execution time: ~6 minutes
- Command: `mix test --exclude slow` or `just test-fast` - Command: `mix test --exclude slow` or `just test-fast`
**Slow/Nightly Tests:** **Slow Tests:**
- Tests tagged with `@tag :slow` (25 tests) - Tests tagged with `@tag :slow` or `@describetag :slow` (25 tests)
- Low risk, >1 second execution time - 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 - Execution time: ~1.3 minutes
- Command: `mix test --only slow` or `just test-slow` - 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:** **All Tests:**
- Includes both fast and slow 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. 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?** **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?** **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. 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"?** **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`?** **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. 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.

View file

@ -114,17 +114,36 @@ defmodule MvWeb.GroupLive.IndexTest do
end end
end end
@moduletag :slow
describe "performance" do describe "performance" do
@describetag :slow
test "page loads efficiently with many groups", %{conn: conn} do test "page loads efficiently with many groups", %{conn: conn} do
# Create multiple groups # Create multiple groups
Enum.each(1..10, fn _ -> Fixtures.group_fixture() end) 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 # Should load without N+1 queries
{:ok, _view, html} = live(conn, "/groups") {:ok, _view, html} = live(conn, "/groups")
final_count = Agent.get(query_count_agent, & &1)
:telemetry.detach(handler_id)
# Verify all groups are displayed # Verify all groups are displayed
assert html =~ gettext("Groups") 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 end
test "member count is loaded efficiently via calculation", %{conn: conn} do test "member count is loaded efficiently via calculation", %{conn: conn} do
@ -143,10 +162,29 @@ defmodule MvWeb.GroupLive.IndexTest do
actor: system_actor 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") {: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) # Member count should be displayed (should be 2)
assert html =~ "2" or html =~ gettext("Members") or html =~ "Mitglieder" 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 end
end end

View file

@ -219,8 +219,8 @@ defmodule MvWeb.GroupLive.ShowTest do
end end
end end
@moduletag :slow
describe "performance" do describe "performance" do
@describetag :slow
test "member list is loaded efficiently (no N+1 queries)", %{conn: conn} do test "member list is loaded efficiently (no N+1 queries)", %{conn: conn} do
group = Fixtures.group_fixture() group = Fixtures.group_fixture()
@ -236,12 +236,31 @@ defmodule MvWeb.GroupLive.ShowTest do
) )
end) 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}") {: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 # All members should be displayed
Enum.each(members, fn member -> Enum.each(members, fn member ->
assert html =~ member.first_name or html =~ member.last_name assert html =~ member.first_name or html =~ member.last_name
end) 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 end
test "slug lookup is efficient (uses unique_slug index)", %{conn: conn} do test "slug lookup is efficient (uses unique_slug index)", %{conn: conn} do