Change in vdsm[master]: tests: deduplicate clientIF fakes

danken at redhat.com danken at redhat.com
Thu Jan 15 18:21:57 UTC 2015


Dan Kenigsberg has posted comments on this change.

Change subject: tests: deduplicate clientIF fakes
......................................................................


Patch Set 2: Code-Review-1

(3 comments)

/me is not sure regrading the benefit of this.

http://gerrit.ovirt.org/#/c/36893/2//COMMIT_MSG
Commit Message:

Line 9: both clientIFTests and vmfakelib modules
Line 10: had an implementation of clientIF fake.
Line 11: 
Line 12: This patch reunite the two implementations
Line 13: into one into vmfakelib.
why exactly? I don't see much code duplication.

Usually, mock object are tied to their test
Line 14: 
Line 15: Change-Id: I4b27c3d37f62a1e19bf252290fd6816f5c65adcc


http://gerrit.ovirt.org/#/c/36893/2/tests/vmfakelib.py
File tests/vmfakelib.py:

Line 51: class ClientIF(clientIF.clientIF):
Line 52:     def __init__(self):
Line 53:         # the bare minimum initialization for our test needs.
Line 54:         self.irs = None  # just to make sure nothing ever happens
Line 55:         self.log = logging.getLogger('ClientIFTests')
the logger name isn't precise when the fake object is used by other vm tests.
Line 56:         self.channelListener = None
Line 57:         self.vmContainerLock = threading.Lock()
Line 58:         self.vmContainer = {}
Line 59: 


Line 53:         # the bare minimum initialization for our test needs.
Line 54:         self.irs = None  # just to make sure nothing ever happens
Line 55:         self.log = logging.getLogger('ClientIFTests')
Line 56:         self.channelListener = None
Line 57:         self.vmContainerLock = threading.Lock()
none of the implementations had this lock initialized. would you say way it is suddenly needed?
Line 58:         self.vmContainer = {}
Line 59: 
Line 60: 
Line 61: class Domain(object):


-- 
To view, visit http://gerrit.ovirt.org/36893
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4b27c3d37f62a1e19bf252290fd6816f5c65adcc
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list