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

Do these unit tests cover my method under test?

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

Problem

I've written an extension method to truncate strings:

/// 
/// Returns the string, or a substring of the string with a length of  if the string is longer than maxLength. 
/// 
/// The maximum length of the string to return.
/// If  is smaller than zero, an ArgumentException is raised.
/// 
public static String Truncate(this String input, int maxLength)
{
    if (maxLength = 0", "maxLength");
    }

    if (String.IsNullOrEmpty(input) || input.Length <= maxLength)
    {
        return input;
    }

    return input.Substring(0, maxLength);
}


I have written these test cases:

```
[TestFixture]
public class TruncateTests
{
String longString = "ABC";
String shortString = "A";
String nullOrEmptyString = null;
String output = "";

[Test]
public void LessThanOrEqual()
{
// If input is shorter than or equal to maxLength, the input is returned.
output = longString.Truncate(longString.Length + 5);
Assert.AreEqual(longString, output);

output = longString.Truncate(longString.Length);
Assert.AreEqual(longString, output);
}

[Test]
public void GreaterThan()
{
// If input is longer than maxLength, the first N (where N=maxLength) characters of input are returned.
output = longString.Truncate(1);
Assert.AreEqual(shortString, output);
}

[Test]
public void NullOrEmpty()
{
// If input is null or an empty string, input is returned.
output = nullOrEmptyString.Truncate(42);
Assert.AreEqual(output, nullOrEmptyString);

nullOrEmptyString = "";
output = nullOrEmptyString.Truncate(42);
Assert.AreEqual(output, nullOrEmptyString);
}

[ExpectedException(typeof(ArgumentException))]
[Test]
public void MaxLengthException()
{
// If maxLength is smaller than zero, an ArgumentException is raised.
"".Truncate(-1);
}

//http://www.fileformat.info/info/unicode/char/1f3bc/index.htm

Solution

-
About the function comment:

You're over-complicating the summary. As of how I understand the function does Get the first characters as a summary. Special conditions are mentioned bellow. Those should be simplified as well.

-
For maxLength I'd check for min/max integer as well as for -1/0/1 (does the function behave correctly at the limits?)

-
Check input for null and additionally for those combinations:

  • input.length



  • input.length = maxLength



  • input.length > maxLength



-
You're not testing your code when testing for MB-functionality.

-
Test one thing at a time. If NullOrEmpty fails, how to you know which of those assertions failed? Does it fail if input is empty or if it is null?\

-
Remove comments, make the test-names more clear instead. Tests are simple and easy to understand (at least they should be :)). Safe your time of writing comments and improve the test instead.

  • For most of the functions you're just repeating the behavior asserted. However this is already written down by the assertion itself. The comment just duplicates the code.



  • In my opinion, test names should be speaking of what they do. NullOrEmpty doesn't state anything about what actually happens. NullInputReturnsNull and EmptyInputReturnsEmptyString do however. Furthermore speaking test-names can be used as a documentation (i.e testdox).



-
When are you done? Never. Eventually you (or someone else) will find bugs, misbehavior, ... (e.g. during integration). It's more important to keep the tests up to date at those times as well.

Update

On 2. I'm testing for MAX_INT because your promise is this functions accepts integer values between [0, MAX_INT]. Imagine at some time there is a maxLength + 1 statement for some reason. As a rule of thumb: always test the border values of parameters. Further reading: equivalence partitioning and boundary value analysis

On 3. That'd be four tests. You should split those.

On 4. The point is your are testing the c# library, not your code. You don't have any code dedicated for MB handling. What you are actually doing is testing the Substring method and Length property for MB handling.

Context

StackExchange Code Review Q#19453, answer score: 5

Revisions (0)

No revisions yet.