Skip to content

Commit

Permalink
CA-344254: simplify test-cleanup and use a magicmock for xapi instead…
Browse files Browse the repository at this point in the history 
… of a fake

Signed-off-by: Mark Syms <mark.syms@citrix.com>
  • Loading branch information
@MarkSymsCtx
MarkSymsCtx committed Sep 21, 2020
1 parent 9d8ff5f commit 026caa6
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 45 deletions.
3 changes: 1 addition & 2 deletions tests/run_python_unittests.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ fi

echo "Test coverage"
coverage report -m --fail-under=100 --include=$SMROOT/tests/*

if [ $# -eq 0 ] && [ $? -ne 0 ]
if [ $? -gt 0 -a $# -eq 0 ]
then
echo "Test code not fully covered"
exit 1
Expand Down
75 changes: 32 additions & 43 deletions tests/test_cleanup.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,6 @@ def log(input):
log = staticmethod(log)


class FakeXapi(object):
def __init__(self):
self.srRecord = {
'name_label': 'dummy'
}
self.session = mock.MagicMock()

def isPluggedHere(self):
return True

def isMaster(self):
return True


class AlwaysLockedLock(object):
def acquireNoblock(self):
return False
Expand All @@ -63,8 +49,7 @@ class IrrelevantLock(object):
pass


def create_cleanup_sr(uuid=None):
xapi = FakeXapi()
def create_cleanup_sr(xapi, uuid=None):
return cleanup.SR(uuid=uuid, xapi=xapi, createLock=False, force=False)


Expand All @@ -82,6 +67,11 @@ def setUp(self):
blktap2_patcher = mock.patch('cleanup.blktap2', autospec=True)
self.mock_blktap2 = blktap2_patcher.start()

self.xapi_mock = mock.MagicMock(name='MockXapi')
self.xapi_mock.srRecord = {'name_label': 'dummy'}
self.xapi_mock.isPluggedHere.return_value = True
self.xapi_mock.isMaster.return_value = True

self.addCleanup(mock.patch.stopall)

def setup_abort_flag(self, ipc_mock, should_abort=False):
Expand All @@ -90,9 +80,8 @@ def setup_abort_flag(self, ipc_mock, should_abort=False):

ipc_mock.return_value = flag

def setup_mock_sr(selfs, mock_sr):
xapi = FakeXapi()
mock_sr.configure_mock(uuid=1234, xapi=xapi,
def setup_mock_sr(self, mock_sr):
mock_sr.configure_mock(uuid=1234, xapi=self.xapi_mock,
createLock=False, force=False)

def mock_cleanup_locks(self):
Expand All @@ -108,7 +97,7 @@ def test_lock_if_already_locked(self):
increments the lock counter
"""

sr = create_cleanup_sr()
sr = create_cleanup_sr(self.xapi_mock)
sr._srLock = IrrelevantLock()
sr._locked = 1

Expand All @@ -122,7 +111,7 @@ def test_lock_if_no_locking_is_used(self):
the counter
"""

sr = create_cleanup_sr()
sr = create_cleanup_sr(self.xapi_mock)
sr._srLock = None

sr.lock()
Expand All @@ -138,7 +127,7 @@ def test_lock_succeeds_if_lock_is_acquired(
"""

self.setup_abort_flag(mock_ipc_flag)
sr = create_cleanup_sr()
sr = create_cleanup_sr(self.xapi_mock)
sr._srLock = AlwaysFreeLock()

sr.lock()
Expand All @@ -154,7 +143,7 @@ def test_lock_raises_exception_if_abort_requested(
"""

self.setup_abort_flag(mock_ipc_flag, should_abort=True)
sr = create_cleanup_sr()
sr = create_cleanup_sr(self.xapi_mock)
sr._srLock = AlwaysLockedLock()

self.assertRaises(cleanup.AbortException, sr.lock)
Expand All @@ -168,7 +157,7 @@ def test_lock_raises_exception_if_unable_to_acquire_lock(
"""

self.setup_abort_flag(mock_ipc_flag)
sr = create_cleanup_sr()
sr = create_cleanup_sr(self.xapi_mock)
sr._srLock = AlwaysLockedLock()

self.assertRaises(util.SMException, sr.lock)
Expand All @@ -182,7 +171,7 @@ def test_lock_leaves_sr_consistent_if_unable_to_acquire_lock(
"""

self.setup_abort_flag(mock_ipc_flag)
sr = create_cleanup_sr()
sr = create_cleanup_sr(self.xapi_mock)
sr._srLock = AlwaysLockedLock()

try:
Expand Down Expand Up @@ -542,7 +531,7 @@ def test_file_vdi_delete(self, mock_lock):
mock_lock.Lock = mock.MagicMock(spec=lock.Lock)

sr_uuid = uuid4()
sr = create_cleanup_sr(uuid=str(sr_uuid))
sr = create_cleanup_sr(self.xapi_mock, uuid=str(sr_uuid))
vdi_uuid = uuid4()

vdi = cleanup.VDI(sr, str(vdi_uuid), False)
Expand All @@ -559,7 +548,7 @@ def test_coalesceLeaf(self, mock_srSnapshotCoalesce,
mock_vdi.canLiveCoalesce.return_value = True
mock_srLeafCoalesce.return_value = "This is a test"
sr_uuid = uuid4()
sr = create_cleanup_sr(uuid=str(sr_uuid))
sr = create_cleanup_sr(self.xapi_mock, uuid=str(sr_uuid))
vdi_uuid = uuid4()
vdi = cleanup.VDI(sr, str(vdi_uuid), False)

Expand All @@ -580,7 +569,7 @@ def test_coalesceLeaf_coalesce_failed(self,
mock_srSnapshotCoalesce.return_value = False
mock_srLeafCoalesce.return_value = False
sr_uuid = uuid4()
sr = create_cleanup_sr(uuid=str(sr_uuid))
sr = create_cleanup_sr(self.xapi_mock, uuid=str(sr_uuid))
vdi_uuid = uuid4()
vdi = cleanup.VDI(sr, str(vdi_uuid), False)

Expand All @@ -601,7 +590,7 @@ def test_coalesceLeaf_size_the_same(self,
mockliveCoalesce):

sr_uuid = uuid4()
sr = create_cleanup_sr(uuid=str(sr_uuid))
sr = create_cleanup_sr(self.xapi_mock, uuid=str(sr_uuid))
vdi_uuid = uuid4()
vdi = cleanup.VDI(sr, str(vdi_uuid), False)

Expand All @@ -623,7 +612,7 @@ def test_coalesceLeaf_size_bigger(self, mock_log,
mock_vdiLiveCoalesce):

sr_uuid = uuid4()
sr = create_cleanup_sr(uuid=str(sr_uuid))
sr = create_cleanup_sr(self.xapi_mock, uuid=str(sr_uuid))
vdi_uuid = uuid4()
vdi = cleanup.VDI(sr, str(vdi_uuid), False)

Expand Down Expand Up @@ -657,7 +646,7 @@ def test_coalesceLeaf_success_after_4_iterations(self,
mock_vhdSize.side_effect = iter([1024, 1023, 1023, 1022, 1022, 1021])

sr_uuid = uuid4()
sr = create_cleanup_sr(uuid=str(sr_uuid))
sr = create_cleanup_sr(self.xapi_mock, uuid=str(sr_uuid))
vdi_uuid = uuid4()
vdi = cleanup.VDI(sr, str(vdi_uuid), False)

Expand All @@ -671,7 +660,7 @@ def test_coalesceLeaf_success_after_4_iterations(self,
@mock.patch('cleanup.Util.log')
def test_findLeafCoalesceable_forbidden1(self, mock_log):
sr_uuid = uuid4()
sr = create_cleanup_sr(uuid=str(sr_uuid))
sr = create_cleanup_sr(self.xapi_mock, uuid=str(sr_uuid))
sr.xapi.srRecord = {"other_config": {cleanup.VDI.DB_COALESCE: "false"}}

res = sr.findLeafCoalesceable()
Expand All @@ -681,7 +670,7 @@ def test_findLeafCoalesceable_forbidden1(self, mock_log):
@mock.patch('cleanup.Util.log')
def test_findLeafCoalesceable_forbidden2(self, mock_log):
sr_uuid = uuid4()
sr = create_cleanup_sr(uuid=str(sr_uuid))
sr = create_cleanup_sr(self.xapi_mock, uuid=str(sr_uuid))
sr.xapi.srRecord =\
{"other_config":
{cleanup.VDI.DB_LEAFCLSC: cleanup.VDI.LEAFCLSC_DISABLED}}
Expand All @@ -693,7 +682,7 @@ def test_findLeafCoalesceable_forbidden2(self, mock_log):
@mock.patch('cleanup.Util.log')
def test_findLeafCoalesceable_forbidden3(self, mock_log):
sr_uuid = uuid4()
sr = create_cleanup_sr(uuid=str(sr_uuid))
sr = create_cleanup_sr(self.xapi_mock, uuid=str(sr_uuid))
sr.xapi.srRecord = {"other_config":
{cleanup.VDI.DB_LEAFCLSC:
cleanup.VDI.LEAFCLSC_DISABLED,
Expand All @@ -707,7 +696,7 @@ def test_findLeafCoalesceable_forbidden3(self, mock_log):
@mock.patch('cleanup.Util.log')
def test_findLeafCoalesceable_forbidden4(self, mock_log):
sr_uuid = uuid4()
sr = create_cleanup_sr(uuid=str(sr_uuid))
sr = create_cleanup_sr(self.xapi_mock, uuid=str(sr_uuid))
sr.xapi.srRecord = {"other_config": {cleanup.VDI.DB_LEAFCLSC:
cleanup.VDI.LEAFCLSC_DISABLED,
cleanup.VDI.DB_COALESCE:
Expand All @@ -720,7 +709,7 @@ def test_findLeafCoalesceable_forbidden4(self, mock_log):
@mock.patch('cleanup.Util.log')
def test_findLeafCoalesceable_forbidden5(self, mock_log):
sr_uuid = uuid4()
sr = create_cleanup_sr(uuid=str(sr_uuid))
sr = create_cleanup_sr(self.xapi_mock, uuid=str(sr_uuid))
sr.xapi.srRecord = {"other_config": {cleanup.VDI.DB_LEAFCLSC:
cleanup.VDI.LEAFCLSC_FORCE,
cleanup.VDI.DB_COALESCE:
Expand All @@ -734,7 +723,7 @@ def test_findLeafCoalesceable_forbidden5(self, mock_log):

def srWithOneGoodVDI(self, mock_getConfig, goodConfig):
sr_uuid = uuid4()
sr = create_cleanup_sr(uuid=str(sr_uuid))
sr = create_cleanup_sr(self.xapi_mock, uuid=str(sr_uuid))

vdi_uuid = uuid4()
if goodConfig:
Expand Down Expand Up @@ -872,7 +861,7 @@ def findLeafCoalesceable(self, mock_gatherLeafCoalesceable, goodSize,
expectedNothing=False):

sr_uuid = uuid4()
sr = create_cleanup_sr(uuid=str(sr_uuid))
sr = create_cleanup_sr(self.xapi_mock, uuid=str(sr_uuid))

good = self.makeVDIReturningSize(sr, goodSize, canLiveCoalesce,
liveSize)
Expand Down Expand Up @@ -998,7 +987,7 @@ def getStorageSpeed(self, mock_lock, mock_unlock, mock_isFile, sr,
def test_getStorageSpeed(self, mock_unlock, mock_lock, mock_chmod,
mock_isFile, mock_open):
sr_uuid = uuid4()
sr = create_cleanup_sr(uuid=str(sr_uuid))
sr = create_cleanup_sr(self.xapi_mock, uuid=str(sr_uuid))
fakeFile = self.makeFakeFile()
mock_open.return_value = FakeFile

Expand Down Expand Up @@ -1066,7 +1055,7 @@ def writeSpeedFile(self, mock_lock, mock_unlock, sr, speed, mock_isFile,
def test_writeSpeedToFile(self, mock_lock, mock_unlock, mock_atomicWrite,
mock_isFile, mock_open):
sr_uuid = uuid4()
sr = create_cleanup_sr(uuid=str(sr_uuid))
sr = create_cleanup_sr(self.xapi_mock, uuid=str(sr_uuid))
FakeFile = self.makeFakeFile()
mock_open.return_value = FakeFile

Expand Down Expand Up @@ -1146,7 +1135,7 @@ def canLiveCoalesce(self, vdi, size, config, speed, expectedRes):

def test_canLiveCoalesce(self):
sr_uuid = uuid4()
sr = create_cleanup_sr(uuid=str(sr_uuid))
sr = create_cleanup_sr(self.xapi_mock, uuid=str(sr_uuid))
vdi_uuid = uuid4()
vdi = cleanup.VDI(sr, str(vdi_uuid), False)
# Fast enough to for size 10/10 = 1 second and not forcing
Expand Down Expand Up @@ -1233,7 +1222,7 @@ def test_leafCoalesceForbidden(self, mock_srforbiddenBySwitch):
switchValue = "test"
failMessage = "This is a test"

sr = create_cleanup_sr()
sr = create_cleanup_sr(self.xapi_mock)

side_effect = iter([True, True])
self.leafCoalesceForbidden(sr, mock_srforbiddenBySwitch, side_effect,
Expand Down Expand Up @@ -1380,7 +1369,7 @@ def test_coalesce_success(
mock_abortable.side_effect = self.runAbortable

sr_uuid = uuid4()
sr = create_cleanup_sr(uuid=str(sr_uuid))
sr = create_cleanup_sr(self.xapi_mock, uuid=str(sr_uuid))
sr.journaler = mock_journaler

mock_ipc_flag = mock.MagicMock(spec=ipc.IPCFlag)
Expand Down

0 comments on commit 026caa6

Please sign in to comment.