HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Unit Testing for isPrime function

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
functionisprimetestingforunit

Problem

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++)

Solution

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.

Code Snippets

for (var i = 0; i < numbers.Length; i++)
{
    Assert.IsTrue(Numbers.isPrime(numbers[i]));
}
Assert.IsTrue(numbers.All(Numbers.IsPrime));
for (var i = 0; i < numbers.Length; i++)
{
    Assert.IsFalse(Numbers.isPrime(numbers[i]));
}
Assert.IsFalse(numbers.Any(Numbers.IsPrime));
Assert.IsTrue(numbers.All(n => !IsPrime(n)));

Context

StackExchange Code Review Q#58224, answer score: 10

Revisions (0)

No revisions yet.