Tuesday, April 16, 2013

Writing tests that works

Disclaimer

Although some of the ideas exposed in here are too obvious to some developers, I wanted to write down some of my thoughts about how the "unit" tests should be implemented in some projects, this is a discussion I'd with a fellow co-worker after reading this article: Unit Testing Myths and Practices and I think worth the time to post it in here, maybe some of you will agree with me, and some could call me dumb, anyway, I would love to read your thoughts about this and discuss about them on the comments section.

My opinion about "Unit" Tests

Several definitions of Unit tests are around Internet and in books, but here's a simple demo of what I understand by unit tests, and the way I saw unit tests implemented in several projects.

int myMethod(int arg1, int arg2) {
   return arg1 + arg2;
}
Unit test:
void testMyMethod() {

    // Testing positive numbers
    int a = 1;
    int b = 2;
    int expected = 3;
    int result = myMethod(a, b);
    ASSERT(expected, result);

    // Testing negative numbers
    a = -1;
    b = 2;
    expected = 1;
    result = myMethod(a, b);
    ASSERT(expected, result);

    // testing zero arguments
    ...

    // testing big integer values
    ...
}

As you may noticed I required 10+ lines of code to test 1 line of the business method. Now, what will happen to our test cases if we want to test the following method:

int myMethod2(Customer customer, int arg2) {
   int age = customer.age();
   return myMethod(age, arg2);
}

Now I will need to write tons of lines to check how my code will react to every single combination of customers' age or any value to be added. For example: test cases to check NULL customer, customers without age, summing up a negative age, etc, etc... Is this what I wanted in the first place? will this ensure that, if a reckless developer changes myMethod to get sum Integers instead of int, avoid null errors in the myMethod2? or do we need to rewrite all the tests that depends on the first method? do I even care about nulls been sent to myMethod?

Dependency injection will save the day!

Maybe some of you thought, "hey this guy does not know a sh**t about coding and he lacks of expertise, this is easily managed by dependency injection and method contracts". Maybe you are right, but lets check how our methods would change to ensure the usage of dependency injection to separate both concepts (bear in mind that the tests are now going to insert a dummy class that will return the proper values for each test).


[di]
IMySumClass sumClass;

int myMethod2(Customer customer, int arg2) {
   int age = customer.age();
   return sumClass.myMethod(age, arg2);
}

Done!, now we have a new problem, both tests are going to pass up the tests phase, and the error will popup in our production system. Yes, I know... the methods' contract... someone would say "the method says that it will receive X and Y and if the X and Y changes then it's a new method because it's a new contract. Ok, here we go:

int myMethod(int arg1, int arg2) {
   return arg1 + arg2;
}

Integer myMethodWithIntegers(Integer arg1, Integer arg2) {
   return arg1 + arg2;
}

Solved!, now we have 2 methods that executes the same logic, therefore we will need to mantain them as well. (But don't forget that our tests cases will need to be maintained as well, so we have now 30+ lines to keep updated). Too much work to ensure that a simple sum will work, don't you think so?

Integrity tests

Ok, let's go back to the simple code we have in the first place:

int myMethod(int arg1, int arg2) {
   return arg1 + arg2;
}

int myMethod2(Customer customer, int arg2) {
   int age = customer.age();
   return myMethod(age, arg2);
}
Unit test:
void testMyMethod() {

    // Testing positive numbers
    int a = 1;
    int b = 2;
    int expected = 3;
    int result = myMethod(a, b);
    ASSERT(expected, result);

    // Testing negative numbers
    a = -1;
    b = 2;
    expected = 1;
    result = myMethod(a, b);
    ASSERT(expected, result);

    // testing zero arguments
    ...

    // testing big integer values
    ...
}

What I found useful is to keep the tests as simple as possible and targeting the class we want to check, so instead of doing a double check of all the different options for a "a + b" operation in the method testMyMethod2, or in the testMyMethod, I would add the tests that are useful to each case, and I will remove the negative checks, zero arguments, etc that will cumbersome my code, also I won't add tests to testMyMethod2 that are not related with myMethod2 itself, therefore I will not add tests that were tested somewhere else. Let's see the sample:

void testMyMethod() {

    // Testing positive numbers
    int a = 1;
    int b = 2;
    int expected = 3;
    int result = myMethod(a, b);
    ASSERT(expected, result);
}

void testMyMethod2() {
    // Customer with age
    Customer c = new Customer("x", "y", 32);
    int expected = 33;
    int result = myMethod2(c, 1);
    ASSERT(expected, result);

    // Test a customer too old (should be rejected because of his age)
    c = new Customer("x", "y", 32);
    try {
       myMethod2(c, 150);
       FAIL("Customer age was not properly checked");
    } catch (AnyCheckedException) {
        // Accepted case
    }
}

Noticed how I removed the "negative", zero and big numbers tests, I really don't care about these cases at this level, and they will be tested by "customer" cases anyway. These kind of tests are going to be more useful and they will protect my code from unexpected "real" failures reducing the amount of work I need to write down all the test cases and covering the business cases instead of arguments misusage.

Off course, this a very simple case, and it would not cover all the "what if" questions, but it will cover what is supported by the system. The last sample test code will check not only cases related to myMethod2, but myMethod as well, this means that myMethod will be checked for every single call made from any of the classes that uses this method, and it's going to be tested in a "business" way, not in a "what if" applied to every single argument.

What usually happens in production environments is that errors, not originally tested, will popup and we will need to add them to the proper test code to ensure they will not rise again, if the error was in the "customer" level then we will add proper lines to the testMyMethod2 (or create a new one), if the problem was with our sum method not covering Big Integers, then we will need to cover it in the customer test as well, any change to the testMyMethod2 will also test myMethod in a business like scenario.

Conclusion

Every project could be different and every one will need a solution that fits to it, but I've been using this heavily and this method of "accumulative" testing have worked really well to check and correct bugs in production systems.

I would love to read your thought, please post a comment. The "you're a dumba**" comments are welcome but please try to support your ideas.

No comments:

Post a Comment