3
\$\begingroup\$

I'm currently working on a unit testing course (NUnit 3.x). I've been tasked with a simple class to test all edge cases.

using System;

namespace TestNinja.Fundamentals
{
    public class DemeritPointsCalculator
    {
        private const int SpeedLimit = 65;
        private const int MaxSpeed = 300;

        public int CalculateDemeritPoints(int speed)
        {
            if (speed < 0 || speed > MaxSpeed) 
                throw new ArgumentOutOfRangeException();

            if (speed <= SpeedLimit) return 0; 

            const int kmPerDemeritPoint = 5;
            var demeritPoints = (speed - SpeedLimit)/kmPerDemeritPoint;

            return demeritPoints;
        }        
    }
}

And -

using System;
using NUnit.Framework;
using TestNinja.Fundamentals;

namespace TestNinja.UnitTests
{
    [TestFixture]
    public class CalculateDemeritPointsTests
    {
        private DemeritPointsCalculator _demeritPointsCalculator;

        [SetUp]
        public void SetUp()
        {
            _demeritPointsCalculator = new DemeritPointsCalculator();
        }

        // my solutions
        [Test]
        [TestCase(65,0)]
        [TestCase(70,1)]
        [TestCase(75,2)]
        [TestCase(80,3)]
        [TestCase(0,0)]
        [TestCase(60,0)]
        [TestCase(66,0)]
        public void CalculateDemeritPoints_WhenCalled_ReturnsExpectedInt(int speed, int expectedResult)
        {
            var result = _demeritPointsCalculator.CalculateDemeritPoints(speed);
            Assert.That(result, Is.EqualTo(expectedResult));
        }

        [Test]
        public void CalculateDemeritPoints_SpeedLessThan0_ThrowsException()
        {
            Assert.That(() => _demeritPointsCalculator.CalculateDemeritPoints(-1), Throws.InstanceOf<ArgumentOutOfRangeException>());
        }

        [Test]
        public void CalculateDemeritPoints_SpeedGreaterThan300_ThrowsException()
        {
            Assert.That(() => _demeritPointsCalculator.CalculateDemeritPoints(301), Throws.InstanceOf<ArgumentOutOfRangeException>());
        }
    }
}

Am I using to many test cases for CalculateDemeritPoints_WhenCalled_ReturnsExpectedInt?

I was thinking about merging the final two tests into one as they would also work with test cases, but I wasn't sure if I was doing to many.

\$\endgroup\$
2
\$\begingroup\$

Am I using too many test cases for CalculateDemeritPoints_WhenCalled_ReturnsExpectedInt?

Yes. I'd say so, anyway.

Let's say I'm maintaining code you've written. When a test case fails, ideally it will give me exactly the information I need to fix it, and it will give me that information as quickly as possible. The very first thing I will see when I learn a test has failed? That test's name.

Currently if I see that this test method has failed, all I learn from the name is that the calculator didn't return what was expected. That doesn't give me very much information about what kind of bug to look for, so I'll have to dig a little deeper to find what what speed was passed in, what demerit value was returned, and why that value was unexpected.

You can save me that digging by adding a test called CalculateDemeritPoints_LegalSpeed_GivesNoDemerits, and putting all your TestCase(_,0) cases there. As a bonus, [TestCase(60)] on top of a method named "Legal Speed" is more instantly readable than [TestCase(60,0)] on top of a method named "Expected".

In general, if a single test method is testing different kinds of behavior, I will say it's too much. As a rule of thumb, different branches of code probably deserve different test methods.


As a side note, I think it would be fine if you combined the two exception test methods into one.

On the other hand, I'd also be fine if you skipped test parameterization and made separate test methods for every single test case.

The key, in my opinion, is to look at it through the maintainer's eyes. How much can you assist that person in understanding what your code is doing, and what's going wrong with it?


As another side note, I also dislike (although it's not a strong dislike) the common practice of class-level setup and teardown methods. Especially in a case like this; the [Setup] is saving you exactly one line of code per test, and test code is cheap.

As a maintenance programmer, if I see that a test has failed, I will always have to go inspect the failing test method. Will you also make me search for any other methods that might have fired as a part of the test? For example: If a test is failing because the test object was constructed with a strange argument, and the call to the constructor was in a setup method, that means cause of the failure has been hidden from me.

For that reason, I like to err on the side of verbose test methods, in order to make each method self-contained (and therefore, easier to follow).

\$\endgroup\$
  • \$\begingroup\$ very good point from the perspective of maintenance, a failed unit test should give a concrete and meaningful error message \$\endgroup\$ – dfhwze Jul 20 at 20:34
2
\$\begingroup\$

You probably want to

  • add a constant for speed 0
  • create a test for MaxSpeed
  • create a test case for 64 (it's one edge of the partition class 0 <= x < speedlimit and it could replace 60 which is in the same partition class)

I would keep the 66 test case since it is and edge case.

Even though the edges are cut pretty clear with ints (compared to using float values) you might still want to test them explicitly.

\$\endgroup\$
1
\$\begingroup\$

Since this code is a black box, because it calculates everything itself without calling any dependencies the caller can be aware of, we should test this method with sufficient variations of input to yield us:

  • any speed < 0 should yield error
  • any speed > 300 should yield error
  • any speed between 0 and 65 (bounds included) should yield 0
  • any speed between 66 and 300 (bounds included) should yield the integer division (speed - 65) / 5

If the method would have been coded as a white box instead, using mockable dependencies, things would have been different. We could then just test Assert.WasCalled(depedency.method(speed)).

In-depth testing

These are the test cases a developer/tester should write that does not have access to the content of the method, only the interface and specification.

Since the entropy is not that big and we are working with discrete integers, I suggest to test any integer from 0 - buffer to 300 + buffer against either a matrix of pre-calculated results or a formula in the unit test that emulates the algorithm. I prefer the former, since a bug in the method flow is easily introduced in the unit test as well.

Minimal test cases

These are the minimal test cases the developer that knows the content of the method could perform.

//     speed    result    test case justification
//        -1     error    upper bound of of speed < 0
//        0          0    lower bound of speed >= 0 and speed <= SpeedLimit
//        65         0    upper bound of speed >= 0 and speed <= SpeedLimit
//        66         0    lower bound of speed > SpeedLimit & speed <= MaxSpeed
//     70-74         1    group #1 to test (speed - SpeedLimit) / 5
//     75-79         2    group #2 to test (speed - SpeedLimit) / 5
//       300        47    upper bound of speed > SpeedLimit & speed <= MaxSpeed
//       301     error    lower bound of speed > MaxSpeed
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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