Allon Mureinik has posted comments on this change. Change subject: core: Clean up query test conf mocking ......................................................................
Patch Set 3: (4 comments) http://gerrit.ovirt.org/#/c/36539/3//COMMIT_MSG Commit Message: Line 14: the descendant classes to know details about the parent's Line 15: implementation. Line 16: Line 17: This patch attempts to improve the situation by have AbstractQueryTest Line 18: define the getExtraCofigDescriptors() method. This allows each > s/getExtraCofigDescriptors/getExtraConfigDescriptors Good catch, will fix. Line 19: inheriting class to specify which Config values it wishes to add to the Line 20: mocking, while leaving the actual mocking to the base class. Line 21: Line 22: Change-Id: I1d511b1f67e842a64b7c9322a0d526e717f4a601 http://gerrit.ovirt.org/#/c/36539/3/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AbstractSysprepQueryTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AbstractSysprepQueryTest.java: Line 15: public abstract class AbstractSysprepQueryTest<P extends VdcQueryParametersBase, Q extends QueriesCommandBase<? extends P>> extends AbstractUserQueryTest<P, Q> { Line 16: Line 17: @Override Line 18: public Set<MockConfigDescriptor<String>> getExtraConfigDescriptors() { Line 19: return Collections.singleton(mockConfig(ConfigValues.AdUserName, "")); > haven't you missed a field here? No - ConfigValues.UserSessionTimeOutInterval is common to all the query tests and is handled by the parent class. Line 20: } http://gerrit.ovirt.org/#/c/36539/3/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GetAddedGlusterServersQueryTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GetAddedGlusterServersQueryTest.java: Line 67 Line 68 Line 69 Line 70 Line 71 > so this is done by the parent class now? yup http://gerrit.ovirt.org/#/c/36539/3/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/GetDeviceListQueryTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/GetDeviceListQueryTest.java: Line 37 Line 38 Line 39 Line 40 Line 41 > same question here? This is handled by the parent. It's just completely redudent to do again here. -- To view, visit http://gerrit.ovirt.org/36539 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1d511b1f67e842a64b7c9322a0d526e717f4a601 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Amit Aviram <aavi...@redhat.com> Gerrit-Reviewer: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches