Unit test is acting weird - can you shed some light on this mystery?

Question

Here is my test:

[TestFixture]
public class DisplayingPageLinks
{
    [Test]
    public void Can_Generate_Links_To_Other_Pages()
    {
        //Arrange: We're going to extend the Html helper class.
        //It doesn't matter if the variable we use is null            
        HtmlHelper html = null;

        PagingInfo pagingInfo = new PagingInfo(){
            CurrentPage = 2,
            TotalItems = 28,
            ItemsPerPage = 10
        };

        Func<int, String> pageUrl = i => "Page" + 1;

        //Act: Here's how it should format the links.
        MvcHtmlString result = html.PageLinks(pagingInfo, pageUrl);

        //Assert:
        result.ToString().ShouldEqual(@"<a href=""Page1"">1</a><a href=""Page2"">2</a><a href=""Page3"">3</a>");
    }
}

Here is the PageLinks extension method:

public static MvcHtmlString PageLinks(this HtmlHelper html, PagingInfo pagingInfo, Func<int,string> pageUrl)
{
    StringBuilder result = new StringBuilder();

    for (int i = 1; i < pagingInfo.TotalPages; i++)
    {
        TagBuilder tag = new TagBuilder("a");
        tag.MergeAttribute("href", pageUrl(i));
        tag.InnerHtml = i.ToString();
        if (i == pagingInfo.CurrentPage)
        {
            tag.AddCssClass("selected");
        }
        result.Append(tag.ToString());
    }

    return MvcHtmlString.Create(result.ToString());
}

Finally here is the result for the running test:

SportsStore.UnitTests.DisplayingPageLinks.Can_Generate_Links_To_Other_Pages: Expected string length 63 but was 59. Strings differ at index 24.
Expected: "123" But was: "12"
-----------------------------------^

The error doesn't copy the way it's shown in the GUI - sorry.

Can you give me some suggestions as to why NUnit is saying it receives something I don't expect it to give out.

According to what I'm reading in the PageLinks extension method, it seems that the markup should be formed correctly.

Any suggestions? I'm a TDD newbie and really trying to learn here. :)

Edit:

It seems the culprit was this. My Test was using: Func pageUrl = i => "Page" + 1;

instead of

Func pageUrl = i => "Page" + i;

But now there's another error. :(

It seems something is wrong when calculating the amount of pages in the PagingInfo class:

public class PagingInfo
{
    public int TotalItems { get; set; }
    public int ItemsPerPage { get; set; }
    public int CurrentPage { get; set; }

    public int TotalPages
    {
        get { return (int)Math.Ceiling((decimal)TotalItems / ItemsPerPage); }
    }
}

It seems the error stems on the fact that this is returning 2 pages, instead of 3.

Is there something wrong in this calculation?

score 4 · Accepted Answer · answered Mar 15 '11 at 22:28

4

(Replying to the new question posed in your edit.) Your loop in PageLinks() is starting with i = 1 and having i < pagingInfo.TotalPages as condition - a classical off-by-one error. :-)

answered Mar 15 '11 at 22:28

Aasmund Eldhuset

37,289
4
68
81

That was it! :) It's funny how learning something new can **really** narrow your field of vision on a problem. Thanks for your time. – Mar 15 '11 at 22:33
+1: as written, the loop will only execute 2 times. For values 1 and 2. When i is incremented to 3 it fails the loop condition and the 3'rd pass never occurs. Change "(int i = 1;" to "(int i = 0;" – NotMe Mar 15 '11 at 22:34
@Sergio Tapia: Indeed - and who knows how often I've been there myself. :-) By the way, see @StriplingWarrior's great suggestion for directly testing `PagingInfo.TotalPages`, which would have revealed that the error is elsewhere. – Aasmund Eldhuset Mar 15 '11 at 22:42

score 1 · Answer 2 · edited May 23 '17 at 12:11

1

Is there something wrong in this calculation?

public int TotalPages
{
  get { return (int)Math.Ceiling((decimal)TotalItems / ItemsPerPage); }
}

Yes, you are playing fast and loose with the types and operations involved. Without knowing operator precedence, you don't know if the decimal convert is applied before the divide, and therefore don't know if the divide is a decimal divide or an int divide.

Use multiple statements and be clear with the types. Read this post.

I will consider the downvote as a prompt for more help. Consider this: the inputs are all integers. Can the problem be resolved in the domain of integers? Is it necessary to involve decimals at all?

public int GetTotalPages()
{
  int fullPages = TotalItems / ItemsPerPage;
  bool partialPage = TotalItems % ItemsPerPage != 0;
  int result = fullPages + (partialPage ? 1 : 0);
  return result;
}

edited May 23 '17 at 12:11

Community

1
1

answered Mar 15 '11 at 22:33

Amy B

108,202
21
135
185

While I wasn't the one who gave the downvote, I would consider it a slap on the wrist for jumping to conclusions. As it turns out, the logic in this method is correct, whereas a simpler solution like `return TotalItems / ItemsPerPage` would be incorrect. And unfortunately your tone comes across as snarky rather than helpful. I am curious, though, to hear how you would solve this problem using only integers. – StriplingWarrior Mar 15 '11 at 22:58
+1, by the way, for linking to that other SO post. Very interesting read. The `( dividend + divisor - 1 ) / divisor` solution looks like it would work well in this case, but it's not quite as clear what you're doing if the person looking at it doesn't know this trick already. – StriplingWarrior Mar 15 '11 at 23:13

score 1 · Answer 3 · answered Mar 15 '11 at 22:34

1

Is there something wrong in this calculation?

Good question. As an exercise in proper Test-Driven Design, try creating a unit test that isolates this specific calculation. Something like this:

Assert.AreEqual(3, new PagingInfo {TotalItems = 4, ItemsPerPage = 9}.TotalPages);

If these unit tests show that the function is calculating your input correctly, then you know that the consuming class must be providing the wrong inputs.

The real issue here is probably the one that Aasmund found, but I hope this answer gives you an idea about how to improve your unit testing to hone in on the real source of errors.

answered Mar 15 '11 at 22:34

StriplingWarrior

151,543
27
246
315

He already knows that when there are 3 pages it returns 2. Your test reveals nothing. – Amy B Mar 15 '11 at 22:37
2

@David B: No, he didn't know that - he _thought_ that, and it was an incorrect assumption. This test (and hopefully some more tests for various corner-cases) would have revealed that `PagingInfo.TotalPages` worked correctly, and that the problem must therefore be elsewhere (which it was, namely in the loop). So +1 to this answer. – Aasmund Eldhuset Mar 15 '11 at 22:44
@David B: He knows that he's getting a string that contains a 2 where it should contain a 3, but he is asking whether his calculation is to blame. A test like this would either reveal that his calculation is not to blame and he should focus on the PageLinks method itself (which is the case here), or that it is to blame and he needs to fix it. Since he says he's trying to learn about TDD, I see this as a good opportunity to show how writing more targeted unit tests can be beneficial. – StriplingWarrior Mar 15 '11 at 22:50

Unit test is acting weird - can you shed some light on this mystery?

3 Answers3