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

Reply via email to