rubyforgood / human-essentials

Human Essentials is an inventory management system for diaper, incontinence, and period-supply banks. It supports them in distributing to partners, tracking inventory, and reporting stats and analytics.
https://humanessentials.app
MIT License
446 stars 468 forks source link

[BUG]: Flaky spec regarding CalendarService in distribution_request_spec.rb #3127

Closed edwinthinks closed 2 years ago

edwinthinks commented 2 years ago

Is there an existing issue for this?

Current Behavior

Test suite fails randomly with:

  1) Distributions While not signed in GET #calendar with a correct hash id should render the calendar
     Failure/Error: expect(CalendarService).to have_received(:calendar).with(@organization.id)

       (CalendarService).calendar(1)
           expected: 1 time with arguments: (1)
           received: 0 times
     # ./spec/requests/distributions_requests_spec.rb:351:in `block (5 levels) in <top (required)>'
     # ./spec/spec_helper.rb:97:in `block (2 levels) in <top (required)>'

Expected Behavior

It should not fail randomly.

Steps To Reproduce

This often occurs in CI but I cannot reproduce it locally.

Environment

N/A

Criteria for Completion

Anything else?

No response

Code of Conduct

edwinthinks commented 2 years ago

@dorner I noticed that this spec was added by you and was wondering if we might be able to refactor it to be resilient? I suspect that it may have to do with stubbing the secret? https://github.com/rubyforgood/human-essentials/blame/3c5a37174317d73c80d139405dc15a436f54a048/spec/requests/distributions_requests_spec.rb#L12

dorner commented 2 years ago

@edwinthinks have you tried reproducing by running the suite with the same files and the same seed? I can't see how the stub would result in flakiness since it's always stubbing it with the same value.

edwinthinks commented 2 years ago

@dorner I haven't, can you tell me how I can do that? There are a few tests out there with the failure on CalendarService. I also am not sure how stubbing would fix this.

dorner commented 2 years ago

Yep - so whenever a spec suite runs, it prints out the seed it's running with. All you have to do is 1) copy/paste the list of tests, which knapsack should print out, and 2) run rspec with --seed {the seed you copied} {the list of tests}.

edwinthinks commented 2 years ago

@dorner thanks! I'll give it a shot and see what I discover. Here is the output for reference of a failed spc:

rspec ./spec/requests/distributions_requests_spec.rb:351 # Distributions While not signed in GET #calendar with a correct hash id should render the calendar

Randomized with seed 10515

Stopped processing SimpleCov as a previous error not related to SimpleCov has been detected
Coverage report generated for RSpec to /home/runner/work/human-essentials/human-essentials/coverage. 2877 / 5097 LOC (56.44%) covered.
/opt/hostedtoolcache/Ruby/3.1.2/x64/bin/ruby -I/home/runner/work/human-essentials/human-essentials/vendor/bundle/ruby/3.1.0/gems/rspec-core-3.10.2/lib:/home/runner/work/human-essentials/human-essentials/vendor/bundle/ruby/3.1.0/gems/rspec-support-3.10.3/lib /home/runner/work/human-essentials/human-essentials/vendor/bundle/ruby/3.1.0/gems/rspec-core-3.10.2/exe/rspec  --default-path spec "spec/system/donation_system_spec.rb" "spec/system/admin/base_items_system_spec.rb" "spec/system/admin/account_requests_system_spec.rb" "spec/system/sign_in_system_spec.rb" "spec/system/account_system_spec.rb" "spec/requests/distributions_requests_spec.rb" "spec/system/dashboard_system_spec.rb[1:2:4:1]" "spec/system/dashboard_system_spec.rb[1:2:6:10:1:1]" "spec/system/dashboard_system_spec.rb[1:2:6:9:1:1]" "spec/system/dashboard_system_spec.rb[1:2:3:4:1:1]" "spec/requests/organization_requests_spec.rb" "spec/system/dashboard_system_spec.rb[1:2:5:4:1:1]" "spec/system/dashboard_system_spec.rb[1:2:4:4:1:1]" "spec/system/dashboard_system_spec.rb[1:2:3:3:1:1]" "spec/system/dashboard_system_spec.rb[1:2:7:1]" "spec/system/layout_system_spec.rb" "spec/requests/dashboard_requests_spec.rb" "spec/system/partners/helps_system_spec.rb" "spec/requests/partners/family_requests_controller_spec.rb" "spec/requests/users_requests_spec.rb" "spec/requests/partners/user_requests_spec.rb" 
dorner commented 2 years ago

OK - I think the issue here is being masked by rspec-retry. It seems like it always fails the first time, but subsequent retries usually make it pass. Not sure why yet, but it does seem likely that the secret part is to blame. Will dive in a bit more.

dorner commented 2 years ago

I think this should work! https://github.com/rubyforgood/human-essentials/pull/3140

edwinthinks commented 2 years ago

@dorner good catch. Yeh let me know what you find out. At a high-level it looks like the code written should work, and I am not sure how it would fail like that.