100% code coverage, Hibernate Validator and Design by Contract
Join the DZone community and get the full member experience.
Join For FreeCode coverage in unit testing was one of the first things I learned in
my software engineering career. The company I worked at taught me that
you should have 100% coverage as a goal, but achieving it does not mean
there are no bugs in the system. At that time, I worked at a company
whose big thing was that they delivered very reliable billing
software to telecomms operators. We used to invest as much time writing
unit tests as we did writing the code. If you included unit tests,
integration tests, system tests and acceptance tests, more time was
spent testing than designing and implementing the code which ran in
production. It was impressive and I have never worked with or for a
company with that kind of model since, although I am sure there are many
companies which operate like that.
The other day I was thinking back to those days and reading up about
unit testing to refresh myself in preparation for a unit testing course
I'm attending soon (don't want the instructor to know more than me!) and
I started to wonder about what kind of code could be fully covered
during unit testing, but which could still contain a bug. While I
learned that 100% coverage should be the goal, in over 10 years I have
never worked on a project which achieved that. So I have never
surprised to find a bug in code which I thought was "fully" tested.
It's fun write whacky code - you don't normally try and write bugs :-) I started with the specification:
/** * @return String "a" or "b" depending upon the values * passed to the init method. If variable "a" is true, * then string "a" is returned. If variable "b" is true, * then "b" is returned. If neither is true, then "" is * returned. Variable "b" is important than "a", so if * both are true then return "b". */
Now, the tricky part was to write code which contained a bug yet was
easy to cover with incomplete tests. I came up with the following. The
specification above is for the "getResult()" method.
public class SomeClass { private boolean a; private boolean b; public SomeClass init(boolean a, boolean b) { this.a = a; this.b = b; return this; } public String getResult() { String s = ""; if(a) { s = "a"; } if(b){ s += "b"; // <-- oops! } return s; } }
The "bug" is in the method "getResult" and the problem is that instead
of just an assignment, the "plus equals" operator has been used. An
"else" would probably make the code a little safer too. But I think
code like this is quite typical of the buggy code that finds its way
into productive systems. Even the best programmers have lapses of
concentration where they write typos like this (especially in open plan
offices!).
The unit test which a programmer trying to achieve 100% coverage, would look something like this:
@Test public void test() { assertEquals("a", new SomeClass().init(true, false).getResult()); assertEquals("b", new SomeClass().init(false, true).getResult()); }
Using Emma for Eclipse
I measured complete coverage. But wait! There is still a bug. The
code does not do what it says in the Javadoc spec. If I initialise the
object with "true, true", then the result will be "ab", because of the
"plus equals" operator. So even though I have 100% coverage, I still have a bug.
I asked myself what that means to me as a programmer. What do I need to
look out for, when writing tests. Imagine the code above was tucked
away among hundereds of other lines of code, then the chances of seeing
it are really quite small. The test wouldn't be just two lines long and
the problem wouldn't be jumping out of the page.
One way to look at the problem is to say that there is a bug because the
code isn't fulfilling its contract. OK, I use "contract" in the loose
sense of the word, but the Javadoc is basically a contract. It tells
anyone calling that method what to expect, yet the codes is not doing
what the user expects.
So perhaps one solution is to not only entirely exercise the code, but
entirely exercise the contract? Is there a way to translate the Javadoc
contract into something more concrete which the testing tools can help
me check? Yes, namely my using some kind of Design (or Program) by
Contract (DBC or PBC) framework. JSR-303
isn't strictly DBC, but close. It lets you use annotations to state
your expectations about parameters passed to methods, as well as your
expectation about the result being returned. You can create your own complex constraints quite easily. I added the following annotations to my code to help in my quest for bug free code:
@Valid @NotNull @Size(min=0, max=1) public String getResult() { ...
Now, method validation (validating method calls, rather than validating
the bean itself) is something which comes as an extra in Hibernate
Validator, and which really isn't part of JSR-303 - it's only described
in Appendix C as optional. To test this, I used Google Guice to add an
AOP interceptor to any methods marked with the @Valid annotation, and in that interceptor, I call the Hibernate Method Validator. I end up with something like this:
. . . Injector injector = Guice.createInjector(new AbstractModule(){ protected void configure() { bindInterceptor(Matchers.any(), Matchers.annotatedWith(Valid.class), new MethodValidatorInterceptor()); } }); SomeClass someClass = injector.getInstance(SomeClass.class); someClass.init(true, false); assertEquals("a", someClass.getResult()); someClass = injector.getInstance(SomeClass.class); someClass.init(false, true); assertEquals("b", someClass.getResult()); . . . public class MethodValidatorInterceptor<T> implements MethodInterceptor { public Object invoke(MethodInvocation invocation) throws Throwable { //validate all inputs Set<MethodConstraintViolation<T>> mcvs = methodValidator.validateAllParameters((T)invocation.getThis(), invocation.getMethod(), invocation.getArguments()); if(mcvs.size() != 0){ throw new IllegalArgumentException(String.valueOf(mcvs)); } //call through to the actual method being called Object ret = invocation.proceed(); //validate result mcvs = methodValidator.validateReturnValue((T)invocation.getThis(), invocation.getMethod(), ret); if(mcvs.size() != 0){ throw new IllegalArgumentException(String.valueOf(mcvs)); } return ret; } } . . .
The above is something which a (Java EE) container should do for me - I
was just messing around with simple classes in a simple Java Project.
Now, it isn't quite complete, because I still have 100% coverage, and I
still have that bug, because the new annotations haven't really done
anything useful. Well that isn't entirely true - the reader of the code
knows that the contract is a little stricter than it was when it was
simple Javadoc. The reader may assume that the system will check these
constraints when the method is called. But while there is still a bug,
I've laid the path for adding some full pre- or post-conditions. The
next step was to add a new annotation, an interface and make use of them
in the interceptor.
@Target( { ElementType.METHOD }) @Retention(RetentionPolicy.RUNTIME) @Documented public @interface PreCondition { Class<? extends ConditionChecker> implementation(); } /** Any pre- or post-condition is written in * an implementation of this interface */ public interface ConditionChecker { void checkCondition() throws ConstraintViolationException; }
The annotation can be added to the business code, in this case to add a
pre-condition. I created a similar annotation for post-conditions.
When I add the pre-condition to a method, I also state which class
contains the code for checking that pre-condition:
@Valid @NotNull @Size(min=0, max=1) @PreCondition(implementation=MyPreAndPostCondition.class) public String getResult() { ...
The interceptor can check for the presence of such a pre-condition
annotation before calling through to the method being called. If the
annotation is found, the interceptor attempts to create an instance of
the implementation class, and calls it's "checkCondition" method.
So the final part of this puzzle is to write a pre-condition which will help me to fail
to get 100% coverage when I test with the test shown near the top of
this post. Here it is, implemented as a static final inner class inside
the SomeClass class, so that it has access to the fields "a" and "b":
public class MyPreAndPostCondition implements ConditionChecker { @Override public void checkCondition() throws ConstraintViolationException { //im going to list all combinations of //a and b which I support if(!a && b) return; if(!b && a) return; if(!b && !a) return; if(a && b) return; } }
When I now test my code, I no longer get 100% coverage, because the last
two lines in the pre-condition are not covered. Hooray! The tools can
now tell me what I still need to test...
Now, while this technical solution works, I think it is really ugly.
Like I said, if the code which is causing the problem were to be found
within one or two hundred other lines of code, and were reliant on
local variables rather than fields in my class, I would have no chance
of using a pre-condition like this to help me locate the problem.
So to sum up, DBC isn't going to help me solve the problem that 100%
code coverage can still contain errors. I think DBC frameworks (and
there are a lot out there, some which do exactly what I have done here
using the @PreCondition annotation) help to make contracts
more concrete. If you use method validation from Hibernate Validator,
you don't have to write as much Javadoc, because the reader knows that
the container will give up before calling the method if anything fails
to validate. To me as a programmer, that is much more satisfying than
praying that some Javadoc reflects what I have really implemented.
I have known programmers who don't write tests because a DBC framework
is in place and that makes them feel safe. But just because you declare
a contract, does not mean the code actually works. Whether the code
fails hard with a validation exception or at some time later because
your code is buggy, makes no difference - both are inacceptable! From
that perspective, DBC contracts are simply hints to the tester what
could be useful tests, and they ensure that the code fails hard, early.
While I was refreshing my testing skills, I also learned the difference
between mocks and stubs. For years I had always thought they were the
same thing, but it turns out that stubs return pre-fed answers, whereby
mocks check the sequence of calls to them too. On a different thread
at DZone, someone made a point that unit testing was pointless because
it never helped them find bugs, and it caused lots of work when
refactoring, because all that does is break their tests. I'd say that
this is simply a question of black box versus white box testing. Black
box unit testing should never break if you refactor your code - the
tests are simply clients to the code which you are refactoring, and
tools like Eclipse will modify the calling code if you modify the
interfaces being called, including tests. You can get pretty good
testing results by just using black box tests - the large majority of
the tests I write are black box tests and when I write such tests, I
tend to have bug free code.
I've talked about writing contracts to help the reader determine what
they should expect when they use your code. Unit tests themselves work
similarly, because they show the reader examples of how to call your
code and what to expect when your code changes system state. They hint
to the reader how the writer intended the code to be used. While I
don't advocate TDD (perhaps only because I have never been on a project
which used it or at a company which valued quality enough to warrant
TDD), I do encourage writing tests and using validation and
pre-/post-conditions because they help document the code with the added
bonus of finding the occassional bug. At the same time, I am an
architect and we need to keep things like budgets in mind. You have an
influence on your budget when you estimate, assuming you are allowed to
do that. Your customer might push you to reduce your estimates, and
that is an indication about their commitment to quality, because they
won't budge on scope or delivery dates! So write as many tests as you
can, within your budget and start with the code which is most complex
and which gets called most often. and remember, 100% coverage is not
really your best friend because bugs may still lurk.
Â
From http://blog.maxant.co.uk/pebble/2011/11/28/1322465377854.html
Opinions expressed by DZone contributors are their own.
Comments