Re: [jbpm-dev] jbpm human task core tests refactorings <- feedback plz!

Sure, I will do that if all the other points make sense, and I will not push the commented out tests :) On Fri, May 11, 2012 at 12:04 PM, Marco Rietveld <mrietvel(a)redhat.com&gt;wrote: > Hi Mauricio, > > Feel free to delete the code instead of commenting it out, especially if > you can't use @Ignore. > > Commented out code is more often confusing than not. :) > > Thanks, > Marco > > 11-05-12 10:46, Mauricio Salatino: > > Hi guys, > I was working yesterday trying to understand why we have 390 test > running inside jbpm-human-task-core and if they are really necessary. > This is my initial approach so forgive me if I'm missing something, the > following notes and changes are what I found until now: > > 1) All the test classes using directly TaskClient should be removed, > now everything should go through TaskService and AsyncTaskService > test which are placed inside async and sync. For that reason all the test > inside org.jbpm.task.service are commented out. > I've checked that all the test classes that were using TaskClient now has > their correspondency inside the org.jbpm.task.service.base.async package. > Where the only important change is from TaskClient to AsyncTaskService. > We were running the same tests for both wasting a lot of time. > Now the base.sync package tests everything using the TaskService > interface. Right now there are not the same tests for the sync and the > async interface but I can migrate them soon. > > > 2) Reviewing the org.jbpm.task.service package I found all the > UserGroupCallback tests and the class called BaseTestNoUserGroupSetup, > which basically run almost > all the test using no hardcoded users and using the UserCallbacks > resolutions. Talking yesterday with Tiho, we both agree that we should > remove the tests > that are using hardcoded users and just leave the one that are using > callbacks to avoid running unnecessary tests. Just doing this we will > remove a lot of overhead > and we can have a simplified way of organizing and running the tests. > I've moved all the UserGroupCallback tests to the async package and migrate > them to use the AsyncTaskService instead of the TaskClient. > > 3) I've created a package called org.jbpm.task.identity to place the > LDAP stuff which is ignored and the UserGroupCallBack stuff that is not > using the TaskService/TaskClient stuff, when I was doing that I notice that > those tests uses some properties files that needs to be placed inside the > same package structure to be picked up, for that reason I've moved two > classes inside src/main/java to the following new > package: org.jbpm.task.identity -> UserGroupCallbackManager, > UserGroupCallback, DefaultUserGroupCallbackImpl, LDAPUserGroupCallbackImpl, > which I believe that is better than have the UserGroupCallback stuff mixed > with the task.service stuff. I notice that the > package: org.jbpm.task.service is a wild area, that we should avoid as much > as possible. I know that this is a change that can impact in other modules > like the jbpm-console but I think that it will avoid future troubles. I > really need your feedback on this.. to see if you guys think that this can > cause a lot of troubles. > > 4) for the same reason as 1 I've commented out all the tests > inside org.jbpm.task.service.test, those were using the TaskClient and > because the async and sync test are already provided we don't miss > anything. The only thing that we need to do inside > the org.jbpm.task.service.test package is to align these tests to cover all > the base async and sync tests, which is not the case right now, but it's > simple to do it. > > 5) I've added covertura to the jbpm-human-task-core project to do an > initial analysis about what we are covering with our tests. Attached is the > initial chart that I would love to see improved in the following months :) > > > 6) I need to work on the other modules to propagate these changes as > well, but for doing that I need your feedback on the previous points, I > think that during the weekend I will be able to finish the other modules, > if we all are OK with this suggestions. > > Cheers > > > > -- > - MyJourney @ http://salaboy.wordpress.com > - Co-Founder @ http://www.jugargentina.org > - Co-Founder @ http://www.jbug.com.ar > > - Salatino "Salaboy" Mauricio - > > > > -- > jBPM/Drools developer > Utrecht, the Netherlands > > -- - MyJourney @ http://salaboy.wordpress.com - Co-Founder @ http://www.jugargentina.org - Co-Founder @ http://www.jbug.com.ar - Salatino "Salaboy" Mauricio - _______________________________________________ jbpm-dev mailing list jbpm-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/jbpm-dev