Francesco Romani has posted comments on this change.

Change subject: tests: split vm tests helpers in a module
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.ovirt.org/#/c/32650/2/tests/vmtestlib.py
File tests/vmtestlib.py:

Line 16: # along with this program; if not, write to the Free Software
Line 17: # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  
02110-1301 USA
Line 18: #
Line 19: # Refer to the README and COPYING files for full details of the license
Line 20: #
> Can we rename this to vmfakelib ?
Indeed much nicer :) Will implement it
Line 21: 
Line 22: from contextlib import contextmanager
Line 23: 
Line 24: import libvirt


Line 32: from testlib import namedTemporaryDir
Line 33: from monkeypatch import MonkeyPatchScope
Line 34: 
Line 35: 
Line 36: class ConnectionMock:
> Lets unify the name in another patch to FakeConnection.
Definitely yes.
Line 37:     def __init__(self, *args):
Line 38:         pass
Line 39: 
Line 40:     def domainEventRegisterAny(self, *arg):


Line 137:             fake.conf['devices'] = [] if devices is None else devices
Line 138:             fake._guestCpuRunning = runCpu
Line 139:             if status is not None:
Line 140:                 fake._lastStatus = status
Line 141:             yield fake
> This has no __exit__ code, so why it has to be a contextmanager?
We don't stricly need a context manager here, but we need to be able to 
redirect P_VDSM_RUN to a temporary dir, and to get rid of it automatically when 
Fake VM goes away.

The core problem here is test code may call saveState, directly or indirectly, 
and saveState will write the recovery file in that directory.

I'll be happy to use a simpler solution which possibly doesn't need a nested 
scope in the test code, because the current idiom is

  with FakeVM() as fake:
     test_code_which_uses(fake)

and I don't really like it that much.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ied7bb1d38d3814c406a53e1b7dbcac34eb6ffeb7
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Martin Polednik <[email protected]>
Gerrit-Reviewer: Martin Sivák <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to