Net Objectives

Net Objectives
If you are interested in coaching or training in ATDD or TDD please click here.

Friday, December 11, 2015

Specifying The Negative in TDD

One of the issues that frequently comes up is "how do I write a test about a behavior that the system is specified not to have?"  It's an interesting question given the nature of unit tests.  Let's examine it.

The Decision Tree of Negatives


When it comes to behaviors that the system should not have, there are different ways that this can be specified and ensured for the future:

Inherently Impossible


Some things are inherently impossible, depending on the technology being used.  For example you cannot write to read-only memory.  This is in the nature of the memory and thus does not require a specification (nor a test, since that would be a test that could never fail).  In languages like C# and Java, there exists the concept of “private”, and we know that an attempt to read or write a private value from outside a class will not compile and so will never exist in the executable system. 

Some things are inherently impossible and cannot be made possible even accidentally.  Read-only memory cannot be made writable.  However other things which are impossible by nature can be made possible if desired.  A good example of this is an immutable object.

Let's say there exists in our system a SaleAmount class that represents an amount of money for a given retail sale in an online environment.  Such a class might exist in order to restrict, validate, or perfect the data it holds.  In this case, however, there is a customer requirement that the value held must be immutable, for reasons of security and consistency in their transactions. 

This brings up the question "how do I specify in a test that you cannot change the value?"
How can we test-drive such an entity when part of what we wish to specify is that the value, once established in an instance of this class, cannot be changed from the outside?  A typical way this questions is stated is "how can I show, in a test, that there is no SetValue() method?  Any test that references such a method simply will not compile because it does not exist.  Therefore, I cannot write the test.”

Developers will sometimes suggest two different ideas:
  1. Add the SetValue() method, but make it throw an exception if anyone ever calls it.  Write a test that calls this method and fails if the exception is not thrown.[1]  Sometimes other actions are suggested if the method gets called, but an exception is quite common.
  2. Use reflection in the test to examine the object and, if SetValue() is found, fail the test.

The problem with option #1 is that this is not what the requirement says, it is not what was wanted.  The specification should be "you cannot change the value" not "if you change the value, thing x will happen."  So here, the developer is creating his own specification and ignoring the actual requirements.

The problem with option #2 is twofold:  First, reflection is typically a very sluggish thing and in TDD we want our tests to be extremely fast so that we can run them frequently without this slowing down our process.  But even if we overcame that somehow, what would we have the test look for?  SetValue()PutValue()ChangeValue()AlterValue()? The possibilities are vast and the cost of fully verifying immutability, in this case, would be enormous compared to the value.

The key to solving this is in reminding ourselves once again that TDD is not initially about testing but creating a specification.  Developers have always worked from some form of specification it's just that the form was usually some kind of document.

So think about the traditional specification, the one you're likely more familiar with.  Ask yourself this: Does a specification indicate everything the system does not do?  Obviously not, for this would create a document of infinite length.  Every system does a finite set of things, and then there is an infinite set of things it does not do.

For example, here is an acceptance test for the positive requirement [2]:

Given: A SaleAmount S with value V
When: You ask for the value of S
Then: V is retrieved

This could be made into an executable specification by the following simple test:

[TestClass]
public class SaleAmountTest
{
    [TestMethod]
    public void TestSaleAmountPersistence()
    {
        var initialValue = 10.50d;
        var testDollar = new SaleAmount(
initialValue);

        var retrievedValue = testDollar.GetValue();

        Assert.AreEqual(retrievedValue, initialValue);
    }
}


Which would drive the entity and its behavior into existence:

public class SaleAmount
{
    private double myValue;
    public SaleAmount(double aValue)
    {
        myValue = aValue;
    }

    public double GetValue()
    {
        return myValue;
    }
}


Ask yourself the following question:  If we were using the TDD process to create this SaleAmount object, and if the object had a method allowing the value to be changed (SetValue() or whatever), how would it have gotten there?  Where is the test that drove that mechanism into existence?  It's not there because there is a specific requirement that it not be there.  In TDD we never add code to the system without having a failing test first, and we only add the code that is needed to make the test pass, and nothing more. 

Put another way, if a developer on our team added a method that allowed such a change, and did not have a failing test written first, then he would be ignoring the rules of TDD and would be creating a bug as a result.  TDD does not work if you don't do it.  We don't know of any process that does. 

And if we think back to the concept of a specification there is an implicit rule here, which basically has two parts.

1.    Everything the system does, every behavior, must be specified.
2.    Given this, anything that is not specified is by default specified as not a behavior of the system. 

If it is a behavior nonetheless it is a defect.

 

Inherently possible


We don’t have a test that shows the value being changed, so it cannot be.  But this does not mean we have a “test for immutability.”  Anything that comes from the customer must be retained; we never want to lose that knowledge.  So if we think of this requirement in terms of acceptance testing we could express it using the ATDD nomenclature:

Given: A SaleAmount S with value V exists in the system
Then: You cannot change V

There is no “When” in this case because this is a requirement that is always true, it is not based on system state.  But this, of course, implies a strongly-typed, compiled language with access-control idioms (like making things "private" and so forth).  What if your technology does not provide this?  What if it is an interpreted language, or one with no enforcement mechanism to prevent access to internal variables?

The first answer is: You have to ask the customer.  You have to tell them that you cannot do precisely what they are asking for, and consider other alternatives in that investigation.   It may well be that we are using the wrong technology.

The second answer is that there will be some occasions where the only way you can ensure that an illegal or unwanted behavior is not added to a system accidentally is through static analysis (a traditional code review, or perhaps a code analysis tool).  This is still “a test” but one that either cannot or should not be automated in all cases.

On the other hand, sometimes we can make an inherently possible thing impossible by adding behaviors.  Such behaviors must, of course, be test driven.

Let's add a requirement to our SaleAmount class.  If the context of this object was, say, an online book store, the customer might have a maximum amount of money that he allows to be entered into a transaction.

We used a double-precision number [3] to hold the value in SaleAmount. A double can hold an incredibly large value inherently.  In .net, for example, it can hold a value as high as 1.7976931348623157E+308 [4].  It does not seem credible that any purchase made at our customer's site could total up to something like that!  So the requirement is: Any SaleAmount object that is instantiated with a value greater than the customer's maximum credible value should raise a visible alarm, because this probably means the system is being hacked or has a very serious calculation bug.

As developers, we know a good way to raise an alarm is to throw an exception.  We can do that, but we also capture the customer's view of what the maximum credible value is, so we specify it.  Let's say he says "nothing over $1,000.00 makes any sense".  But... how much "over"?  A dollar?  A cent?  We have to ask, of course.  Let's say the customer says "one cent".

In TDD everything must be specified, all customer rules, behaviors, values, everything.  So we start with this:

Given: The system
Then: The Maximum value for a Sale Amount is $1000.00

We also have to capture the tolerance in its own specification:

Given: The System
Then: Tolerance for comparing SaleAmount to its Maximum is one cent

These tests establish bits of domain-specific language that can then be used in any number of other specifications (we won’t have to repeatedly define them whenever we make comparisons).

[TestMethod]
public void SpecifyMaximumDollarValue()
{
    Assert.AreEqual(1000d, SaleAmount.MAXIMUM);
}

[TestMethod]
public void SpecifyMaximumDollarValue()
{
    Assert.AreEqual(.01, SaleAmount.TOLERANCE);
}


In order to get these to pass we drive the Maximum and the Tolerance into the system.
Now we can write this test, which will also fail initially of course:

Given: Value S greater than or equal to Maximum + Tolerance
When: An attempt is made to create a SaleAmount with value S
Then: A warning is issued

[TestMethod]
public void TestUSDollarThowsUSDollarValueTooLargeException()
{
    var saleAmountMaximum = SaleAmount.MAXIMUM;
    var tolerance = SaleAmount.TOLERANCE;
    var excessiveAmount = saleAmountMaximum + tolerance;

    try
    {
        CreateSaleAmount(excessiveAmount);
        Assert.Fail("SaleAmount created with excessive"+"
                    "
value should have thrown an exception");
    }
    catch (SaleAmountValueTooLargeException)
    { }
}


But now the question is, what code do we write to make this test pass?  The temptation would be to add something like this to the constructor of SaleAmount:

if(aValue => MAXIMUM + TOLERANCE) 
          throw new SaleAmountValueTooLargeException();

But this is a bit of a mistake.  Remember, it's not just "add no code without a failing test", it is "add only the needed code to make the failing test pass."

Your spec is supposed to be your pal.  He's supposed to be there at your elbow saying "don't worry.  I won't let you make a mistake.  I won't let you write the wrong code, I promise."  He's not just your pal, he's your best pal. 

Here, however, the spec is just a mediocre friend because he will let you write the wrong code and say nothing about it.  He’ll let you get in your car when you are in no condition to drive.  He'll let you do this, and let it pass:

throw new SaleAmountValueTooLargeException();

There is no conditional.  We’re just throwing the exception all the time.  That's wrong, obviously. This behavior has a boundary (as we discussed in our blog about test categories) and every boundary has two sides.  We need a little more in specification.  We need something like this:

try
{
    new SaleAmount(SaleAmount.MAXIMUM);
}
catch (SaleAmountValueTooLargeException)
{
    Assert.Fail("SaleAmount created with value at the maximum"+
                "should not have thrown an exception");
}


Now the "anAmount => MAXIMUM + TOLERANCE" part must be added to the production code or your best buddy will let you know you're blowing it.  Friends don’t let friends implement incorrectly. 
...
[1] There are a variety of ways to do this.  We’ll show one way here a bit further on.
[2] [TODO] Link to ATDD blog
[3] If you’re thinking “you used the wrong type, a long would be better” it’s a fair point.  We simply wanted to make the conceptual point that primitives do not impose domain constraints inherently, and the use of the double just makes the idea really clear.
[4] For those who dislike exponential notation, this is:
$179,769,313,486,231,520,616,720,392,992,464,536,472,240,560,432,240,240,944,616,576,160,448,992,408,768,712,032,320,616,672,472,536,248,456,776,672,352,088,672,544,960,568,304,616,280,032,664,704,344,880,448,832,696,664,856,832,848,208,048,648,264,984,808,584,712,312,912,080,856,536,512,272,
952,424,048,992,064,568,952,496,632,264,936,656,128,816,232,688,512,496,536,552,712,648,144,200,160,624,560,424,848,368
...and no cents. :)


10 comments:

  1. Hi, thanks for the post. A lot of foood for thought, so I will need to re-read at least once more, however, one small comment on using reflection to check whether things are immutable.

    1. I agree it cannot be checked, even with reflection. I once wrote a custom assertion that made a best-effort attempt to check immutability in C#. The way I tried to do that was, among other small checks, to ensure all the fields of a class are readonly and private. This is not enough, however, since someone may say "private readonly List<int> something" and we have mutable state anyway.

    2. I always wrestled with the issue whether I should be stating such things in tests. I still think documenting some design constraints might be useful, especially that immutable objects are usually so called "value objects" and they should be handled, maintained and extended in a different manner than the usual objects that "provide services" (sorry, don't know how to say it better, I hope you know what I mean). On the other hand, I believe this is something that the language should support with its type system.

    3. I don't see a big performance issue in using reflection in unit tests (I'm not commenting here about whether it makes sense or not). There is a lot of reflection used in unit tests already, e.g.
    a) unit test runner uses it to detect test methods, create test classes etc.
    b) dynamic mocking libraries use reflection for creating mocks and setting up calls.
    c) tools for generating test values, such as AutoFixture, do use reflection as well

    Which does not mean that reflection cannot be abused.

    Thanks you very much for breaking down the different cases into a tree. I still have to think it through, however, I feel it will bring a lot clarity to my thinking about "stating the impossible".

    ReplyDelete
    Replies
    1. Helpful comments, Grzerorz, much appreciated. It is interesting to note how the mechanics of TDD can be effected by the specifics of the language we use to implement it. Languages like C++, which were created long before anyone though about testing as a developer's activity, can present unique challenges compared to others, like Ruby, that were created with TDD in mind from the outset.

      In terms of "whether something should be stated in a test" my general answer is... would you put it into a traditional specification? If so, then generally I would say yes.

      Delete
  2. Why did you use SalesAmount in the example? It would appear that a Dollar class is more appropriate for some of the things you are testing. Are you going to show a future example of how you eliminate redundancy when you add an ItemAmount and refactor both to use a Dollar class?

    ReplyDelete
  3. Why did you use SalesAmount in the example? It would appear that a Dollar class is more appropriate for some of the things you are testing. Are you going to show a future example of how you eliminate redundancy when you add an ItemAmount and refactor both to use a Dollar class?

    ReplyDelete
    Replies
    1. That was Amir's change. You probably should ask him directly... :)

      Delete
    2. Haven't heard from him :)

      Delete
  4. I agree with much of what Grzegorz Gałęzows says in his comment. Part of the reason for directly accessing the unit under test is to test its API.

    Let’s take your example. One starts with:

    mathUtils.ArithmeticMean(anyFirstValue,anySecondValue);

    and now you decide you need a method that takes any number of values.
    You can change the API in at least two ways:

    1.) Add an additional method that takes an array (or some collection)

    mathUtils.ArithmeticMean(int [] values );

    If you do not get rid of the current method, you do not need to change anything in your tests.

    Inside of mathUtils, the two value version can call this version, so there is no redundancy in implementation.

    2.) Add this additional method and then take away the two-value one. That means that anyone calling the two value version (particularly production code) has to change.

    The fact that you have to change tests indicates that you are changing an API which means that production code has to change. That suggests that maybe you want to keep the original method.

    If you did not keep that two-value method, then production code would probably create a convenience method to avoid redundant code. Where else to better put that convenience methods but in MathUtils?

    You suggest that:

    “Now if we change the ArithmeticMean() method to take a container rather than discrete parameters, or whatever, then we only change this private helper method and not all the various specification-tests that show the behavior with more parameters, etc...”

    I would suspect that you are over-testing ArithmeticMean () if it is getting called from lots of places. And even if these extra tests were not redundant, than those tests suggest that there will be lots of production code that has to change .

    There are tradeoffs in everything and how you see tradeoffs is based on your experiences. To me, adding another layer of abstraction to every class under test would have little benefit and much cost. Your experience may differ and I’d really like to see real life examples of where this has paid

    ReplyDelete
    Replies
    1. You put this comment on the wrong entry, Ken....

      Delete
  5. I had three tabs open on your site. That must have confused things.

    ReplyDelete
  6. This comment has been removed by a blog administrator.

    ReplyDelete