Sunday, 7 April 2013

MD5 tests

Consider this test:
   @Test
   public void testMapRowWhenDocIdNotInSolr() throws Exception {
      idToScore.put(DOC_ID101, 0l);
      context.checking(new Expectations(){{
         oneOf(rs).getString(1);will(returnValue(DOC_ID101));
         oneOf(rs).getLong(2);will(returnValue(DOC_SCORE));  

         oneOf(solr).query(with(solrQueryHolder));
         will(returnValue(solrResponse));
         
         oneOf(solrResponse).getResults();will(returnValue(new SolrDocumentList()));
         
         never(solr).getBinder();will(returnValue(new DocumentObjectBinder()));
         never(solr).addBean(with(baseDocumentHolder));
         
      }});
      unit.mapRow(rs, 101);
      assertEquals("id:docId101",solrQueryHolder.getFirstHeldObject().getFilterQueries()[0]);
      assertEquals(new Integer(1),solrQueryHolder.getFirstHeldObject().getRows());
      assertEquals("*",solrQueryHolder.getFirstHeldObject().getQuery());      
   }
And what exactly is this testing? Alas, the name of the test doesn't really tell us - it tells us what's going to be called (unit.mapRow), and what the pre-conditions are (the docId is not in Solr), but doesn't give any clue as to what the expected outcome is. That's a first smell of a bad test. Nor does it get any better. The test proceeds to make a bunch of mock expectations about what's going to get called, but none of this helps us understand what is really expected to happen.  

The problem here is that I want to refactor this class. I want to keep the same behaviour, but I don't want it in mapRow(). This is unfortunate, because all the tests in this project look like this, and if I move functionality elsewhere I don't have any tests at a functional level that will show that I'm still achieving the same end goal.  

The colloquial name for these tests in our team is an "MD5 test". That is, the test just tests that the method is exactly what I've written. I may as well test the MD5 hash of the method body. 

Mocking frameworks make these kind of tests very easy to accidentally introduce.  The unfortunate coder will stray into the wilderness of mocking up every single method call, making sure to examine and verify every method signature along the way.  The alert coder will consider what the intent of the mocking is.  In some cases, verifying a specific method call is what you're intending to do, but I'd suggest a majority of the time mocking is simply a way to return fake values, in which case you probably want a stub.