On 03/28/2012 10:55 AM, Moti Asayag 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. +1 On your idea. Too bad QueriesCommandBase and CommandBase do no share the same parent. Your idea is needed in queries as well.
> >> 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@ovirt.org >> http://lists.ovirt.org/mailman/listinfo/engine-devel > > _______________________________________________ > Engine-devel mailing list > Engine-devel@ovirt.org > http://lists.ovirt.org/mailman/listinfo/engine-devel _______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel