-3

Given the function:

static boolean chance(int percentage) {
    return percentage != 0 && random.nextInt(101) <= percentage;
}

and a test for that function:

@Test
public void chance_AlwaysFalse_Percentage0() {
    assertFalse(chance(0));
}

The test does not provide with certainty that chance will always return true. I could change the change function to the following:

static boolean chance(int percentage) {
    return random.nextInt(101) <= percentage;
}

and the test would still pass. However, there is a very small chance that random.nextInt(100) will return 0, which would make the function return true, making the test fail.

I could execute this test a billion times as well, but given the nature of random numbers, there is still a minimal chance of failure.

How should I go about testing a function like this? Should it be tested at all? What is a better approach to this problem?

the duck wizard
  • 105
  • 2
  • 8
  • 3
    The method always returns `false` if `percentage == 0`. Before deciding how to test a method you need to decide what the method does. If you are considering changing _the method_ to adjust your testing, you've got it backwards. The method has a set of requirements and a specification, test to those. – Jim Garrison Nov 16 '17 at 17:52
  • @JimGarrison No it doesn't. Notice `<=` – the duck wizard Nov 16 '17 at 18:03
  • 1
    So what you're saying is, the second example of `chance()` has a bug (in that it sometimes returns `true` even if the chance is supposed to be 0) and that the test will only catch this 1% of the time? – D M Nov 16 '17 at 18:10
  • @DM Yes. The test passes with a bug. – the duck wizard Nov 16 '17 at 18:11
  • *but given the nature of random numbers, there is still a minimal chance of failure.* this is fundamentally incorrect. Software based random number generators, especially the one you are using are deterministic if you provide the same seed each time it is ran. –  Nov 16 '17 at 18:53
  • love how the factually incorrect comment as an answer gets accepted –  Nov 16 '17 at 19:48

3 Answers3

2

There are some functionalities in java that may serve for repeatable unit tests:

  • Random numbers: the Random constructor with a seed, for instance new Random(13) will always give the same sequence of random numbers. This already has been exploited to find a random sequence starting with 1, 2, 3, ..., 10. Or the alphabetic letters of someones name.
  • Time: the new java time functions allow for a faked clock, so "now" is always at a given time. Clock.fixed
  • And then there are mocking frameworks, that instrument the byte code of arbitrary calls so the return a provided result. Also useful for blending out more complex contexts.
Joop Eggen
  • 107,315
  • 7
  • 83
  • 138
0

Automated tests can't catch everything. In fact, the first example of your function might also have a bug, in that chance(1) will have a 2% chance instead of a 1% chance of returning true (since it would return true if the random number was either 0 or 1). But if you only test 0 and 100, it would pass those tests 100% of the time. (In fact, 0 and 100 are the only values that will return true the correct percentage of the time; everything else will be off by 1%. The effects of that bug are subtle enough that you may or may not ever notice it, depending on how the function is used.)

You could put your tests in a loop, if you really wanted to. If you ran the chance_AlwaysFalse_Percentage0 test 10,000 times, the odds would be very much in favor of any particular random number being hit. (No, it's not technically guaranteed, but if you calculate the odds, I think you'd be satisfied.) I'm not sure if that's worth doing.

D M
  • 1,410
  • 1
  • 8
  • 12
  • 1
    Please read [How do I write a good answer?](http://stackoverflow.com/help/how-to-answer) before attempting to answer more questions. this is not really an answer but a long opinion based comment, that is actually factually incorrect as well, because it does not actually answer the question on how to deterministically test that method. –  Nov 16 '17 at 19:45
  • I'll accept that my answer might be incorrect, but the "how do I write a good answer" page doesn't suggest anything that would help, so I'm not sure why you're linking to it. – D M Nov 16 '17 at 19:54
  • By the way, after the question edit, the bug I mentioned is reduced but not gone; there are now 101 possibilities, so a 1 would have about a 1.98% chance of returning `true` instead of 2%. – D M Nov 16 '17 at 20:10
  • @DM actually, testing 0 will return `false` in the first example, because the first condition is testing whether it is not zero (it is, and with the and operator, it automatically returns false there). I do agree however, that "automated tests don't catch everything", especially given the pseudorandom nature of the generator. For OP, I would simply do these by paper and pencil and test each value individually. For larger quantities of test numbers (i.e. if we had to test, say, 1,000,000 values), then we would probably test intervals. – ragingasiancoder Aug 02 '18 at 09:46
  • @ragingasiancoder "testing 0 will return false in the first example" - Yes, I never said otherwise. "0 and 100 are the only values that will return true the correct percentage of the time", and the correct percentage for 0 is 0. – D M Aug 02 '18 at 14:28
  • @DM mathematically it makes sense now. That blew over my head. – ragingasiancoder Aug 03 '18 at 00:47
0

Software random number generators are not random

They are deterministic if they are seeded with the same seed. I also corrected the comparison logic to be correct in the range check.

Testable Implementation

import javax.annotation.ParametersAreNonnullByDefault;
import java.util.Random;

@ParametersAreNonnullByDefault
public class Q47336122
{
    private final Random random;

    public Q47336122(final long seed)
    {
        this.random = new Random(seed);
    }

    public boolean chance(final int percentage)
    {
        if (percentage <= 0) { return false; }
        else if (percentage >= 100) { return true; }
        else
        {
            final int r = this.random.nextInt(100);
            return r > 0 && r <= percentage;
        }
    }
}

Deterministic Test

import org.junit.Test;

import javax.annotation.ParametersAreNonnullByDefault;

import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;

@ParametersAreNonnullByDefault
public class Q47336122Test
{


    @Test
    public void testChance()
    {
        final Q47336122 q = new Q47336122(1000L);
        /* poor man's code generator */
//        for (int i=0; i <= 100; i++) {
//        System.out.println(String.format("assertThat(q.chance(%d),is(%s));", i, q.chance(i)));
//        }
        assertThat(q.chance(0),is(false));
        assertThat(q.chance(1),is(false));
        assertThat(q.chance(2),is(false));
        assertThat(q.chance(3),is(false));
        assertThat(q.chance(4),is(false));
        assertThat(q.chance(5),is(false));
        assertThat(q.chance(6),is(false));
        assertThat(q.chance(7),is(false));
        assertThat(q.chance(8),is(false));
        assertThat(q.chance(9),is(false));
        assertThat(q.chance(10),is(false));
        assertThat(q.chance(11),is(false));
        assertThat(q.chance(12),is(false));
        assertThat(q.chance(13),is(false));
        assertThat(q.chance(14),is(false));
        assertThat(q.chance(15),is(false));
        assertThat(q.chance(16),is(false));
        assertThat(q.chance(17),is(false));
        assertThat(q.chance(18),is(true));
        assertThat(q.chance(19),is(false));
        assertThat(q.chance(20),is(false));
        assertThat(q.chance(21),is(false));
        assertThat(q.chance(22),is(false));
        assertThat(q.chance(23),is(false));
        assertThat(q.chance(24),is(true));
        assertThat(q.chance(25),is(false));
        assertThat(q.chance(26),is(false));
        assertThat(q.chance(27),is(false));
        assertThat(q.chance(28),is(false));
        assertThat(q.chance(29),is(false));
        assertThat(q.chance(30),is(false));
        assertThat(q.chance(31),is(false));
        assertThat(q.chance(32),is(false));
        assertThat(q.chance(33),is(false));
        assertThat(q.chance(34),is(false));
        assertThat(q.chance(35),is(false));
        assertThat(q.chance(36),is(true));
        assertThat(q.chance(37),is(false));
        assertThat(q.chance(38),is(true));
        assertThat(q.chance(39),is(false));
        assertThat(q.chance(40),is(true));
        assertThat(q.chance(41),is(false));
        assertThat(q.chance(42),is(true));
        assertThat(q.chance(43),is(false));
        assertThat(q.chance(44),is(false));
        assertThat(q.chance(45),is(false));
        assertThat(q.chance(46),is(false));
        assertThat(q.chance(47),is(false));
        assertThat(q.chance(48),is(false));
        assertThat(q.chance(49),is(false));
        assertThat(q.chance(50),is(true));
        assertThat(q.chance(51),is(true));
        assertThat(q.chance(52),is(false));
        assertThat(q.chance(53),is(true));
        assertThat(q.chance(54),is(false));
        assertThat(q.chance(55),is(true));
        assertThat(q.chance(56),is(false));
        assertThat(q.chance(57),is(true));
        assertThat(q.chance(58),is(false));
        assertThat(q.chance(59),is(false));
        assertThat(q.chance(60),is(true));
        assertThat(q.chance(61),is(false));
        assertThat(q.chance(62),is(false));
        assertThat(q.chance(63),is(true));
        assertThat(q.chance(64),is(true));
        assertThat(q.chance(65),is(true));
        assertThat(q.chance(66),is(true));
        assertThat(q.chance(67),is(true));
        assertThat(q.chance(68),is(true));
        assertThat(q.chance(69),is(false));
        assertThat(q.chance(70),is(false));
        assertThat(q.chance(71),is(true));
        assertThat(q.chance(72),is(true));
        assertThat(q.chance(73),is(true));
        assertThat(q.chance(74),is(true));
        assertThat(q.chance(75),is(false));
        assertThat(q.chance(76),is(true));
        assertThat(q.chance(77),is(true));
        assertThat(q.chance(78),is(true));
        assertThat(q.chance(79),is(false));
        assertThat(q.chance(80),is(true));
        assertThat(q.chance(81),is(false));
        assertThat(q.chance(82),is(true));
        assertThat(q.chance(83),is(true));
        assertThat(q.chance(84),is(true));
        assertThat(q.chance(85),is(true));
        assertThat(q.chance(86),is(true));
        assertThat(q.chance(87),is(true));
        assertThat(q.chance(88),is(true));
        assertThat(q.chance(89),is(true));
        assertThat(q.chance(90),is(true));
        assertThat(q.chance(91),is(true));
        assertThat(q.chance(92),is(true));
        assertThat(q.chance(93),is(true));
        assertThat(q.chance(94),is(true));
        assertThat(q.chance(95),is(true));
        assertThat(q.chance(96),is(true));
        assertThat(q.chance(97),is(true));
        assertThat(q.chance(98),is(true));
        assertThat(q.chance(99),is(true));
        assertThat(q.chance(100),is(true));
    }
}

No matter how many times you run this test it will always pass.

It is a trivial exercise for the reader to expand this to test for thousands or millions of inputs. As long as the seed is the same as when the test data is generated, it will pass when tested.

An injectable random number generator instance would be a better design and would make testing even easier in cases where the random number source is actually not deterministic.

static methods are an anti-pattern and is the crux of your problem with testing as it stands, there is nothing here that can not be fixed with some simple refactoring to get rid of the static stuff and make it deterministic and testable as my code shows.

  • "As long as the seed is the same as when the test data is generated, it will pass when tested." - Yes it will. And that's the problem. By generating the test like this, you're simply defining "what passes this test" as "whatever the code was doing when I generated this test". So if OP used his bugged code (the intentionally bugged code *or* the unintentionally bugged code) to generate this test, that code would *also* pass 100% of the time, in spite of the bug. I see little value in a test that doesn't actually verify the function is doing its job correctly. – D M Aug 02 '18 at 15:51