• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Retrofitting a unit test for legacy code

 
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
This is a continuation of what was started in this thread.
So far, I have created a few tests that pass, committing a number of sins I'm sure just to get to this point.
Any tips you may have on retrofitting a class with a unit test or some other sort of test would be appreciated. I will post more details as I get further along.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
On Ilja's suggestion, I took a look at the material in the WELC Yahoo! group. It seems very relevant to my questions (I even found a couple of my questions in there!).
Thanks, Ilja!
 
author
Posts: 14112
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Originally posted by Junilu Lacar:
On Ilja's suggestion, I took a look at the material in the WELC Yahoo! group. It seems very relevant to my questions (I even found a couple of my questions in there!).
Thanks, Ilja!


Nice to see that you found it to be useful!
 
Ilja Preuss
author
Posts: 14112
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
If your main concern is meeting requirements (which I get from your last post in the other thread), I think that system/acceptance tests really would be more appropriate. (Actually I think that ideally you had both, but you have to start *somewhere*.) They actually test the system from the point of view of a user and are less fragile regarding refactoring.
My personal tool of choice is FitNesse: http://fitnesse.org/
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Another reason the class under test (let's call this the CUT) was chosen is because it is a collaborator of a few other classes. The other classes call this "processor" to complete their work. The processes handled by the CUT are many and complex and the classes it services work under many different conditions, each of which having different pre-conditions for calling the CUT. As such there is code in the CUT that is structure like this (names of objects and methods changed to protect the not-so-innocent ):

And that's just the tip of the iceberg. Can you imagine how many knuckle-biting moments I have where I have to restrain myself from changing the procedural code into something more OO? The one thing I did have to do just to understand what the code was trying to accomplish was to break up the big update(Task) method into many smaller ones. Originally, the method took up almost a third of the source. Breaking it into many smaller methods resulted in another code smell though: VerbSubject methods, e.g. updateVAgentTask(), which I eventually had to break up into updateOpenVAgentTask() and updateClosedVAgentTask() where 'open' and 'closed' are statuses of a task. This isn't so bad because now it's clearer that further refactoring is needed.
Anyway, I have decided to write JUnit tests that ensure that the class behaves correctly as long as pre-conditions are satisfied. One thing I hope to get out of this first exercise is to find out if there are any pre-conditions that should be checked which are not. This at least will give me something concrete to work with when more bugs are found and fingers start pointing.
OK after all that, I guess I just want to get this off my chest and have no real question right now except for maybe "Can you sympathize with what I have to deal with?"
I started to try to figure out how to work with FitNesse on my last project but didn't really get too far. I'll try it out again with this one.
[ January 23, 2004: Message edited by: Junilu Lacar ]
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Well, maybe I do have a question. But first, to summarize:
The problem: We're getting bugs and it's hard to tell if the TaskProcessor and/or the classes it services is/are doing anything wrong.
The task: To retrofit the CUT with a unit(?) test to make sure it works correctly.
So, in what other ways can I approach this problem? Ilja, you suggested system/acceptance tests. That will definitely help although I haven't really used FitNesse before. So these tests would simulate entering data as a user and then verify what it expects to find in the database, right? In some cases, the "user" is actually a background process and it's still not certain if this background process is correctly doing everything it should.
[ January 23, 2004: Message edited by: Junilu Lacar ]
 
Ilja Preuss
author
Posts: 14112
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
First, let me state that I sympathize.
On the other hand, I found that working on such code actually has one advantage: the Customer typically is quite thankfull when things get better and tends to turn a blind eye to minor bugs - he is used to it. And it isn't that hard to make things better if you are only a little bit competent... (let alone if you are a JavaRanch bartender )

Originally posted by Junilu Lacar:
Well, maybe I do have a question. But first, to summarize:
The problem: We're getting bugs and it's hard to tell if the TaskProcessor and/or the classes it services is/are doing anything wrong.


How do you know that they are bugs?


The task: To retrofit the CUT with a unit(?) test to make sure it works correctly.


I am still not sure why that single CUT should be your first priority...


Ilja, you suggested system/acceptance tests. That will definitely help although I haven't really used FitNesse before.


I am rather new to FitNesse, too. Try it, it's really easy! And if you encounter problems, you know where to ask...

So these tests would simulate entering data as a user and then verify what it expects to find in the database, right?


Yes, correct.

In some cases, the "user" is actually a background process


That should actually make things easier.

and it's still not certain if this background process is correctly doing everything it should.


But you *do* know how it *should* behave? And what is the implication for the happiness of the customer with your work if it doesn't do everything it should (but your system expects it to)?
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Code ownership is individualized in this project. The CUT is actually "owned" by one team member who has been reassigned to install and troubleshoot the application at the client site. So I have taken over ownership of this class. That's one of the main reasons I'm focusing on testing this particular class.
Another problem I have seen with the current code is that the CUT is taking on responsibities that seem to be more appropriate for its client classes. Conversely, client classes to the CUT take on responsibilities that seem more appropriate for the CUT. This results in unnecessarily complex, unclear, duplicate and redundant code (in one case I found that checking whether or not a Task has recipients is done twice for the same request, once in the client code and once in the CUT).
We know there are bugs because we are getting unexpected and inconsistent data (e.g. two rows created in the DB when only one should have been). What makes things worse is that the bugs are intermittent. Because there are many different conditions involved in the process (which, of course, can be threaded :rolleyes it's hard to tell which particular combination of conditions cause the errors.
My half of the "solution" to the problem is to have the CUT check for requests that would result in these data inconsistencies and ignore them. We still have to figure out why the invalid request is being made in the first place.
I feel that all these problems stem from the improper assignment of responsibilities (or not knowing who exactly is responsible for doing something). Once more I am faced with the chicken and egg dilemma of writing tests first vs. refactoring first. Since writing the unit test is a smaller step than refactoring to assign responsibilities properly, I'm going to write the unit test first. I have identified the pre-conditions for sending messages to the CUT so I can write tests to see what happens when pre-conditions are met and what happens when they aren't. I guess these will be my "smoke tests" then.
 
Ilja Preuss
author
Posts: 14112
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Originally posted by Junilu Lacar:
Code ownership is individualized in this project.
...
Another problem I have seen with the current code is that the CUT is taking on responsibities that seem to be more appropriate for its client classes. Conversely, client classes to the CUT take on responsibilities that seem more appropriate for the CUT.


I wonder wether there might be a connection...


We know there are bugs because we are getting unexpected and inconsistent data (e.g. two rows created in the DB when only one should have been). What makes things worse is that the bugs are intermittent. Because there are many different conditions involved in the process (which, of course, can be threaded :rolleyes it's hard to tell which particular combination of conditions cause the errors.
My half of the "solution" to the problem is to have the CUT check for requests that would result in these data inconsistencies and ignore them.


My gut feel (and I might be totally worng) is that the problem you are trying to solve actually is a symptom of a bigger problem. I am not sure what it might be (probably a process/culture thing), let alone what to do about it, though...
Imagine your class were already fully tested. Do you feel that would really improve your situation that much?
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
In case anybody was wondering what became of this...well, not much really. After putting in code to prevent data inconsistencies (my half of the "solution"), it was decided not to pursue the matter further. The entire process needs to be revisited anyway and we'll probably end up doing rewrites to the majority of the code. When that happens, I'll just make sure that the tests are written first. C'est la vie :roll:
 
With a little knowledge, a cast iron skillet is non-stick and lasts a lifetime.
reply
    Bookmark Topic Watch Topic
  • New Topic