Testability vs. Encapsulation

I'm always on the side of Testability, but I'd like to hear some other peoples opinions on the subject. I think (in most situations) its totally OK to relax encapsulation in favor of testability, but some people at my work do not. Let me give a rather long example of something I came across that bothered me a bit.

I was trying to test a class - well call it G, with a Logger - LOG , which was declared like this:

static final Logger LOG = Logger.instance();

Later on in G, I found a method shutdown, which called LOG.logAndExit() and logAndExit was final and called System.exit().

G:
public void shutdown(){ LOG.logAndExit( "shutting down" ); }

Logger:
public final void logAndExit( String message ){
System.out.println(message);
System.exit(0);
}

If my tests wanted to test the shutdown method on G, the entire JVM would shutdown which simply shutdown my tests. Brilliant. I had to find some way to Mock or override the LOG variable. But there were several problems with that.

1. It was private
   
2. It was final
3. It was static
   
4. The logAndExit method was final

All of these things make difficult testing. I tried to get around the "private" by writing my PrivateFieldHelper Class. But, as it turns out, you cannot change the accessibility of static final variables at Runtime. It only works for instance fields of all types, and non final static variables. So I was stuck with the LOG object that I had.

Unless of course I relaxed encapsulation and removed the final from LOG. Then I could use my PrivateFieldHelper to set it to a new Logger of some kind. BUT...I still had a problem.
The logAndExit method was final. So even if I extended our Logger class and tried to override logAndExit so that it wouldn't call System.exit(), I still could not do so. Even when I created a Mock Logger object using JMock I had the same problem. It appears that even mock objects can't override final methods.

So once again I decided it was best to relax encapsulation. I removed the final keyword from Logger logAndExit, and created a new class - SafeLog - that overrode the logAndExit method. I used my PrivateFieldHelper to set the (now only static private) LOG field on G, and I was able to safely call the shutdown method from my test code.

What a pain.


I understand that you can go way too far on this. Some people say you should never relax encapsulation for testability, because in doing so you relax intent and readability which later creates more of a maintenance problem. In some ways I do agree with this. On public API's and Libraries you most certainly will have higher maintenance costs. But, I do think removing a final here and a final there is ok, especially if its documented. I also think removing private in favor of default (package private) is ok.

What do you think? Does anyone know of any good articles or books explaining the trade-offs?

God this post is about to get long....

Some say you shouldn't relax private for package private, and all testing should be done through public methods. This is another one I just don't agree with. Lets say you have a reasonably complicated class that only exposes one public method. People reading the class later on might not know what inputs are valid for and what outputs are expected for each of the private methods in the class. You certainly can, and should get 100% code coverage through testing public methods, but it still might not be immediately obvious to someone reading the code later on what those private methods are doing.

Testing private (or package private) methods extensively should make it immediately obvious. Once again, What do you think? Does anyone know of any good articles or books explaining the trade-offs?

I think there can and should be things built into the languages themselves to expose hidden members to testing. Something like a test keyword. Or like JSR 294, superpackages, which define what classes in a package are accessible, and to whom. If it things like this were built right into the language, exposing members to testing, then we wouldn't even be having these debates.

How would it work? Maybe a lot like Generics. All the type information is removed at compile time, and so could any test availability information. You could turn this off. You could say, produce a jar for testing, and produce a jar for delivery. Its not that hard. Just ideas...You have any?

CPU Simulator Library in Progress

I've been working on a lot of stuff lately and not writing. For that I apologize. I want to keep everyone updated on what I'm doing, but mostly I want to keep myself update for tracking progress and what not.

I'm still wanting to design languages, write compilers, write IDE thingies, and all of that. If you recall, I decided that in order to do that I should become a compiler master. In reading a lot about compilers I decided I should take yet another step back to refresh my memory on Computer Architecture.

So I've been reading about that a LOT. Really low level, as low as possible. When I say as low as possible I even mean Quantum Physics. But, I don't intend to become a master in Quantum Physics. Its so fascinating but it also really almost drove me to insanity about 6 years ago. A simple refresher was all I needed.

After that I read a bunch about the history of computers, mixed in with some history of philosophy, and mathematics. All really great stuff. Once again I don't intend to become an expert in it but I want solid foundations in all this stuff, which I feel I don't quite have. I'm close.

So heres where the fun part begins. I started writing this cool little CPU simulator in Java. I don't have much yet but I'll keep the world informed. What I plan to do is - write classes that represent pretty much everything you'll find in a computer hardware. So far I have a MemoryModule class and things like that. You can imagine this pretty easily.

We can start with a Computer class which has CPU, Memory, IO, and a Bus. We can then break the CPU down into ControlUnit, ALU, Registers, and maybe Connections. The ControlUnit can be broken down further into SequencingLogic, Registers, Decoders, ControlMemory, etc...

This process of drilling down can probably go down to the atomic level. I'm not sure how low I'd go, where I'd draw the line. But the point is by doing all this, by actually writing the code, ensures that I understand each part. I could take some parts down further than others, and in doing so I'd learn more about that component. This is fine. I probably don't need to know things down to the atomic level on say, video. But going down to FullAdders and HalfAdders in the ALU, or even down to LogicGates is probably a useful thing to do.

Maybe I can invent my own instruction set for my "Hardware", or reuse an existing one. Either way, once all of this is done it would be nice to write a little OperatingSystem class that makes use of that instruction set. I would learn a lot in doing so I imagine. Then once my OperatingSystem is in a reasonable state (I don't have any idea what that would look like yet), I could attempt to write a little compiler that compiles some random language down to an Assembly Language that runs on my hardware.

This is a long way off but I think its a damn good goal. Yes its reproducing things that people have done 50 years ago in a way that most certainly won't be reusable, but whatever, it will get me towards my goal of language writing.

PrivateFieldHelper Class

Heres a little class I wrote to set private fields. I plan to use it a little for testing, but only when I'm backed into a corner, and can't change code otherwise.

/**
 * 
 * @author Josh Cough
 * 
 */
public class PrivateFieldHelperImpl implements PrivateFieldHelper {

 Class clazz;

 /**
  * 
  * @param c
  */
 public PrivateFieldHelperImpl(Class c) { this.clazz = c; }

 /*
  * (non-Javadoc)
  * @see com.joshcough.reflect.PrivateFieldHelper#setStaticFieldValue(java.lang.String, java.lang.Object)
  */
 public void setStaticFieldValue(String fieldName, Object newValue)
   throws IllegalArgumentException, NoSuchFieldException {
  setPrivateFieldValue(findPrivateStaticField(fieldName), null, newValue);
 }

 /*
  * (non-Javadoc)
  * @see com.joshcough.reflect.PrivateFieldHelper#getStaticFieldValue(java.lang.String)
  */
 public Object getStaticFieldValue(String fieldName) 
  throws NoSuchFieldException, IllegalArgumentException  {
  return getPrivateFieldValue(null, findPrivateStaticField(fieldName));
 }

 /*
  * (non-Javadoc)
  * @see com.joshcough.reflect.PrivateFieldHelper#getInstanceFieldValue(java.lang.Object, java.lang.String)
  */
 public Object getInstanceFieldValue(Object instance, String fieldName) 
  throws NoSuchFieldException, IllegalArgumentException  {
  return getPrivateFieldValue(instance, findPrivateInstanceField(fieldName));
 }
 
 /*
  * (non-Javadoc)
  * @see com.joshcough.reflect.PrivateFieldHelper#setInstanceFieldValue(java.lang.Object, java.lang.String, java.lang.Object)
  */
 public void setInstanceFieldValue(Object instance, String fieldName, Object newValue)
   throws IllegalArgumentException, NoSuchFieldException {
  setPrivateFieldValue(findPrivateInstanceField(fieldName), instance, newValue);
 }
 
 /**
  * 
  * @param f
  * @return
  * @throws 
  */
 private Object getPrivateFieldValue(Object instance, Field f) {
  f.setAccessible(true);
  Object o;
  try {
   o = f.get(instance);
  }  catch (IllegalAccessException e) {
   throw new PrivateFieldException(e);
  }
  f.setAccessible(false);
  return o;
 }

 /**
  * 
  * @param f
  * @param newValue
  * @throws 
  */
 private void setPrivateFieldValue(Field f, Object instance, Object newValue){
  f.setAccessible(true);
  try {
   f.set(instance, newValue);
  } catch (IllegalAccessException e) {
   throw new PrivateFieldException(e);
  }
  f.setAccessible(false);
 }

 /**
  * 
  * @param fieldName
  * @return
  * @throws NoSuchFieldException
  */
 private Field findPrivateStaticField(String fieldName) throws NoSuchFieldException {
  for (Field f : clazz.getDeclaredFields()) {
   if (f.getName().equals(fieldName)) {
    if (Modifier.isStatic(f.getModifiers()))
     return f;
   }
  }
  throw new NoSuchFieldException();
 }

 /**
  * 
  * @param fieldName
  * @return
  * @throws NoSuchFieldException
  */
 private Field findPrivateInstanceField(String fieldName) throws NoSuchFieldException {
  for (Field f : clazz.getDeclaredFields()) {
   if (f.getName().equals(fieldName)) {
    if (! Modifier.isStatic(f.getModifiers()))
     return f;
   }
  }
  throw new NoSuchFieldException();
 }
 
}

Cruise Control Remote Management API

I've never contributed to an OSS project before. Why that is I'm not quite sure. Looking through the CruiseControl Java code its pretty clear to me that I have the skills. I understand the code and I understand ways that I could refactor and improve it. I understand that I probably had those skills years ago. So why I haven't is beyond me; maybe I just never had the confidence.

Anyway, I've finally submitted something. I wrote a CruiseControl Remote Management library that wraps the exposed JMX attributes and operations for a server and its projects. You can do nice things like getting a reference to a server, getting all its projects, force builds on projects, set labels on projects, lots of nice stuff. Here's a simple example that forces a build on all projects:

CruiseServer server = new Server("localhost");
List projects = server.getProjects();
for( CruiseProject p: projects ){ p.forceBuild(); }



You could do nice things with Build Pipelining like trigger builds in other CruiseControl instances on different servers after a project on your server builds. Indeed, thats actually what I've done.

I think that this code could be used to clean up a lot of the code in their current tree. Maybe I'm wrong, or maybe they already have something like this, but I didn't see it.

Hopefully they will take this and add it in. Maybe they will ask me to add it in. Maybe I'll get to refactor a bunch of their Dashboard code. I'm almost certain it would be cleaner using this.

We'll see what happens.