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
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-devel
_______________________________________________
Engine-devel mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-devel
_______________________________________________
Engine-devel mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-devel
_______________________________________________
Engine-devel mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-devel
