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

[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 -
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/jbpm-dev/attachments/20120511/e3ad8dda/attachment-0001.html 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Screen Shot 2012-05-11 at 11.34.00 AM.png
Type: image/png
Size: 64523 bytes
Desc: not available
Url : http://lists.jboss.org/pipermail/jbpm-dev/attachments/20120511/e3ad8dda/attachment-0001.png