[jbpm-dev] jbpm human task core tests refactorings - next steps

Mon May 14 09:26:48 EDT 2012

Hi guys, I've already merged the proposed changes my next steps will be:

1) migrate to Junit 4 all the tests using the @Test annotation and remove
the TestCase extensions
2) refactor all the tests with hardcoded users and just leave the tests
which uses the UserGroupCallBacks (refactoring their names to not include
UserGroupCallBacks )
3) write docs about the new Handlers and probably an example to see what
tests needs to be implemented if a new functionality wants to be tested.
4) Write the documentation about the Exceptions and review if we can
introduce more specific ones.

I will be spending some time these week in the form builder so probably I
will create a separate thread about that.

On Sun, May 13, 2012 at 11:26 AM, Mauricio Salatino <salaboy at gmail.com>wrote:

> Ok, because I didn't get more feedback I'm pushing the changes today to
> master.
> I will be looking jenkins to see if something else gets affected, please
> get in touch with me if you notice something wrong before I do.
> Cheers
>
>
> On Fri, May 11, 2012 at 7:16 PM, Mauricio Salatino <salaboy at gmail.com>wrote:
>
>> @Marco: Hehe totally agree with the approach. I just don't want to make
>> those reports looks better, I want to be sure that the code that we have is
>> good.
>>
>> I've already migrated the changes to the transport projects, which it was
>> easier than I thought, meaning that the changes doesn't affect too much. I
>> will delete all the commented classes as well.
>>
>> I'm ready to push now, but I will wait for a while to see if I get more
>> feedback. I'm confident with fixing issues with the callbacks if they
>> appear. If you guys knows where the callbacks are being used in the
>> jbpm-console please let me know.
>> Cheers
>>
>>
>>
>> On Fri, May 11, 2012 at 12:48 PM, Marco Rietveld <mrietvel at redhat.com>wrote:
>>
>>>  Oh yeah: coverage reports.
>>>
>>> IMHO, coverage reports are like weather reports: good to check now and
>>> then, but not something you want to plan around.
>>> Also, wrt agile/efficiency, we're better off making sure that the tests
>>> do actually test what we want them to -- which is pretty much exactly what
>>> you've been doing, Mauricio.
>>>
>>> 11-05-12 11:36, Maciej Swiderski:
>>>
>>> +1 for cleaning up the test, it takes quite some time to run all of them.
>>>
>>> Regarding changes of package good to keep in mind:
>>> - installer stuff that it declares DefaultUserGroupCallback in both
>>> web.xml and jbpm.usergroup.callback.properties.
>>> - human task war tests could declare...
>>>
>>> There should be LDAPUserInfo class that could go into identity package
>>> as well.
>>>
>>> Coverage report look quite ok :) and improvement is always welcome.
>>>
>>> Cheers
>>> Maciej
>>>
>>> 2012/5/11 Mauricio Salatino <salaboy at gmail.com>
>>>
>>>> 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 at redhat.com>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 at lists.jboss.org
>>>> https://lists.jboss.org/mailman/listinfo/jbpm-dev
>>>>
>>>>
>>>
>>>
>>> --
>>> 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 -
>>
>>
>
>
> --
>  - MyJourney @ http://salaboy.wordpress.com
>  - Co-Founder @ http://www.jugargentina.org
>  - Co-Founder @ http://www.jbug.com.ar
>
>  - Salatino "Salaboy" Mauricio -
>
>

-- 
 - 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/20120514/27675785/attachment-0001.html 