[Engine-devel] Getting rid of PowerMock?

[Mike Kolesnik]

> 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();
}

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
>