1
\$\begingroup\$

So, chance, huh? Randomness.

I'm looking into how I can test a class whose sole responsibility is to return weighted random results, that is, where there is a set probability of returning true.

The trouble is, random being what it is, sometimes the result doesn't reflect the initial probability.

Here's my attempt, which passes the specs most of the time.

chance.rb

class Chance

  include Singleton

  def self.percent(amount)
    (rand(100) + 1) <= amount
  end

  # x in y chance of being true
  def self.proportion(x, y)
    rand(y) <= x
  end

end

chance_spec.rb

describe Chance do

  context 'percent' do

    it 'should return true 50% of the time' do
      results = 100.times.collect{ Chance.percent(50) }
      results.select{|x|x}.count.should be_within(10).of(50)
    end

    it 'should return true 75% of the time' do
      results = 100.times.collect{ Chance.percent(75) }
      results.select{|x|x}.count.should be_within(10).of(75)
    end

    it 'should return true 25% of the time' do
      results = 100.times.collect{ Chance.percent(25) }
      results.select{|x|x}.count.should be_within(10).of(25)
    end

    it 'should return true 100% of the time' do
      results = 100.times.collect{ Chance.percent(100) }
      results.select{|x|x}.count.should eq(100)
    end

    it 'should return true 0% of the time' do
      results = 100.times.collect{ Chance.percent(0) }
      results.select{|x|x}.count.should eq(0)
    end

  end

  context 'proportion' do

    it 'should return true 6 in 12 times' do
      results = 12.times.collect{ Chance.proportion(6, 12) }
      results.select{|x|x}.count.should be_within(2).of(6)
    end

    it 'should return true 2 in 6 times' do
      results = 6.times.collect{ Chance.proportion(2, 6) }
      results.select{|x|x}.count.should be_within(1).of(2)
    end

  end

end

Any clues?

\$\endgroup\$
2
  • 2
    \$\begingroup\$ My first instinct would be to increase the test by 2 orders of magnitude to reduce random fluctuation (the more test the closer to real probability) \$\endgroup\$
    – Caridorc
    Commented Apr 8, 2017 at 21:18
  • \$\begingroup\$ this will be a maximum likelihood estimation problem: this looks promising - petertessin.com/MaxLik.pdf \$\endgroup\$
    – BenKoshy
    Commented Apr 9, 2017 at 1:32

2 Answers 2

1
\$\begingroup\$

A few notes on your current code:

  • Your test case descriptions don't make a lot of sense - try reading them out loud: It should return true 50% of the time... and 75% of the time... and 25% of the time... etc.. Wait, which is it? You're omitting the argument in the description, so the descriptions become confusing and/or meaningless.

  • You're adding 1 in the percent method, but not in the proportion method. So a zero-in-1 chance always works out to true! And even a 0 in 1,000,000 is still a chance! Probably not what you want. For both methods I'd suggest using a range argument instead.

  • You're including Singleton for no reason, since you define your methods as class/static methods. So there's no point in instantiating the class - as a singleton or otherwise.

  • What happens if you pass floating point or negative numbers? That's an exercise for the reader.

  • Your results.select{|x|x}.count can be simplified to just results.count(&:itself)

  • Let one method call the other: percent could just call proportion(amount, 100). That way you can focus on testing proportion as that's the only one that actually doesn't anything random. And you'd more easily avoid/fix the bug mentioned above, since it'd only exist in one method, not two.

But as for testing randomness in general: In this case, I'd say don't bother. Because you can't really test it. Your current tests are fuzzy (±10 within target) just so they'll pass more reliably. But that's changing the tests to suit the output, rather than the other way around.

What you want are tests that are strictly pass/fail. This you can do for things like it "raises an ArgumentError for negative arguments" but not so much for the normal operation of the methods.

What it comes down to is whether you trust Ruby to:

  • do basic arithmetic and comparison
  • reliably generate pseudo random numbers (here's the C source for Ruby's random, in case you're interested)

If you trust that, well, testing doesn't make much sense anymore.

Instead spend the time reviewing your code throughly. As seen above, your tests didn't cover the zero-in-N chance bug, and you didn't catch it either. For 2 single-line methods, you wrote an order of magnitude more test code - that effort seems misplaced, when the tests are fudged to pass and the code is buggy.

If you still insist on writing a test, then write a single test case that takes maybe a million samples for 50% or 1-in-2, and checks whether the result is within a narrower tolerance. But I'd run this test by itself a dozen times, but disable it when running the full suite (e.g. tag it so it can be included/excluded easily). It may still fail because it is still random, but that's no reason to fail the entire suite if you've tested thoroughly beforehand. (Edit: For a small project, it's fine to run this test every time, but if your code is on a CI server or you have automation relying on the result of the tests - e.g. can't commit/push to SCM without green tests - it's a pain to have all that at the mercy of randomness.)

Meanwhile you can still do single-sample tests for outliers: 0% chance should be false, 100% should be true, and 0-in-1 should be false, 1-in-1 should be true. Those can be run with the rest of the suite everytime - and should they suddenly, randomly fail, well, something's definitely wrong so start debugging.

\$\endgroup\$
6
  • \$\begingroup\$ Wow! I'd never seen the itself method before. \$\endgroup\$
    – AJFaraday
    Commented Apr 8, 2017 at 21:38
  • \$\begingroup\$ @AJFaraday It's a recent'ish addition (Ruby 2.x-something, I think). But it's a welcome one to be sure! \$\endgroup\$
    – Flambino
    Commented Apr 8, 2017 at 21:39
  • \$\begingroup\$ There is some math to use (I am not sure which) that will give tests that fail arbitrarly rarely (like 1 time in 10 million tries or even less) a test like that could be included in the test suit with no problem in my opinion \$\endgroup\$
    – Caridorc
    Commented Apr 8, 2017 at 21:45
  • \$\begingroup\$ @Flambino Just incidentally, I did find a solution, involving seeding the random. It's probably not ideal, but it is, at least, consistent. \$\endgroup\$
    – AJFaraday
    Commented Apr 8, 2017 at 21:47
  • 1
    \$\begingroup\$ @Caridorc it could. Really depends on how you use your tests. If you're just running them yourself, it's probably fine. But if a spurious failure causes all sorts of automated red flags on some CI server, it can be a hassle \$\endgroup\$
    – Flambino
    Commented Apr 8, 2017 at 21:49
0
\$\begingroup\$

The real problem is you are testing chance (pun intended). Don't test the chances. Test the logic behind the chances.

If you test chance, you are essentially writing unit tests for the rand function, which I'm sure has been tested thoroughly. You don't need to test this. You do want to test the logic in your class. As such, you need to make the random, unrandom. Composition and dependency injection is the solution here. The random number generator should be its own object, which you can mock in your tests. It also means your Chance class is no longer a singleton:

class Chance

  def initialize(number_generator)
    @number_generator = number_generator
  end

  def percent(amount)
    (generate_number(100) + 1) <= amount
  end

  # x in y chance of being true
  def proportion(x, y)
    generate_number(y) <= x
  end

private

  def generate_number(seed)
    number_generator.generate seed
  end

end

Then you just need two sub classes. The first is the "real" class used in your application:

class RandomNumberGenerator
  def generate(seed)
    rand(seed)
  end
end

The other is a test stub:

class TestRandomNumberGenerator
  def generate(seed)
    raise 'This method should be stubbed in your tests'
  end
end

And some code constructing the Chance objects:

real_chance = Chance.new(RandomNumberGenerator.new)
test_chance = Chance.new(TestRandomNumberGenerator.new)

Since the generate_number method exists, you should be able to mock calls to it, even if it is private:

real_chance.should_receive(:generate_number).with(3).and_return(80)

Then you don't need the number generator objects and the Chance#generate_number method becomes:

def generate_number(seed)
  rand(seed)
end
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.