Unit Testing for isPrime function

Question

I've decided that I want to take a stab at test first programming. So, before I tackled writing an isPrime function, I wrote this unit test. It's my first and I'm not sure I'm doing this right.

I was thinking that I might want to extract the loops to just two methods that I would pass an array to. One for Assert.IsTrue and one for Assert.IsFalse, but I wasn't sure if that was a good idea in a unit test.

• Am I covering my bases here?
• What other cases am I missing?
• What would you do differently?

using System;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Challenges;
using System.Numerics;

namespace ChallengesTest
{
    [TestClass]
    public class PrimeTest
    {
        [TestMethod]
        public void SmallPrimes()
        {
            int[] numbers = { 2, 3, 5, 7, 11, 13, 17, 19, 23, 29 };

            for (var i = 0; i < numbers.Length; i++)
            {
                Assert.IsTrue(Numbers.isPrime(numbers[i]));
            }

        }

        [TestMethod]
        public void Negatives()
        {
            int[] numbers = { -2, -3, -5, -7, -11, -13, -17, -19, -23, -29 };

            for (var i = 0; i < numbers.Length; i++)
            {
                Assert.IsFalse(Numbers.isPrime(numbers[i]));
            }
        }

        [TestMethod]
        public void PositiveNotPrime()
        {
            int[] numbers = { 4, 6, 8, 9, 10, 12, 14, 15, 16, 18, 20, 21, 22, 24, 25 };

            for (var i = 0; i < numbers.Length; i++)
            {
                Assert.IsFalse(Numbers.isPrime(numbers[i]));
            }
        }

        [TestMethod]
        public void ZeroAndOne()
        {
            Assert.IsFalse(Numbers.isPrime(0));
            Assert.IsFalse(Numbers.isPrime(1));
        }

        [TestMethod]
        public void BigPrimes()
        {
            int[] numbers = {104677,104681, 104683, 104693, 104701, 104707, 104711, 104717, 104723, 104729 };

            for (var i = 0; i < numbers.Length; i++)
            {
                Assert.IsTrue(Numbers.isPrime(numbers[i]));
            }
        }
    }
}

Answer 1

This is quite an extensive set of tests, as each of the numbers in the list is a test of sorts.

Here are some suggestions:

1. How about testing negatives that are non-primes?
2. Add a set of tests for big non-primes (pseudoprimes ideally)
3. Add tests for values that are close to the limit of ints

Lastly, this is optional, but you can add some that are randomly generated so you can't cheat and hardcode the results.

Answer 2

Methods in C# are written in PascalCase, so let's change isPrime to IsPrime.

You can use LINQ to make the code a bit clearer. Instead of

for (var i = 0; i < numbers.Length; i++)
{
    Assert.IsTrue(Numbers.isPrime(numbers[i]));
}

You can write

Assert.IsTrue(numbers.All(Numbers.IsPrime));

Instead of

for (var i = 0; i < numbers.Length; i++)
{
    Assert.IsFalse(Numbers.isPrime(numbers[i]));
}

You can write

Assert.IsFalse(numbers.Any(Numbers.IsPrime));

or

Assert.IsTrue(numbers.All(n => !IsPrime(n)));

whichever you find easier to read (I prefer the former).

In fact, I would be a bit lazy and copy a big list of prime numbers from somewhere and then write

var primes = Enumerable.Range(0, 1000000).Where(Numbers.IsPrime);
Assert.IsTrue(primes.SequenceEqual(PrimesLessThanOneMillion));

---

If you take @mleyfman's suggestion to test with randomly generated primes, be sure to seed the random number generator. Unit tests must be reproducible.

That is, never do this

var random = new Random();

but do this

var random = new Random(42);

One of the benefits of unit tests is that if they fail, you know it is due to a change in the code, and can then git bisect (or similar) to find the breaking commit. If a unit test fails due to something that is not a change to the code (e.g. a different RNG seed) you have lost this important benefit.

Edit: as @svick pointed out, using the same seed may not generate the same sequence between different versions or implementations of .NET. See these answers on StackOverflow. If all dev machines (including the build server) are running the same version of .NET, you should be fine.

Answer 3

Naming

To understand why a test fails we need to be able to understand what the test is supposed to test by just glancing at the method name.

So looking at your testmethod

public void SmallPrimes()

wouldn't tell you, in the case it fails, what this test method should be testing.

Unit Test methods should be named following MethodName_StateUnderTest_ExpectedBehavior.

So a better name would be:

public void IsPrime_RangeOfPrimesFromTwoToTwentyNine_Evaluated() 

And

public void Negatives()

would become

public void IsPrime_RangeOfNegativeNumbers_Evaluated()

For

public void ZeroAndOne()

I see another problem.The method's name implies that this method has two responsibilities. If you don't mind , you should at least provide a message to the Assert to indicate which part has failed.

So changing this method to

[TestMethod]
public void IsPrime_NumbersZeroAndOne_Evaluated()
{
    Assert.IsFalse(Numbers.isPrime(0),"Failed for {0}",0);
    Assert.IsFalse(Numbers.isPrime(1),"Failed for {0}",1);
}

will show you, if this test will fail, with which number your code has problems with.

or better

[TestMethod]
public void IsPrime_NumberZero_Evaluated()
{
    Assert.IsFalse(Numbers.isPrime(0), "Failed for {0}",0);
}

[TestMethod]
public void IsPrime_NumberOne_Evaluated()
{
    Assert.IsFalse(Numbers.isPrime(1), "Failed for {0}",1);
}

the former SmallPrimes() testmethod will become

 [TestMethod]
 public void IsPrime_RangeOfPrimesFromTwoToTwentyNine_Evaluated()()
 {
     int[] numbers = { 2, 3, 5, 7, 11, 13, 17, 19, 23, 29 };

     for (var i = 0; i < numbers.Length; i++)
     {
         Assert.IsTrue(Numbers.isPrime(numbers[i]), "Failed for {0}",numbers[i]);
     }

 }

Some might complain that the method names are too long but in the end it is all about readability.

Answer 4

What would you do differently?

I would use NUnit rather than the Microsoft test tools. NUnit has a better assertions library (for example Assert.Throws<T>() is quite useful). It also has easier data-driven testing that would let you remove all of the looping logic from your code. For example:

[Test]
[TestCase(2)]
[TestCase(3)]
[TestCase(5)]
[TestCase(7)]
[TestCase(11)]
[TestCase(13)]
[TestCase(17)]
[TestCase(19)]
[TestCase(23)]
[TestCase(29)]
public void SmallPrimes(int num)
{
    Assert.IsTrue(Numbers.isPrime(num));
}

Admittedly it's a lot of lines, but they're very easy to understand. Also, the test method itself becomes dead simple.

Since I'm on the topic of tooling, you might want to look into the TestDriven.NET Visual Studio plugin, which provides very nice "right-click -> run test" integration.