The Testing Anti-Patterns Drafts Vol. 1
George Malamidis, September 23rd, 2006“The Testing Anti-Patterns Drafts” is a collaborative effort between Stuart “Stubs are evil” Caborn and myself, which aims to identify cases of Testing gone bad.
It consists of a single document that will undergo constant enhancements and modifications, in a “pair-authoring” manner, utilizing our respective weblogs as the platform.
We hope to get input from anyone following the document, our goal being to produce an interesting resource for the TDD, or Testing Oriented in general community.
Design Pervasive Testing
Both Stuart and I consider ourselves fortunate enough to work as members of teams where TDD is practiced by default.
There are two reasons beyond the obvious as to why I find it relatively harder to code in a non TDD approach. Test Driving my code enhances my vision on how to design/model/instrument my application’s universe. Also, starting with a test means work begins and ends with coding, not meetings, discussions or modeling our vision in pictures bound to be proven unrealistic when the first coding bottlenecks arise.
Having said that, testing properly is not easy. Past the simplistic examples, proper tests are hard work, often much harder than the actual code.
Encapsulation and Behavior are two of the most important characteristics of Object Oriented design. Applying them properly is difficult, and after all the hard work, testing Classes that are very concise about their behavior and very strict about what their peers can do with them, writing meaningful, readable, fine grained tests becomes a mission.
It’s the extra effort required that sometimes phases developers who end up taking shortcuts in order to facilitate testing.
Let’s look at an example:
public void testShouldProduceComplexObject() {
ComplexObject expected = ObjectMother.getComplexObject();
ComplexObject actual = classUnderTest.getComplexObject();
assertEquals(expected, actual);
}
Obviously, such an assertion would only be valid if ComplexObject overrides the hashCode() and equals() methods. Countless times have I witnessed Classes implementing these two methods only to accommodate tests similar to the aforementioned example.
This is terminally wrong and a potential bug. Equality is an integral attribute of a Class’s behavior and should be strictly meaningful under the context of the Class’s definition.
I’m not aware of an easy way out of this sticky situation. Specific fields of the Class need to be tested for equality in isolation.
Does this mean that we need to expose all the Class fields we need to test by giving them public, or package level access? Not necessarily. In fact, not at all. After all, changing access levels would further compromise our Class’s behavior.
More and more, we have discovered Reflection to be an invaluable tool in our quest for less intrusive to purist OO design testing:
public void testShouldProduceComplexObject() {
ComplexObject expected = ObjectMother.getComplexObject();
ComplexObject actual = classUnderTest.getComplexObject();
assertEquals(getPrivateField(expected, "fieldOne"),
getPrivateField(actual, "fieldOne");
assertEquals(getPrivateField(expected, "fieldTwo"),
getPrivateField(actual, "fieldTwo");
//etc...
}
If this were a container-bean type of Class with fifteen fields, that’s a lot typing we’d have to deal with. This is not necessarily bad for a Unit Test, but we could all do with less sore fingertips.
public void testShouldProduceComplexObject() {
ComplexObject expected = ObjectMother.getComplexObject();
ComplexObject actual = classUnderTest.getComplexObject();
String[] fields = new String[] { "fieldOne", [...], "fieldFifteen" };
for (String field : fields) {
assertEquals(getPrivateField(expected, field),
getPrivateField(actual, field);
}
}
TBC…
If you’d like to follow the discussion, feel free to grab The Testing Anti-Patterns Drafts feed.
September 23rd, 2006 at 6:32 pm
Hi George,
I read your post and totally agree. I always had this doubt about equals/hashCode and object behavior. Thanks a lot!!
I added some comments at my blog: http://www.jroller.com/page/alexRuiz?entry=don_t_break_oo_in
Best regards,
Alex.
September 24th, 2006 at 12:53 am
I think the underlying design problem is “Complex Object”.
What is a specific example of a reasonably scoped Class that should not have equals/hashCode behaviour based on values?
Alex mentioned Hibernate in his blog entry but that’s only because Hibernate is forcing your object’s equals definition to be an entity equals instead of a values equals.
I’m thinking more that Java should have Object.equals() and Object.same()
September 24th, 2006 at 10:53 am
Using reflection to assert equality etc of fields private to the class under test is itself an anti-pattern! Test drive the public api and don’t expose the guts of a class, even if you are using reflection to do it.
September 24th, 2006 at 3:34 pm
Slightly off-topic, but if I need a class like ObjectMother, I prefer to name the methods “make…” or “create..” rather than “get…”. The get prefix is heavily overused and most people expect the semantics to be that I call getComplexObject() twice in a row I’ll get the same object, which probably isn’t true in this case.
September 24th, 2006 at 10:32 pm
I’m pretty sure I agree with Damian - there’s little difference between a public field and a private one accessed via reflection. It may well be that coupling the tests so tightly to their class in this way leads to a better design, but I am not convinced.
Jim
September 24th, 2006 at 11:25 pm
>Alex mentioned Hibernate in his blog entry but that’s only because Hibernate is forcing your object’s equals definition to be an entity equals instead of a values equals.
>I’m thinking more that Java should have Object.equals() and Object.same()
It is not just Hibernate that requires this behavior, the JCF does too, which is integral to java. Java already has == for references, equals is meant to test semantic equivalence. What could same() possibly test for? If you wanted different levels of equality, then declare your own.
btw, Junit-objects offers reflective assertion via expressions on beans:
http://junit-objects.sourceforge.net
September 24th, 2006 at 11:33 pm
I agree with Damian Guy.
Through interfaces (or even those crazy superclass things), one object can have many public members, but they aren’t accessible everywhere (without crazy casting).
So if you have ComplexObject, an interface, you could have your implementation expose the fields, and only have your automated tests call the implementation directly.
A further problem is ‘exhausting testing’. I don’t mean ‘exhaustive testing’. It’s impossible to write complete (exhaustive) tests for most code. You end up testing some things, not quite at random, but often just according to what’s easiest to test.
In fact, you can end up just duplicating the ‘real’ code. I’d suggest instead to make sure that the tests exercise code that is used by users, i.e., don’t write tests that test things that can’t happen in a live environment.
That doesn’t mean you shouldn’t write tests that probe behaviour under strange situations, but you shouldn’t bother doing that if those strange situations can be filtered out instead. Instead of *exhausting* yourself trying to write exhaustive tests, write better application code.
September 29th, 2006 at 7:44 pm
The issue of what constitutes equality between objects is fundamental to OO design. I just don’t see how it’s something that should be avoided.
Admitted, in a unit test on the object I may want to test that when I say a.equals(b) that the equality is determined by a set of _particular_ fields.
I completely agree with you when you say ‘Equality is an integral attribute of a Class’s behavior and should be strictly meaningful under the context of the Class’s definition.’, but, I’m not sure I see why tests that assertEquals are wrong, if indeed you are trying to assert an object’s equality. Your tests are revealing your intention for the code, and there’s just no getting around the fact your looking for equality.
November 14th, 2006 at 9:37 pm
+1 To Damiens Comment - Using reflection to test protected/private memebers is going to product very fragile tests…It is easier to describe good practices and bad practices when you create a real test in your example - What are you trying to test?
November 14th, 2006 at 11:20 pm
Matt,
The assertion is made on the value of the private field being the one expected after the method under test has been invoked. If the value is not what it’s expected to be, the test will break as it should. If a field with that name is no longer there, the test will break. As is should.
I would suggest there is no such thing as a ‘fragile test’. Tests are meant to break when the behavior they describe is not met.
In terms of good and bad practices:
When you want to test the value of a private field, you have three options:
- Expose a public getter on it or bend your application code to accommodate access, thus breaking encapsulation.
- Not test it, thus leaving a window for a potential bug to creep in.
- Reflect and assert on it.
To everyone who gets phased by reflection:
If you’ve ever used Spring, WebWork, Hibernate, JMock (… and the list goes on and on, way beyond Java frameworks), I’ve got bad news for you: Your applications are ‘fragile’, because most frameworks out there use reflection all over the place.
Just because reflection is dangerous in the hands of inexperienced developers, as are most powerful techniques, it doesn’t mean it’s bad. Fire burns, but there’s a million useful things you can do with it if you know how.
Don’t buy the reflection FUD just because somebody told you they got burned by it. Learn how to use it properly instead.