Re: [Engine-devel] Getting rid of PowerMock?

> 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(a)ovirt.org >> http://lists.ovirt.org/mailman/listinfo/engine-devel > _______________________________________________ > Engine-devel mailing list > Engine-devel(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/engine-devel > _______________________________________________ Engine-devel mailing list Engine-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel