Skip to content

Fix flakey IPP sample data rake spec #7363

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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aduth merged 2 commits into from
Nov 18, 2022
Merged

Fix flakey IPP sample data rake spec #7363

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
02ea837
Fix flakey IPP sample data rake spec
aduth Nov 18, 2022
1de8106
Call / stub Kernel.sleep
aduth Nov 18, 2022
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
8 changes: 7 additions & 1 deletion lib/tasks/dev.rake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
class SampleDataCreator
def self.delay(seconds)
sleep(seconds)
end
end

namespace :dev do
desc 'Sample data for local development environment'
task prime: :environment do
Expand Down Expand Up @@ -146,7 +152,7 @@ namespace :dev do
else
success = true
end
sleep(usps_request_delay_ms / 1000.0) if usps_request_delay_ms
SampleDataCreator.delay(usps_request_delay_ms / 1000.0) if usps_request_delay_ms
Comment thread
aduth marked this conversation as resolved.
Outdated
end
else
enrollment = InPersonEnrollment.create!(
Expand Down
4 changes: 2 additions & 2 deletions spec/lib/tasks/dev_rake_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@
stub_request_token
stub_request_enroll

expect_any_instance_of(Object).to receive(:sleep).exactly(10).times.with(0.2)

@zachmargolis zachmargolis Nov 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oh I didn't even see we had expect_any_instance_of in here! Yeah no wonder this spec was flaking :shakes-fist:!

@eileen-nava eileen-nava Nov 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For my own curiosity, is it generally bad practice to use expect_any_instance_of in rspec? Or is it problematic when it's used in this specific way with sleep? (I think it's the latter, but want to check I'm not missing anything.)

@zachmargolis zachmargolis Nov 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

personally I think that expect_any_instance_of or allow_any_instance_of are kind of antipatterns. when stubbing, I think it's crucial to know which instance you're stubbing on, since there can be a lot of different things happening (like with a common method like sleep)

However, the _any_instance_of methods are useful in situations where we don't have access to the instances (acceptance specs, or classes that don't expose clear methods to stub), or cases like this where we don't know what instance the rake task is being called on.

And I think the issue here is that sleep is a fairly common method, and it's mixed in to every object via Kernel so it was just an unforunate combination of things that lead to this flakiness.

@eileen-nava eileen-nava Nov 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Got it, thank you.

expect(SampleDataCreator).to receive(:delay).exactly(10).times.with(0.2)

Rake::Task['dev:random_in_person_users'].invoke
end
Expand Down Expand Up @@ -436,7 +436,7 @@
).times(1)
expect(UspsInPersonProofing::EnrollmentHelper).
to receive(:schedule_in_person_enrollment).and_call_original.exactly(3).times
expect_any_instance_of(Object).to receive(:sleep).exactly(3).times.with(0.2)
expect(SampleDataCreator).to receive(:delay).exactly(3).times.with(0.2)

Rake::Task['dev:random_in_person_users'].invoke
end
Expand Down