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