Skip to content
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.

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

Adds rspec.config randomize spec order #8461

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

filipefurtad0
Copy link
Contributor

What? Why?

Sometimes specs fail because they are ran in a specific order. This PR adds a randomization config to Rspec, which also outputs which seed was used - this was found here. This can be useful in reproducing the error locally with the bisect option.

Adds a randomization config to Rspec

What should we test?

Green build.
After each build is complete a line stating which seed was used for the build should appear as an output, something like this:
Randomized with seed 61761

Release notes

Changelog Category: Technical changes

Dependencies

Documentation updates

@mkllnk
Copy link
Member

mkllnk commented Nov 8, 2021

Yay, you found a random test failure this way! The error message looks familiar. Last time, the problem was that a spec set up Stripe and part of that setup leaked into other specs which then failed to load Stripe code. Here was the solution: #8361

@filipefurtad0
Copy link
Contributor Author

I have to say, I'm not sure this approach is working. The build fails consistently, with different seeds. I don't think this is usable in master right now, there must be some fine tuning around settings, not sure.

For documentations sake, adding below an attempt to debug one of the failures locally, using bisect, with:
bundle exec rspec --seed 13856 --bisect spec/system/admin/

Bisect started using options: "--seed 13856 spec/system/admin/"
Running suite to find failures... (31 minutes 5 seconds)
Starting bisect with 26 failing examples and 344 non-failing examples.
Checking that failure(s) are order-dependent... failure appears to be order-dependent

Round 1: bisecting over non-failing examples 1-344 .. ignoring examples 173-344 (28 minutes 43 seconds)
Round 2: bisecting over non-failing examples 1-172 . ignoring examples 1-86 (14 minutes 2 seconds)
Round 3: bisecting over non-failing examples 87-172 .. ignoring examples 130-172 (19 minutes 13 seconds)
Round 4: bisecting over non-failing examples 87-129 . ignoring examples 87-108 (7 minutes 12 seconds)
Round 5: bisecting over non-failing examples 109-129 . ignoring examples 109-119 (6 minutes 44 seconds)
Round 6: bisecting over non-failing examples 120-129 . ignoring examples 120-124 (6 minutes 19 seconds)
Round 7: bisecting over non-failing examples 125-129 .. ignoring examples 128-129 (11 minutes 18 seconds)
Round 8: bisecting over non-failing examples 125-127 .. ignoring example 127 (11 minutes 16 seconds)
Round 9: bisecting over non-failing examples 125-126 .. ignoring example 126 (11 minutes 11 seconds)
Bisect complete! Reduced necessary non-failing examples from 344 to 1 in 121 minutes 2 seconds.

The minimal reproduction command is:
  rspec ./spec/system/admin/authentication_spec.rb[1:2] ./spec/system/admin/bulk_order_management_spec.rb[1:3:7:1,1:3:7:2,1:3:7:3] ./spec/system/admin/enterprises_spec.rb[1:3] ./spec/system/admin/order_cycles/simple_spec.rb[1:5:4] ./spec/system/admin/order_print_ticket_spec.rb[1:1:1:1] ./spec/system/admin/order_spec.rb[1:7:1] ./spec/system/admin/payments_stripe_spec.rb[1:2:3:1] ./spec/system/admin/product_import_spec.rb[1:1:1,1:1:2,1:1:3,1:1:4,1:1:5,1:1:6,1:1:7,1:1:8,1:1:9,1:1:10,1:1:11,1:1:12,1:1:13,1:2:1,1:2:4,1:3:1,1:4:1:1] ./spec/system/admin/tag_rules_spec.rb[1:2:1] --seed 13856

@filipefurtad0 filipefurtad0 marked this pull request as draft November 9, 2021 09:04
@mkllnk
Copy link
Member

mkllnk commented Nov 9, 2021

The build fails consistently, with different seeds.

Yikes. That demonstrates that we have a lot of order dependent failures which need sorting out. I agree that we can't merge this yet. We need to fix all the failures first. And that could take a while because we will discover them randomly. Or is there a way to bisect the whole test suite with all possible seeds? Maybe something to run on CI...

You could generate a lot of commits with different seeds and push them on separate branches so that CI shows you which ones fail.

@filipefurtad0 filipefurtad0 marked this pull request as ready for review July 25, 2022 15:02
@filipefurtad0 filipefurtad0 marked this pull request as draft July 25, 2022 15:09
@filipefurtad0 filipefurtad0 marked this pull request as ready for review July 25, 2022 15:34
@filipefurtad0 filipefurtad0 force-pushed the config_rspec_order branch 2 times, most recently from 6002333 to 6339865 Compare October 31, 2022 17:32
@filipefurtad0
Copy link
Contributor Author

The randomization of specs seems to play a role in pass rate, when the build runs may parallel jobs - like we're currently attempting to make use of out current plan GH-Actions (max. 60 parallel jobs). So I felt it is the right time to revive this previous PR.

@filipefurtad0
Copy link
Contributor Author

Hum, interesting results:

  • build run #1 - only this one failed:
rspec ./spec/models/customer_spec.rb:26 # Customer update shipping address updates the shipping address

Randomized with seed 57907
  • build run #2 - only this one failed:

Failed examples:

rspec ./spec/helpers/shop_helper_spec.rb:14 # ShopHelper shop_tabs distributor with groups should return the groups tab
rspec ./spec/helpers/shop_helper_spec.rb:38 # ShopHelper shop_tabs distributor with shopfront message should show the home tab

Randomized with seed 47836
  • build run #3:
rspec ./spec/models/customer_spec.rb:26 # Customer update shipping address updates the shipping address

Randomized with seed 49323

(+ a flaky spec on bulk order managenent)

@mkllnk
Copy link
Member

mkllnk commented Nov 2, 2022

Yes, the randomisation can help discover more flaky specs. But we should probably try to stabilise the current build before we probe for more flakiness.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: All the things 💤
Development

Successfully merging this pull request may close these issues.

2 participants