Use random order by default

[santib]

Hi, in rubygems/rubygems#5296 I proposed using --order random on gem creation by default to help prevent introducing order dependant tests. But it might not make sense to have bundler do that if it's not a default on rspec.

So this PR is actually to ask what do you think about it. Have you discussed this in the past? Assuming that this is a best practice in the community, wouldn't it help everyone to have --order random by default?

[pirj]

RSpec project initializer (rspec --init) does suggest the random order, but it doesn't insist and implies that users would manually opt-in to get the benefits of a good initial experience with RSpec:

# The settings below are suggested to provide a good initial experience
# with RSpec, but feel free to customize to your heart's content.
=begin
  config.order = :random

.rspec is not the best place to define order due to reduced discoverability compared to spec/spec_helper.rb, especially for newly created gems.

I think it makes sense for RubyGems to at least provide the same spec/spec_helper.rb that rspec --init would generate. Several reasons:

1. Give gem authors an option to make an informed decision by keeping all necessary comments in spec_helper.rb.
2. Not leave out rspec-mocks (that to my best memory won't work out of the box without config.mock_with :rspec

Would you accept a PR updating the RubyGems spec/spec_helper.rb template to match the one we offer with rspec --init, @deivid-rodriguez? 👋

Note. Some options that presently appear in RSpec 3.x, like e.g. config.shared_context_metadata_behavior and expectations.include_chain_clauses_in_custom_matcher_descriptions will disappear in RSpec 4. This is completely fine for RubyGems generator, though, as it pins RSpec to version 3. Once it's prime time for RSpec 4, the spec_helper.rb template can be updated along with bumping the pinned version to 4.

[deivid-rodriguez]

That makes sense to me, yeah!

[pirj]

@santib Thanks for bringing this problem up.
I'll close this, effectively pinging the PR back to RubyGems repository.

Whether to trust our users' or force-feed them best practices by uncommenting those lines is a different topic. To me it's an improvement to give them choice and unburden from looking up all those options in other projects or on GitHub.

[deivid-rodriguez]

@pirj My question is, why does RSpec not provide this "good initial experience" by default?

[pirj]

@deivid-rodriguez We trust our users to be able to make a good choice if we give them an easy way to do so?

We prefer not to push anything to our users, as there certainly are cases when the opposite defaults matter. For example, Puppet infra specs:

we sometimes count the setup time in minutes

See the official RSpec style guide ticket discussion:

could be used to silence discussion

What we've already seen this concern in reality 😄

The proof that there's no consensus is that RSpec doesn't do this by default

It would not be effective to "count the setup time in minutes" if Puppet specs would go with order: :random that we would ship as a default, wouldn't it?

[deivid-rodriguez]

Sorry, I couldn't understand the point of your links. You certainly provide some defaults, but it feels weird to me that you recommend different settings than your own defaults in your generated helper.

I believe it's great that you provide a self-contained helper with a lot of comments explaining the different options that users can tweak. But if feels weird to me to recommend some settings (specifically with the " The settings below are suggested to provide a good initial experience" wording) that you don't enable by default.

[JonRowe]

FWIW I'd support changing to random as the default in RSpec 4.

[pirj]

Would you like to contribute such a change to 4-0-dev branch, @santib ?
I guess it should affect:

• project initializer's spec_helper template
• configuration

An additional pull request might be needed to warn users who don't explicitly set the global order that it has changed (see e.g. #2880), to main branch.

[santib]

Sure @pirj, I will do it

[santib]

@pirj I just pushed a first version, but I have some questions:

1. I removed the config.order config from the spec/spec_helper.rb template because it is under a "suggested settings" section, so changing it to config.order :defined wouldn't make sense even for reference. Is that ok?
2. Should we also make some change to the Kernel.srand config.seed setting?
3. I took the simplest approach to fix tests, which is going back to a :defined order in some cases, is that fine?
4. I ran the tests N times to avoid introducing intermittent tests, but just in case be aware that it could happen

When things are ok I can work on the deprecation warning and maybe we could also say it in some doc?

[JonRowe]

For our spec suite changes, you should be able to tag the specs / groups :order => :defined where necessary.

[santib]

For our spec suite changes, you should be able to tag the specs / groups :order => :defined where necessary.

I tried this but it didn't work 🤔 Any ideas?

[pirj]

I tried this but it didn't work

Can you please push the failing changes?

Any luck with pushing your changes to RubyGems?

[JonRowe]

Can you rebase this or bump commit it to see if we can get this to build? Github actions seems to be MIA

[santib]

@JonRowe CI is waiting for approval

[JonRowe]

Merging because the build failurs are not related to this PR, and 4-0-dev is a dev branch.