[Engine-devel] Getting rid of PowerMock?

[Roy Golan]

On 03/29/2012 08:35 AM, Mike Kolesnik wrote:
>> On 03/27/2012 07:33 PM, Yair Zaslavsky wrote:
>>> Hi all,
>>> As (almost) all of us can see,
>>> Running BLL tests both takes considerable time, and also we have to
>>> take
>>> class loading dependencies between classes with static methods when
>>> we
>>> use mockStatic - IMHO, this is kinda frustrating.
>>> Maybe we should start get rid of  PowerMock.
>>> Here is what I'm thinking of -
>>> Currently, as we use no IoC for DAOs , for tested class we do not
>>> use
>>> DbFacade.getInstance().getXXXDao()
>>>
>>> instead we define:
>>>
>>> protected XXXDao getXXXDao() {
>>>    return DBFacade.getInstance().getXXXDao();
>>> }
>>>
>> +1 for this. IMO all DAOs should be reachable from the CommandBase as
>> this is a repeated code scattered in the various commands.
>>
>> I'd suggest to create an interface that declares all of available
>> DAOs.
>> The new interface should be implemented by both DbFacade (which
>> already
>> contains that implementation) and CommandBase to assure new DAOs
>> added
>> to DbFacade will be added to the CommandBase as well.
> I'm not sure an interface is needed, since you won't be replacing it with different implementations.
>
> However, maybe a good approach is to put this method in bases class:
> protected DbFacade getDbFacade() {
>      return DbFacade.getInstance();
> }
I think we should use insert an injection stage in the commandFactory to 
fulfill the command resources .

@EngineInject
DbFacade dbFacade;

We can do something similar with the Config.
> This of course can be spied on to return a mock of the facade, and this mock can in turn return whatever mock DAOs you need.
>
>>> And then in our tests we use Mockito to define how getXXDao acts in
>>> the
>>> test.
>>>
>>> The following can be achieved also for config values , the
>>> following way -
>>>
>>> protected<T>  T getConfig(ConfigValues key) {
>>>          return Config.<T>  GetValue(key);
>>>      }
>>>
>>> And then in code (for example, in a query test) -
>>>
>>>   doReturn(5).when(query).<Integer>
>>> getConfig(any(ConfigValues.SomeIntegerConstant));
>>>
>>>
>>> This effort can be done in small steps -
>>> a. Define getConfig method in base classes (i.e AuditLoggalbeBase).
>>> b. Rewrite parts (i.e Commands, and their tests) step by step
>>> (small
>>> commits)
>>>
>>> Thoughts and ideas are more than welcome,
>> Searching for the static mocked classes brought the following list:
>>
>>        1 AuditLogDirector.class
>>        1 CommandsFactory.class
>>        1 FileUtil.class
>>        1 LoggerFactory.class
>>        1 QuotaHelper.class
>>        1 QuotaManager.class
>>        1 SchedulerUtilQuartzImpl.class
>>        1 StopVdsCommand.class
>>        1 TransactionSupport.class
>>        1 VmTemplateCommand.class
>>        2 CpuFlagsManagerHandler.class
>>        2 StorageDomainSpaceChecker.class
>>        2 VdsGroupDAO.class
>>        3 EjbUtils.class
>>        3 LogFactory.class
>>        3 MultiLevelAdministrationHandler.class
>>        3 StorageHelperDirector.class
>>        3 VmTemplateHandler.class
>>        6 ImagesHandler.class
>>        6 VmHandler.class
>>       10 Backend.class
>>       28 Config.class
>>       33 DbFacade.class
>>
>> So handling the Config and DbFacade will be a major step toward
>> powermock elimination.
>> (list retrieved by running the next command from ovirt-engine head:
>> find
>> . -name "*Test.java" -exec grep -r "@PrepareForTest" {} \; | tr "{)}"
>> "
>> " | cut -d \(  -f2 | tr "," "\n" | tr -d " " | sort | uniq -c | sort
>> -n)
>>
>>
>>> Yair
>>> _______________________________________________
>>> Engine-devel mailing list
>>> Engine-devel at ovirt.org
>>> http://lists.ovirt.org/mailman/listinfo/engine-devel
>> _______________________________________________
>> Engine-devel mailing list
>> Engine-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/engine-devel
>>
> _______________________________________________
> Engine-devel mailing list
> Engine-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel