Skip to content

Fix to hopefully resolve the flakey seed test by ensuring different ids are used #600

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+26 −12
Open

Fix to hopefully resolve the flakey seed test by ensuring different ids are used #600

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1548609
Fix to hopefully resolve the flakey seed test by ensuring different i…
danhalson Oct 1, 2025
6780ff2
Make the comments less verbose
danhalson Oct 1, 2025
721f5b5
Merge branch 'main' into fix-flakey-seed-test
danhalson Oct 2, 2025
6322321
Merge branch 'main' into fix-flakey-seed-test
danhalson Oct 27, 2025
6619284
Ensure the school is also unique
danhalson Oct 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 20 additions & 12 deletions spec/lib/tasks/for_education_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@

require 'rails_helper'
require 'rake'
require 'climate_control'

RSpec.describe 'for_education', type: :task do
let(:creator_id) { '583ba872-b16e-46e1-9f7d-df89d267550d' } # [email protected]
let(:teacher_id) { 'bbb9b8fd-f357-4238-983d-6f87b99bdbb2' } # [email protected]
let(:student_1) { 'e52de409-9210-4e94-b08c-dd11439e07d9' } # student
let(:student_2) { '0d488bec-b10d-46d3-b6f3-4cddf5d90c71' } # student
let(:school_id) { 'e52de409-9210-4e94-b08c-dd11439e07d9' }
let(:creator_id) { 'f83ba872-b16e-46e1-9f7d-df89d267550d' }
let(:teacher_id) { 'ccc9b8fd-f357-4238-983d-6f87b99bdbb2' }
Comment on lines +14 to +15
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardcoded UUIDs for creator_id and teacher_id do not match the actual TEST_USERS values from SeedsHelper (which uses 583ba872-b16e-46e1-9f7d-df89d267550d for jane_doe and bbb9b8fd-f357-4238-983d-6f87b99bdbb2 for john_doe). Consider using the actual SeedsHelper::TEST_USERS values or documenting why different UUIDs are being used for testing.

Suggested change
let(:creator_id) { 'f83ba872-b16e-46e1-9f7d-df89d267550d' }
let(:teacher_id) { 'ccc9b8fd-f357-4238-983d-6f87b99bdbb2' }
let(:creator_id) { '583ba872-b16e-46e1-9f7d-df89d267550d' } # jane_doe from SeedsHelper
let(:teacher_id) { 'bbb9b8fd-f357-4238-983d-6f87b99bdbb2' } # john_doe from SeedsHelper

Copilot uses AI. Check for mistakes.
let(:student_1) { 'e52de409-9210-4e94-b08c-dd11439e07d9' } # jane.smith from SeedsHelper
let(:student_2) { '0d488bec-b10d-46d3-b6f3-4cddf5d90c71' } # john.smith from SeedsHelper
let(:school_id) { 'e52de409-9210-4e94-b08c-dd11439e07d9' } # Use TEST_SCHOOL value so destroy works
Copy link
Contributor

@loiswells97 loiswells97 Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I fully follow why changing the id fixes the flakiness?

Copy link
Contributor Author

@danhalson danhalson Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah good spot...I did this ages ago, but I'm pretty sure it was another attempt to ensure the data between runs doesn't bleed


describe ':destroy_seed_data' do
let(:task) { Rake::Task['for_education:destroy_seed_data'] }
Expand All @@ -24,7 +25,9 @@
end

it 'destroys all seed data' do
task.invoke
ClimateControl.modify SEEDING_CREATOR_ID: creator_id, SEEDING_TEACHER_ID: teacher_id do
task.invoke
end
Comment on lines +34 to +36
Copy link
Contributor

@loiswells97 loiswells97 Oct 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean these values weren't getting picked up before?

Copy link
Contributor Author

@danhalson danhalson Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they were getting picked up, but state was bleeding between tests

Copy link
Contributor

@loiswells97 loiswells97 Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure I understand how this fixes the bleed in that case 🤔

expect(Role.where(user_id: [creator_id, teacher_id, student_1, student_2])).not_to exist
expect(School.where(creator_id:)).not_to exist
expect(ClassStudent.where(student_id: student_1)).not_to exist
Expand All @@ -39,7 +42,9 @@
let(:task) { Rake::Task['for_education:seed_an_unverified_school'] }

it 'creates an unverified school' do
task.invoke
ClimateControl.modify SEEDING_CREATOR_ID: creator_id do
task.invoke
end
expect(School.find_by(creator_id:).verified_at).to be_nil
end
end
Expand All @@ -48,7 +53,9 @@
let(:task) { Rake::Task['for_education:seed_a_verified_school'] }

it 'creates a verified school' do
task.invoke
ClimateControl.modify SEEDING_CREATOR_ID: creator_id do
task.invoke
end
expect(School.find_by(creator_id:).verified_at).to be_truthy
end
end
Expand All @@ -58,8 +65,9 @@
let(:school) { School.find_by(creator_id:) }

before do
Rake::Task['for_education:destroy_seed_data'].invoke
Copy link
Contributor

@loiswells97 loiswells97 Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this no longer needed? Or was it never needed?

Copy link
Contributor Author

@danhalson danhalson Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it shouldn't have been needed, given DatabaseCleaner cleans the db between runs...but I think the concurrency was the prob...using unique ids fixes that

task.invoke
ClimateControl.modify SEEDING_CREATOR_ID: creator_id, SEEDING_TEACHER_ID: teacher_id do
task.invoke
end
end

it 'creates a verified school' do
Expand All @@ -71,8 +79,8 @@
end

it 'adds two lessons to the school' do
lesson = Lesson.where(school_id: school.id)
expect(lesson.length).to eq(2)
lessons = Lesson.where(school_id: school.id)
expect(lessons.count).to eq(2)
end

it 'adds two projects' do
Expand Down