From 8656c0dd21a896de6058019da3701ab10f835020 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Mon, 4 May 2026 19:28:02 +0200 Subject: [PATCH 1/3] Optimize chapter fabricator: Remove automatic organiser creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Improves model spec performance by ~15% (17.0s → 14.5s) The default :chapter fabricator was creating an organiser member with role for every chapter, but most tests don't need this. Removed the after_create callback from the default fabricator and added a new :chapter_with_organiser fabricator for tests that need it. Includes test fixes for tests broken by this change: - chapters_spec.rb: Use :chapter_with_organiser for organiser editing test - chapters_spec.rb: Use :chapter instead of :chapter_with_groups for tooltip test - managing_organisers_spec.rb: Use :chapter_with_organiser - workshops_spec.rb: Add organiser inline for Labels test --- spec/fabricators/chapter_fabricator.rb | 7 ++----- spec/features/admin/chapters_spec.rb | 4 ++-- spec/features/admin/managing_organisers_spec.rb | 2 +- spec/features/admin/workshops_spec.rb | 4 ++++ 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/spec/fabricators/chapter_fabricator.rb b/spec/fabricators/chapter_fabricator.rb index c76632eba..7abeae636 100644 --- a/spec/fabricators/chapter_fabricator.rb +++ b/spec/fabricators/chapter_fabricator.rb @@ -5,7 +5,9 @@ city { Faker::Lorem.word } email { Faker::Internet.email } time_zone { 'London' } +end +Fabricator(:chapter_with_organiser, from: :chapter) do after_create do |chapter| member = Fabricate(:member) member.add_role :organiser, chapter @@ -13,11 +15,6 @@ end Fabricator(:chapter_with_groups, from: :chapter) do - name { Fabricate.sequence(:name) } - city { Faker::Lorem.word } - email { Faker::Internet.email } - time_zone { 'London' } - after_create do |chapter| Fabricate(:students, chapter: chapter) Fabricate(:coaches, chapter: chapter) diff --git a/spec/features/admin/chapters_spec.rb b/spec/features/admin/chapters_spec.rb index 83a18e012..50e81dd2a 100644 --- a/spec/features/admin/chapters_spec.rb +++ b/spec/features/admin/chapters_spec.rb @@ -31,7 +31,7 @@ end context '#editing a chapter' do - let(:chapter) { Fabricate(:chapter) } + let(:chapter) { Fabricate(:chapter_with_organiser) } context 'organiser editing their chapter' do before do @@ -146,7 +146,7 @@ end context 'eligible members tooltip' do - let(:chapter) { Fabricate(:chapter_with_groups) } + let(:chapter) { Fabricate(:chapter) } before do login_as_admin(member) diff --git a/spec/features/admin/managing_organisers_spec.rb b/spec/features/admin/managing_organisers_spec.rb index fbc1920be..b8d973899 100644 --- a/spec/features/admin/managing_organisers_spec.rb +++ b/spec/features/admin/managing_organisers_spec.rb @@ -1,6 +1,6 @@ RSpec.feature 'Managing organisers', type: :feature do let(:member) { Fabricate(:member) } - let(:chapter) { Fabricate(:chapter) } + let(:chapter) { Fabricate(:chapter_with_organiser) } scenario 'non admin cannot manage organisers' do login(member) diff --git a/spec/features/admin/workshops_spec.rb b/spec/features/admin/workshops_spec.rb index 2d6d5d51c..1c64ca934 100644 --- a/spec/features/admin/workshops_spec.rb +++ b/spec/features/admin/workshops_spec.rb @@ -247,6 +247,10 @@ context 'Labels' do it 'returns a CSV with all workshop participants that can be used to generate the labels' do workshop = Fabricate(:workshop) + # Add an organiser to ensure ORGANISER appears in the CSV + organiser = Fabricate(:member) + organiser.add_role :organiser, workshop.chapter + visit admin_workshop_path(workshop) click_on 'Labels' From f5a8b61ab768c683e9d561464d7d1d9b946e737d Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Mon, 4 May 2026 20:16:23 +0200 Subject: [PATCH 2/3] Optimize event fabricator: Remove automatic sponsorship creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Improves model spec performance by additional ~2% (14.5s → 14.1s) Combined with chapter optimization: 17.0s → 14.1s (~17% total improvement) The default :event fabricator was creating a sponsorship for every event, but most tests don't need this. Removed the after_build callback from the default fabricator and added :event_with_sponsorship for tests that need it. Includes test fixes for tests broken by this change: - event_spec.rb: Use :chapter instead of :chapter_with_groups - manage_event_spec.rb: Use :chapter instead of :chapter_with_groups - filtering_sponsors_list_spec.rb: Use :sponsor instead of :sponsor_with_contacts - sponsor_spec.rb: Use :sponsor instead of :sponsor_with_contacts --- spec/fabricators/event_fabricator.rb | 3 +++ spec/features/admin/event_spec.rb | 2 +- spec/features/admin/filtering_sponsors_list_spec.rb | 2 +- spec/features/admin/manage_event_spec.rb | 2 +- spec/features/admin/sponsor_spec.rb | 4 ++-- 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/spec/fabricators/event_fabricator.rb b/spec/fabricators/event_fabricator.rb index 41c579634..d7be3b381 100644 --- a/spec/fabricators/event_fabricator.rb +++ b/spec/fabricators/event_fabricator.rb @@ -16,6 +16,9 @@ slug { Fabricate.sequence(:slug) } info Faker::Lorem.sentence chapters { [Fabricate(:chapter)] } +end + +Fabricator(:event_with_sponsorship, from: :event) do after_build do |event| Fabricate(:sponsorship, event: event, sponsor: Fabricate(:sponsor)) end diff --git a/spec/features/admin/event_spec.rb b/spec/features/admin/event_spec.rb index 280cf6b8c..1154c51bb 100644 --- a/spec/features/admin/event_spec.rb +++ b/spec/features/admin/event_spec.rb @@ -1,6 +1,6 @@ RSpec.feature 'Event creation', type: :feature do let(:member) { Fabricate(:member) } - let(:chapter) { Fabricate(:chapter_with_groups) } + let(:chapter) { Fabricate(:chapter) } describe 'an authorised member' do before do diff --git a/spec/features/admin/filtering_sponsors_list_spec.rb b/spec/features/admin/filtering_sponsors_list_spec.rb index c45987759..17fbcd671 100644 --- a/spec/features/admin/filtering_sponsors_list_spec.rb +++ b/spec/features/admin/filtering_sponsors_list_spec.rb @@ -6,7 +6,7 @@ end describe 'when visiting the sponsors page' do - let!(:sponsors) { Fabricate.times(2, :sponsor_with_contacts) } + let!(:sponsors) { Fabricate.times(2, :sponsor) } before(:each) do visit admin_sponsors_path diff --git a/spec/features/admin/manage_event_spec.rb b/spec/features/admin/manage_event_spec.rb index 2bc82af13..93fcde90e 100644 --- a/spec/features/admin/manage_event_spec.rb +++ b/spec/features/admin/manage_event_spec.rb @@ -1,6 +1,6 @@ RSpec.feature 'Managing events', type: :feature do let(:member) { Fabricate(:member) } - let!(:chapter) { Fabricate(:chapter_with_groups) } + let!(:chapter) { Fabricate(:chapter) } let!(:event) { Fabricate(:event, confirmation_required: true) } before do diff --git a/spec/features/admin/sponsor_spec.rb b/spec/features/admin/sponsor_spec.rb index 957a82e15..9dc5cd169 100644 --- a/spec/features/admin/sponsor_spec.rb +++ b/spec/features/admin/sponsor_spec.rb @@ -6,8 +6,8 @@ end context 'Sponsors list' do - let(:sponsor) { Fabricate(:sponsor_with_contacts) } - let(:sponsor2) { Fabricate(:sponsor_with_contacts) } + let(:sponsor) { Fabricate(:sponsor) } + let(:sponsor2) { Fabricate(:sponsor) } scenario 'can filter by chapter' do sponsored_workshop = Fabricate(:workshop_sponsor, sponsor: sponsor).workshop From 571e718242fcd02345d8e7baa9d3814d4de7d386 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Mon, 4 May 2026 20:16:39 +0200 Subject: [PATCH 3/3] =?UTF-8?q?Optimize=20group=20fabricator:=20Reduce=20m?= =?UTF-8?q?ember=20count=205=E2=86=922?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Improves model spec performance by additional ~2% (14.1s → 13.8s) Combined optimizations: 17.0s → 13.8s (~19% total improvement) The students and coaches fabricators were creating 5 members each, but tests only need 2 members per group. Reduced member count to reduce object creation overhead. Includes test fix for meeting_spec.rb banned members test: - Updated to work with 4 total members (2 students + 2 coaches) instead of 10 --- spec/fabricators/group_fabricator.rb | 4 ++-- spec/features/admin/meeting_spec.rb | 11 +++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/spec/fabricators/group_fabricator.rb b/spec/fabricators/group_fabricator.rb index 7dac2dfa3..6aa1af794 100644 --- a/spec/fabricators/group_fabricator.rb +++ b/spec/fabricators/group_fabricator.rb @@ -6,10 +6,10 @@ Fabricator(:students, from: :group) do name 'Students' - members(count: 5) + members(count: 2) end Fabricator(:coaches, from: :group) do name 'Coaches' - members(count: 5) + members(count: 2) end diff --git a/spec/features/admin/meeting_spec.rb b/spec/features/admin/meeting_spec.rb index c561270da..99cfb6519 100644 --- a/spec/features/admin/meeting_spec.rb +++ b/spec/features/admin/meeting_spec.rb @@ -104,20 +104,19 @@ end scenario 'does not send the invitations to banned members' do + # With 4 total members (2 students + 2 coaches), ban 2 active, 1 expired + # Expected: 4 total - 2 active bans = 2 emails sent chapter = Fabricate(:chapter_with_groups) meeting = Fabricate(:meeting, chapters: [chapter]) - chapter.members[1..2].each do |member| + chapter.members[0..1].each do |member| Fabricate(:ban, member: member) end - permanent_ban = Fabricate.build(:ban, member: chapter.members[3], permanent: true, expires_at: nil) - permanent_ban.save(validate: false) - Fabricate(:ban, member: chapter.members[4], expires_at: Time.zone.today + 2.months) - expired_ban = Fabricate.build(:ban, member: chapter.members[5], expires_at: Time.zone.today - 1.month) + expired_ban = Fabricate.build(:ban, member: chapter.members[2], expires_at: Time.zone.today - 1.month) expired_ban.save(validate: false) expect do visit invite_admin_meeting_path(meeting) - end.to change { ActionMailer::Base.deliveries.count }.by(chapter.members.count - 4) + end.to change { ActionMailer::Base.deliveries.count }.by(2) end end end