Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I've written an extension method to truncate strings:

/// <summary>
/// Returns the string, or a substring of the string with a length of <paramref name="maxLength"/> if the string is longer than maxLength. 
/// </summary>
/// <param name="maxLength">The maximum length of the string to return.</param>
/// <exception cref="ArgumentException">If <paramref name="maxLength"/> is smaller than zero, an ArgumentException is raised.</exception>
/// <![CDATA[ Documentation:
/// If the string is null or an empty string, the input is returned.
/// If the string is shorter than or equal to maxLength, the input is returned.
/// If the string is longer than maxLength, the first N (where N=maxLength) characters of the string are returned. 
/// ]]>
public static String Truncate(this String input, int maxLength)
{
    if (maxLength < 0)
    {
        throw new ArgumentException("Must be >= 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
    string multiByteString = "\x1F3BC";

    [Test]
    public void MultiByte()
    {
        Assert.IsTrue(multiByteString.Length == 2);

        output = multiByteString.Truncate(1);

        Assert.IsTrue(output.Length == 1);
    }
}

Now how can I confirm that:

  1. The method Truncate() does what it is supposed to do?
  2. I have written test code to test al promises made?
  3. This test code follows practices and guidelines valid for unit testing?

I'm especially curious about the last one. I've written a few tests in my time, but I'm never sure whether I'm testing enough and not too much at the same time. Can anyone shed a general light on this and maybe point me towards invaluable resources about unit testing?

share|improve this question
    
I needed this for UI strings, so I made a version that adds ellipsis: gist.github.com/4250125 –  ANeves Dec 10 '12 at 11:43
    
Why do you throw when maxLength < 0, but not throw when someone calls ((string)null).Truncate(2)? –  ANeves Dec 10 '12 at 11:46
    
@ANeves that's a design decision. I want to be able to run this methods on strings that may be null or of arbitrary length. –  CodeCaster Dec 10 '12 at 11:49
add comment

1 Answer 1

up vote 5 down vote accepted
  1. About the function comment:

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

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

  3. Check input for null and additionally for those combinations:

    • input.length < maxLength
    • input.length = maxLength
    • input.length > maxLength
  4. You're not testing your code when testing for MB-functionality.

  5. 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?\

  6. 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).
  7. 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.

share|improve this answer
    
I agree with you on 1, 5, 6 and 7. For 2: why should I check for int limitations? You can't input Int32.MaxLength + 1, because that won't fit in the int parameter, so why would I want to check for that? I can indeed add a test for maxLength 0. The other cases I'm already testing. 3: see comment on question, I don't want to throw on input == null. I already test the other cases, in LessThenOrEqual(), I will split those up in separate methods. 4: I do, aren't I? I'm checking whether a multibyte (2) character gets split into the amount of characters I expect (1). –  CodeCaster Dec 10 '12 at 11:53
    
@codecaster: answered in my answer –  Fge Dec 10 '12 at 12:32
    
For 2: "Imagine at some time there is a maxLength + 1 statement for some reason." - there currently isn't, so why should I test for something I know will never happen? Just in case I add a +1, sometimes, somewhere, and then forget to add a test for it? Then how far must one go (which basically is the question I asked)? Can you elaborate on that? :-) For 4: I want to make sure my method returns 1 character when I pass maxLength of 1, regardless of the input size (as long as it's >= 1). –  CodeCaster Dec 10 '12 at 12:45
    
For 4: What which lines are dedicated for this feature? None. If this test fails this is because of a c# core function fails the MB-handling, not your code does. Test only stuff you are responsible for, not your dependencies (and speaking strictly, String is a dependency which could be moked). –  Fge Dec 10 '12 at 13:00
    
For 2: I guess we'll have to decide if we want whitebox or blackbox tests then... Assuming blackbox we don't know the code and "have go guess" (see links above). In whitebox tests we could skip those. Choosing the values to test for and how far to go (e.g. how to find "easter-eggs") ... tricky questions with no definite answer :) –  Fge Dec 10 '12 at 13:04
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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