Showing posts with label unit tests. Show all posts
Showing posts with label unit tests. Show all posts

Saturday, August 6, 2011

Living with observational tests

I'd say that at least half of the unit tests I look at end up having a ton of extra work and extra assertions. It's always really frustrated me but I think some developers just find this style of tests to be more helpful. I think some people view tests as a central claim with proof and others view it as a record of observations.

I view unit tests like a math proof or logical argument. I have a claim (also known as an assertion) and I have the simplest proof of that claim I can think of. If my claim is that new bank accounts start with a zero balance then my test will mention the claim in the title and assertion. Maybe something like this:

[Test]
public void NewBankAccountShouldStartWithAZeroBalance()
{
    BankAccount newBankAccount = new BankAccount();
    Assert.AreEqual(0, newBankAccount.Balance, "A new bank account should start with a zero balance.");
}

My recent insight is that others seem to view tests more like a record of observations. Imagine walking through a park and writing what you see and hear along the way. "There's a dog. The dog is barking. There's a tree. There's a guy in a suit. The dog is still barking. There's a jogger." etc. Many of the unit test I see are like this, a walk through some scenario or object lifetime with observations made along the way:

[Test]
public void BankAccountTest()
{
    BankAccount account = new BankAccount();
    Assert.IsTrue(account != null, "The account is not null");
    Assert.IsTrue(account.Balance == 0, "The balance is zero."); 
    
    account.DoSomething("ABC", 123);
    Assert.IsTrue(account.Things.Count == 3, "The account has three things."); 
    Assert.IsTrue(account.Balance == 0, "The balance is zero."); 

    // 60 more lines of stuff somewhat related to bank accounts....
}

My frustrations came from realizing that these tests were written differently than what I prefer but not realizing that it's because there's a completely different way of looking at what a test is. The assertion messages are worded differently, the test is named differently, the assertions are different, the only similarity is that both styles are run from NUnit. These aren't unit tests in the strict meaning, they aren't integration tests, and they aren't really system tests; they're what I think of as observational tests — just a bunch of observations. I couldn't figure out what the point of this kind of test was because there is no point to the test. The assertions seemed irrelevant or superfluous because the assertions aren't used as proof of a central claim, they're just things that the original developer noticed or had on his mind when writing the test. They don't say what something should be or why because they're just observations of how things are — there is no expectation or reasoning behind it — just a bunch of observations.

I'm still not sure what to do with failing assertions on these observational tests. I don't want to delete the entire test because they provide some value to someone (well, actually yes I do want to delete them but I won't). These observational tests often fail but I've never found a bug in the code from these; it's always been a bad test or a valid change in the code that broke the now invalid assertion. There's no point in trying to change the developers who like these tests since not only do they prefer these observational tests but I've already tried and failed. I think that if it doesn't look like an obvious bug then I should just update the assertion or delete it. That's what I've always ended up doing anyway but now I won't waste more than a couple minutes on it since it's not worth the frustration.

Friday, August 5, 2011

A unit test case study

I've been trying to pay more attention to my process for fixing broken unit tests and I'm trying to improve my writing skills, especially when it comes to descriptive narration, so I'm going to fix a test and write down my thoughts as I go. I'll change the business object names to Widgets and Things, but everything else is exactly what happened as it happened. Here goes.



I ran some of our unit tests and the first one to fail has this error message:

XXX.Test.YYY.Widgets.WidgetsTest:
  Two things should be available.
  Expected: True
  But was:  False
Uh, ok. No indication of which two things were expected or which two things were found. Not a very useful error message. The test name doesn't help me much since the Widgets domain encompasses so much. The Widgets code does use Things though so that part makes sense.

I'm guessing the requirements and code changed so instead of two Things available there are now more. This is what happens with at least 95% of the broken tests I look at. I've gotten all I can from the test name and assertion message so it's time to take a look at the test itself.

public void WidgetsTest()
{
    // 44 lines of code
    Assert.IsTrue(thingList.Count == 2, "Two things should be available.");
    // 17 more lines of code
}
Ugh. I'm not even going to try to wrap my head around this yet. Some people are really good at reading through long methods where state winds in and out of methods but I'm not. I'll start with a small change and use AreEqual instead of IsTrue so the failure message indicates what was expected and what was found.
public void WidgetsTest()
{
    // 44 lines of code
    Assert.AreEqual(2, thingList.Count, "Two things should be available.");
    // 17 more lines of code
}
At least now I can see how many were available. It would be better if I know which two Things were expected so I could assert that only ThingA and ThingB are available. Even better than that would be knowing why so I could add that to the message, something like "ThingA and ThingB should be available if Widget.IsAwesome()". That way someone could see it and say "yup, there's a bug" or "no that's not true anymore". At least that would work better for me but I remind myself that I can't really fault others for writing tests in the style they prefer to work with.

Now I wait nearly five minutes while the tests rebuild then wait about a minute for the test to fail again. New error message:

XXX.Test.YYY.Widgets.WidgetsTest:
  Two things should be available.
  Expected: 2
  But was:  3
That's what I expected. I'm pretty sure no one updated the test when they changed the code. Had it said 0 or 500 or something interesting like that then it might be a bug but I'd say there's now a 99% chance it's just an invalid assertion. There is a reference in the test to an issue number so I look that up....

WTF? This issue has absolutely nothing to do with with Things or Widgets! Maybe it's a typo? Maybe it's a bad April Fools joke? Maybe the test was copied and pasted and this reference was left in? I've noticed that's common; I guess people copy the entire thing and leave in a bunch of irrelevant code and assertions that are no longer needed. I search the tests for the original assertion and see that there's one other test with the same line of code. Not only is almost the entire test is the same as the one that's failing but this one is also older so it's almost certainly a copy-and-paste job. This original test does have a comment with an issue number but when I look that up I find the issue has nothing to do with Widgets or Things either. Maybe I can check source control to see when these tests were made and who made them?

The original developer of both tests is long gone so I can't ask him what this is really about. Another developer who's focus is a different domain has made changes to the failing test eight months ago. He probably won't remember but I'll ask him later if I run into a dead end.

I try looking at our continuous integration logs to see when the test started failing but no luck: our logs only go back a couple months and this has been broken the whole time. *sigh* We've always got broken tests but this one has actually been broken since the beginning of recorded history. Looks like I'll have to dive into the test code and see what it's doing.

The test starts by dropping the database then reloading a backup (I've been here 5 or so years and brought this up before and I still don't know why so many of our tests do this), creates a handful of objects while doing a ton of work with them, reading and writing to the file system and database through it all. It even clears some caches along the way; I've always wondered if so many of other peoples tests do this because of superstition or to make sure we do as much work as possible. There are several comments but they just say the same thing as the code following it with no indication of why any of it is being done. This test seems to be running through some very specific scenario involving Widgets from beginning to end. There's a lot of data and I'm not sure what's relevant and what just happened to be that way. There are 14 scattered assertions, not including the assertions buried within other methods, on seemingly random objects and values. A lot of this work is somewhat related to Widgets but this is the only line related to Things. In fact, only one of the assertions is directly related to Widgets.

Time to recap: I've got a failing assertion in a large test filled with seemingly arbitrary assertions and a lot of work. The test name says nothing about what is expected or why. This failing assertion says nothing about what is expected or why. The issue it refers to is completely unrelated to Things or Widgets and the original developer has moved on to another job. I have no idea when it started failing or why and what it's asserting on doesn't even seem to be relevant to the other work happening in the test. This is all quite normal for our tests. Luckily it's the end of the day so I send an email to the other developer who worked on this eight months ago and leave this until tomorrow.

Here's the email:
Unit test XXX.Test.YYY.Widgets.WidgetsTest has a failing assertion on line 2199. The message is "Two Things should be available.". It looks like this test is from you and Mr Doesn't Work Here Anymore. Are there two specific Things or are any two Things acceptable? Why are those two Things expected and why are three Things a problem?

Hopefully he has some answers.



When I get to work the next day there's no response from the developer I emailed. I move on to some other tasks until I can ask him or someone else.

Eventually another developer is available to help me with this one and we walk through almost the same steps that I did. It turns out that if you look at the beginning of the test a certain way and you look at the issue a certain way, then the test looks like it's related to something implied by part of the issue. The weird part is the test has only one assertion related to that and that assertion is the exact opposite of what the issue says.

After about an hour of rereading it and commenting out chunks of the test to see what happens we figure that the last half of the test is completely unnecessary and we delete it — including the failing assertion. Now the test passes and I can move on to the next test.

Friday, June 10, 2011

No longer fit

I recently added long vowels to my latin project. When spoken, long vowels are held a little longer than regular ones and are indicated by putting a little bar, called a macron, above the vowel: ā, ē, ī, ō, or ū. Java can handle these unicode characters if you specify "-encoding utf8" when compiling but apparently fit can not; so any fit tests that were comparing strings that had macrons were failing. I tried changing my code to convert long vowels into regular vowels when comparing but that was messy and didn't always work right since long and regular vowels are different phonemes. Eventually I decided to do away with fit and switch to JUnit. I lost at least two programming sessions to fiddling with fit, working around fit's limits, and finally reimplementing the tests in JUnit.

So even though I was really looking forward to using fit and I think it could solve many problems I've seen with the *Unit frameworks, for me, at least this time, the costs of using fit vastly outweighed the supposed benefits.

Wednesday, June 8, 2011

Finally fit

I'm finally using Fit. I've been interested in it for a long while but I've never had a side project where I really though it would be useful, until lately.

One of my new years resolutions is to learn Latin. And, as usual, while learning that I began thinking about ways to apply it to programming such as an Android app to help someone learn and review Latin. Latin is like everything else invented by people: it's got complicated rules with many exceptions and special cases; perfect fit for TTD and Fit in particular.

I don't have tests around the android specific stuff like moving between actions and handling the gui and persistence, just around conjugating, declining, and translating Latin. Although it's been very helpful it has slowed large scaled refactoring. As I learn more about Latin I have to make major changes to how words and phrases are implemented and then I have to rewrite all the tests so they pass.

It feels helpful but I'll have to wait and see if maintaining all these tests really is worth the extra time and effort.

Wednesday, February 9, 2011

Understanding vs NUnit

Can you tell what MysteriousMethod does from looking at these C# unit tests?
  [Test]
  public void Test1()
  {
    var obj = new MysteriousObject();
    Assert.IsTrue(obj.MysteriousMethod(8));
  }

  [Test]
  public void Test2()
  {
    var obj = new MysteriousObject();
    Assert.IsFalse(obj.MysteriousMethod(11));
  }

Maybe it returns true if the input is even, or a single digit, or if it's not prime, or if it's divisible by 4 or 8, or maybe it only returns true for the number 8, I don't know. All I do know is that it returns true for 8 and false for 11.

Can you tell what mysteriousFunction does from looking at these Haskell QuickCheck properties?

  prop_test1 :: Int -> Bool
  prop_test1 x = x `mod` 4 == 0 ==> mysteriousFunction x

  prop_test2 :: Int -> Bool
  prop_test2 x = x `mod` 4 /= 0 ==> not $ mysteriousFunction x

It looks like it returns true if the input is divisible by four and false if it isn't. Pretty clear to me.



I'm working on a Haskell project and it's reminding me why I like QuickCheck so much. Not only does it do a much better job at catching unexpected cases but the meaning and intention of each function is so clear, much clearer than when using xUnit style testing. Going back to C# and NUnit feels weird; you're basically just comparing hard coded values. I find it hard to tell why one value is used instead of something else, i.e. what's essential and what's accidental, especially when the method being tested has several inputs or other variables it relies on. This can lead to bad assumptions and misunderstanding what something actually does. With QuickCheck there's less room for such confusion because the focus is on properties of the values and not the specific values themselves. The clarity of BDD tools (rspec, etc) seems to help, but I wonder what more can be done.

Thursday, December 23, 2010

Unit tests are not inherently good

Unit tests cause pain. Unit tests, by themselves, do not solve problems, do not make users happy, do not make maintainers happy, do not make a better product, and do not improve design. Unit tests make certain things hurt; nothing more and nothing less. But pain can indicate something is wrong and can lead you to do something else.

The first assumption behind unit tests (and bodily pain) is that if something hurts we'll pay attention and do something else instead. The second assumption is that what we do as an alternative will not only hurt less but make things better. For example, since unit tests make singletons and hard coded dependancies so painful, we can rely on dependency injection techniques and the single responsibility principal. This should cause code that is easier to test and understand and that responds to rapidly changing requirements.

So unit tests are one way to show what hurts and needs attention and avoid blundering into bad and potentially project-killing designs. The problem is when we can't or don't change how things are done. If we must rely on singletons, the filesystem, database, complex api calls, undefined behavior, or slow techniques then writing and running the unit tests will hurt. In these cases the costs of writing and maintaing unit tests can outweigh the benefits of running those tests.