Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Replace Kernel.srand with Random.new #837

Merged
merged 6 commits into from

3 participants

@mshytikov

Hello,

The problem:

I have found that when I run my tests using random order Kernel.rand returns unexpected results as for me.

Simplified example just to show a concept:

describe "something" do

   let(:my_rand_value) { rand }

   it("out rand value") { puts my_rand_value }

   it("out rand value") { puts my_rand_value }

end

My expectation is 2 different numbers in output but they are the same.

The reason:
This happens because under the hood Kernel.srand is called with seed from config.

The fix:
In this pull request I have added the test on this situation and also provide code to fix it.

Note: the test looks ugly and I need help to improve it.

@dchelimsky
Owner

Random was introduced in Ruby 1.9, so we need either a different solution for 1.8.7, or a new solution that supports all supported Rubies (1.8.7, 1.9.2, 1.9.3, 2.0.0 iirc).

@mshytikov

Sorry I have missed version compatibility. I have reverted commit with Random.new and added Kernel.srand after ordering to fix the bug. I guess this is simpler.

@myronmarston

I can't repro the example you pasted:

$ bin/rspec spec/foo_spec.rb --format doc --order random

something
0.19455422523244326
  out rand value
0.8001020731961798
  out rand value

Finished in 0.00067 seconds
2 examples, 0 failures

Randomized with seed 23110

It's printing off different values for me.

When I pass an explicit seed based on the prior run, I get the same values as the prior run:

$ bin/rspec spec/foo_spec.rb --format doc --order random --seed 23110

something
0.19455422523244326
  out rand value
0.8001020731961798
  out rand value

Finished in 0.0007 seconds
2 examples, 0 failures

Randomized with seed 23110

$ bin/rspec spec/foo_spec.rb --format doc --order random --seed 23110

something
0.19455422523244326
  out rand value
0.8001020731961798
  out rand value

Finished in 0.00086 seconds
2 examples, 0 failures

Randomized with seed 23110

I would consider this a feature, though: the point of passing --seed is to "lock down" all the randomness to help isolate something that's intermittently failing.

Am I misunderstanding your problem?

@mshytikov

Oh yes, you are right there is mistake in example, sorry for this. Try this one

describe "parent context" do
  let(:my_rand_value) { rand }

  describe "something" do
    it("out rand value") { puts my_rand_value }
  end

  describe "something" do
    it("out rand value") { puts my_rand_value }
  end

end
@myronmarston

Thanks, I can repro the bug using that. As far as the test goes, what about this?

  it 'does not interfere with per-example randomness when running examples in a random order' do
    values = []

    RSpec.configuration.order = :random

    RSpec::Core::ExampleGroup.describe do
      # The bug was only triggered when the examples
      # were in nested contexts; see https://github.com/rspec/rspec-core/pull/837
      context { example { values << rand } }
      context { example { values << rand } }
    end.run

    expect(values.uniq).to have(2).values
  end

let really has no bearing on this behavior. I would put this in spec/rspec/core/example_spec.rb. I find this easier to read than what you currently have. Thoughts?

@mshytikov

@myronmarston I fully agree with you and your code is very good and clean, thank you.

@myronmarston myronmarston merged commit c648330 into rspec:master
@myronmarston

Thanks, @mshytikov!

@myronmarston myronmarston referenced this pull request from a commit
@myronmarston myronmarston Changelog entry for #837.
[ci skip]
cf60482
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 20, 2013
  1. @mshytikov
  2. @mshytikov
  3. @mshytikov

    Revert "replace rand generation with instance of Random"

    mshytikov authored
    This reverts commit 66b14a919f111cf0b8feb1f08485f098e9c14025.
  4. @mshytikov

    reset seed after ordering

    mshytikov authored
  5. @mshytikov

    Revert "added test for random number generation"

    mshytikov authored
    This reverts commit 4b46c9b2d071f2416f627fad8a76b090bc99f8e6.
  6. @mshytikov
This page is out of date. Refresh to see the latest.
View
4 lib/rspec/core/configuration.rb
@@ -871,7 +871,9 @@ def randomize?
# @private
RANDOM_ORDERING = lambda do |list|
Kernel.srand RSpec.configuration.seed
- list.sort_by { Kernel.rand(list.size) }
+ ordering = list.sort_by { Kernel.rand(list.size) }
+ Kernel.srand # reset random generation
+ ordering
end
# Sets a strategy by which to order examples.
View
15 spec/rspec/core/example_spec.rb
@@ -415,4 +415,19 @@ def run_and_capture_reported_message(group)
expect(example.metadata[:execution_result][:run_time]).to be < 0.2
end
end
+
+ it 'does not interfere with per-example randomness when running examples in a random order' do
+ values = []
+
+ RSpec.configuration.order = :random
+
+ RSpec::Core::ExampleGroup.describe do
+ # The bug was only triggered when the examples
+ # were in nested contexts; see https://github.com/rspec/rspec-core/pull/837
+ context { example { values << rand } }
+ context { example { values << rand } }
+ end.run
+
+ expect(values.uniq).to have(2).values
+ end
end
Something went wrong with that request. Please try again.