Moti Asayag has posted comments on this change. Change subject: engine: Introduce HostValidator ......................................................................
Patch Set 1: (1 comment) http://gerrit.ovirt.org/#/c/36157/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/HostValidator.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/HostValidator.java: Line 21: private final StoragePoolDAO storagePoolDao; Line 22: private final VdsStaticDAO hostStaticDao; Line 23: private final VDS host; Line 24: Line 25: public HostValidator(DbFacade dbFacade, VDS host) { > why have DbFacade as a parameter here? If it's to ease Junit, wouldn't a ge By introducing getDBFacade() to HostValidator, I'll be forced to use spy(HostValidator), and will have to override this method in several tests. The downside of it is not testing the real HostValidator.class, but some inspected version of it, produced by the mockito f/w. It also called partial mocking which is considered a code-smell and its usage should be reduced. (I did made a use of spy for haveSecurityKey(), which could have been avoided by introducing a parameter to securityKeysExists(), or as a parameter to the c'tor - but decided not to do so, to preserve previous validation sequence). Once DAO cdi patch is merged [1], we could just inject the specific daos objects and mock them separately in the test. So the change to this class will also be minimal and restricted to the c'tor without impacting other methods. [1] http://gerrit.ovirt.org/#/c/35793 Line 26: this.hostDao = dbFacade.getVdsDao(); Line 27: this.storagePoolDao = dbFacade.getStoragePoolDao(); Line 28: this.hostStaticDao = dbFacade.getVdsStaticDao(); Line 29: this.host = host; -- To view, visit http://gerrit.ovirt.org/36157 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2256daf1de6a510c5b6ccb18846dd36bdc107f7d Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Moti Asayag <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Sahina Bose <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
