Unit testing a CharGrid

I wrote NUnit tests for class CharGrid from this homework assignment.

My goal here is DRY: I want to keep the testing code as short and simple as possible. In addition, want to run every test independently, as opposed to having a single method with a lot of asserts that either succeeds or fails as a whole.

I went through the NUnit documentation and found this "Parameterized TestFixture" annotation, which seemed to help fulfill my goal.

I would like to ask if there are any improvements to the testing code regarding what I just stated my goals were.

In addition, I would like to know if it is a good idea to have a separate CharGridTestFixture class or if it is preferable to directly annotate class CharGrid with a TestFixture and it's methods with TestCaseSource annotations and test them sort of in-place. This way there would be one less class.

Moreover, I used NUnit 2 without looking at any of the alternatives. Is it still a decent Unit testing framework or should I switch to NUnit 3 or something else?

using System;
using System.Collections;
using System.Collections.Generic;

using NUnit.Framework;

namespace CS108
{

This is a skeleton of the code to be tested:

```
// HW1 2-d array Problems
// CharGrid encapsulates a 2-d grid of chars and supports
// a few operations on the grid.

public class CharGrid {
private char[][] grid;

/**
* Constructs a new CharGrid with the given grid.
* Does not make a copy.
* @param grid
*/
public CharGrid(char[][] grid) {
this.grid = grid;
}

/**
* Returns the area for the given char in the grid. (see handout).
* @param ch char to look for
* @return area for given char
*/
public int charArea(char ch) {
return 0;

C# conventions

The assignment you posted from is written for Java. While the syntax is similar, C# has some slightly different conventions with naming and formatting. In particular, in C# it's normal to put opening braces on a new line, and for method names to be PascalCased rather than camelCased.

Also, you seem to have a very deep indentation. Visual Studio defaults to 4 characters, which is what seems to be conventional for C#.

As a side note, it's also usually considered poor practice to leave commented out ("zombie") code around. If you don't need it, delete it.

Naming

Some of your names don't mean much. What's "that" or "e"? These should be replaced with clear, descriptive names. Similarly, don't abbreviate names. There's no reason to write tc instead of testCase.

Simplicity

When creating complex code structures, it's useful to compare them to the alternative and ask what exactly the trade-offs are. So the pros and cons of your approach as I see them:

Pros

• It reduces repetition in rewriting the same tests for different test cases

Cons

• It requires a weird structure where you have subclasses differing from their parents only by data (which you'd normally just do through constructor arguments)

• It limits you to using the same test data for all of your tests

• It requires a relatively hard to understand construct involving generics

Given this, I'd be a little reluctant to actually use this kind of approach unless I was dead sure that a) the level of repetition for a simpler approach would be too much and, b) the rigidity of which test data I can use would not be a problem.

As it is, if you're not planning to write more tests, I think a) is false, and if you are planning to write more tests then I think b) is false until they're done.
Alternative

Even if you do want to try to cut down on repetition, you might want to look at an alternative, like the TestCase attribute. You could, for example, parameterize a test method like:

public void xxx(string grid, int expectedValue)

And take grid in a format where / represents a new line (so the grid for your first test case would be written "cax/b b/ a"), then have a helper method that translates that to your grid format.

This would give you a lot more flexibility and simplicity, and if you had repeated cases, you could assign the string representations to consts. Then your existing test class would look like:

[TestFixture]
public class CharGridTestFixture
{
    private static CharGrid CreateGrid(string grid)
    {
        //Some implementation
    }

    [TestCase("cax/b b/  a", 0)]
    //Other test cases
    public void CountPlus(string grid, int expected)
    {
        Assert.AreEqual(expected, CreateGrid(grid).countPlus());
    }
}

Types of tests

Given the relative simplicity of the problem, I'd consider the tests you're doing to be acceptance tests rather than unit tests. A unit test would generally target one very specific point of behavior, whereas your tests are more along the lines of setting up a realistic, potentially complex, situation and getting a result without worrying too much about how the code under test handles it.

Things which would be appropriate for unit tests might include:

• Degenerate cases- e.g. a grid with 0 rows and columns, a grid containing no letters

• Simple cases- e.g a grid with only a single cell, a grid with only a single row, a grid containing a single letter

• As-minimal as possible complex cases. For example, when you want to test that going from a single row to multiple rows works, you might pick a 2x1 or 2x2 grid and only move to 3x3 or more if you had a reason to think it'd be qualitatively different in some way. The contents of these grids would also be as minimal as possible while trying to capture whatever it is you're trying to test in that particular case.

• Invalid cases like non-rectangular grids (assuming these are invalid) to test error-handling

Generally these would go along with much more descriptive test method names stating the precise expectation which is being tested.

That isn't to say there's anything wrong with the kinds of tests you've done, acceptance tests have their own uses, I just wouldn't consider it unit testing.

StackExchange Code Review Q#95441, answer score: 5