[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [master 2/2] ut: cleanup the taking-over-io mechanism.



Ok two comments..

> +import __builtin__
> +
> +_orig_open = open

Not used, so probably safe to remove

> class EddTestFS(object):

This is just a container for data filling methods. So it should be probably converted to simple functions which will fill data into existing DiskIO object in TestCases' setups. Since it is in the test directory and works I agree that it can stay there, but it should be cleaner in the future...

--
Martin Sivák
msivak redhat com
Red Hat Czech
Anaconda team / Brno, CZ

----- Original Message -----
> 1) Move the functionality from mock.DiskIO to mock.TestCase itself.
> 2) Use already existing mechanism (tearDownModules()) for cleaning up
> after we override functions in widely used modules.
> ---
> tests/mock/__init__.py | 32 +++++++++++++++++++++---
> tests/mock/disk.py | 29 +---------------------
> tests/storage_test/devicelibs_test/edd_test.py | 32
> +++++++++++------------
> 3 files changed, 44 insertions(+), 49 deletions(-)
> 
> diff --git a/tests/mock/__init__.py b/tests/mock/__init__.py
> index fb9dd07..eb7f9b2 100644
> --- a/tests/mock/__init__.py
> +++ b/tests/mock/__init__.py
> @@ -23,6 +23,9 @@
> from disk import *
> from mock import *
> import unittest
> +import __builtin__
> +
> +_orig_open = open
> 
> def slow(f):
> """Decorates a test method as being slow, usefull for python-nose
> filtering"""
> @@ -38,7 +41,7 @@ class TestCase(unittest.TestCase):
> def __init__(self, *args, **kwargs):
> unittest.TestCase.__init__(self, *args, **kwargs)
> self.injectedModules = {}
> -
> +
> def setupModules(self, a):
> """Mock specified list of modules and store the list so it can be
> properly unloaded during tearDown"""
> @@ -52,12 +55,12 @@ class TestCase(unittest.TestCase):
> 
> def modifiedModule(self, mname, mod = None):
> """Mark module (and all it's parents) as tainted"""
> -
> +
> oldname=""
> for m in mname.split("."):
> self.injectedModules[oldname+m] = mod
> oldname += m + "."
> - self.injectedModules[mname] = mod
> + self.injectedModules[mname] = mod
> 
> def tearDownModules(self):
> """Unload previously Mocked modules"""
> @@ -67,6 +70,27 @@ class TestCase(unittest.TestCase):
> for m in sys.modules.keys():
> if m in self.preexistingModules and not m in self.injectedModules:
> continue
> -
> +
> del sys.modules[m]
> 
> + def take_over_io(self, disk, target_module):
> + """ Trick target_module into using disk object as the filesystem.
> +
> + This is achieved by overriding the module's 'open' binding as well
> + as many global bindings in os.path.
> + """
> + target_module.open = disk.open
> + self.modifiedModule(target_module.__name__)
> +
> + import glob
> + self.modifiedModule("glob")
> + glob.glob = disk.glob_glob
> +
> + import os
> + self.modifiedModule("os.path")
> + # this is what os.path implementaion points at, reimport:
> + import posixpath
> + self.modifiedModule("posixpath")
> + os.listdir = disk.os_listdir
> + os.path.exists = disk.os_path_exists
> + os.path.isdir = disk.os_path_isdir
> diff --git a/tests/mock/disk.py b/tests/mock/disk.py
> index fbd6181..4fe4c76 100644
> --- a/tests/mock/disk.py
> +++ b/tests/mock/disk.py
> @@ -18,14 +18,7 @@
> 
> from StringIO import StringIO
> import fnmatch
> -import glob
> -import os.path
> -
> -_orig_glob_glob = glob.glob
> -_orig_open = open
> -_orig_os_listdir = os.listdir
> -_orig_os_path_exists = os.path.exists
> -_orig_os_path_isdir = os.path.isdir
> +import os
> 
> class DiskIO(object):
> """Simple object to simplify mocking of file operations in Mock
> @@ -133,23 +126,3 @@ class DiskIO(object):
> 
> def os_access(self, path, mode):
> return self.path_exists(path)
> -
> - def take_over_module(self, module):
> - """ Trick module into using this object as the filesystem.
> -
> - This is achieved by overriding the module's 'open' binding as well
> - as some bindings in os.path.
> - """
> - module.open = self.open
> - module.glob.glob = self.glob_glob
> - module.os.listdir = self.os_listdir
> - module.os.path.exists = self.os_path_exists
> - module.os.path.isdir = self.os_path_isdir
> -
> - @staticmethod
> - def restore_module(module):
> - module.open = _orig_open
> - module.glob.glob = _orig_glob_glob
> - module.os.listdir = _orig_os_listdir
> - module.os.path.exists = _orig_os_path_exists
> - module.os.path.isdir = _orig_os_path_isdir
> diff --git a/tests/storage_test/devicelibs_test/edd_test.py
> b/tests/storage_test/devicelibs_test/edd_test.py
> index f9f90ce..f27795a 100644
> --- a/tests/storage_test/devicelibs_test/edd_test.py
> +++ b/tests/storage_test/devicelibs_test/edd_test.py
> @@ -6,9 +6,7 @@ class EddTestCase(mock.TestCase):
> ['_isys', 'logging', 'pyanaconda.anaconda_log', 'block'])
> 
> def tearDown(self):
> - from pyanaconda.storage.devicelibs import edd
> self.tearDownModules()
> - mock.DiskIO.restore_module(edd)
> 
> def test_biosdev_to_edd_dir(self):
> from pyanaconda.storage.devicelibs import edd
> @@ -19,7 +17,7 @@ class EddTestCase(mock.TestCase):
> from pyanaconda.storage.devicelibs import edd
> 
> # test with vda, vdb
> - fs = EddTestFS(edd).vda_vdb()
> + fs = EddTestFS(self, edd).vda_vdb()
> edd_dict = edd.collect_edd_data()
> self.assertEqual(len(edd_dict), 2)
> self.assertEqual(edd_dict[0x80].type, "SCSI")
> @@ -31,7 +29,7 @@ class EddTestCase(mock.TestCase):
> self.assertEqual(edd_dict[0x81].pci_dev, "00:06.0")
> 
> # test with sda, vda
> - fs = EddTestFS(edd).sda_vda()
> + fs = EddTestFS(self, edd).sda_vda()
> edd_dict = edd.collect_edd_data()
> self.assertEqual(len(edd_dict), 2)
> self.assertEqual(edd_dict[0x80].type, "ATA")
> @@ -45,7 +43,7 @@ class EddTestCase(mock.TestCase):
> 
> def test_collect_edd_data_cciss(self):
> from pyanaconda.storage.devicelibs import edd
> - fs = EddTestFS(edd).sda_cciss()
> + fs = EddTestFS(self, edd).sda_cciss()
> edd_dict = edd.collect_edd_data()
> 
> self.assertEqual(edd_dict[0x80].pci_dev, None)
> @@ -53,7 +51,7 @@ class EddTestCase(mock.TestCase):
> 
> def test_edd_entry_str(self):
> from pyanaconda.storage.devicelibs import edd
> - fs = EddTestFS(edd).sda_vda()
> + fs = EddTestFS(self, edd).sda_vda()
> edd_dict = edd.collect_edd_data()
> expected_output = """\ttype: ATA, ata_device: 0
> \tchannel: 0, mbr_signature: 0x000ccb01
> @@ -63,7 +61,7 @@ class EddTestCase(mock.TestCase):
> 
> def test_matcher_device_path(self):
> from pyanaconda.storage.devicelibs import edd
> - fs = EddTestFS(edd).sda_vda()
> + fs = EddTestFS(self, edd).sda_vda()
> edd_dict = edd.collect_edd_data()
> 
> analyzer = edd.EddMatcher(edd_dict[0x80])
> @@ -76,7 +74,7 @@ class EddTestCase(mock.TestCase):
> 
> def test_bad_device_path(self):
> from pyanaconda.storage.devicelibs import edd
> - fs = EddTestFS(edd).sda_vda_no_pcidev()
> + fs = EddTestFS(self, edd).sda_vda_no_pcidev()
> edd_dict = edd.collect_edd_data()
> 
> analyzer = edd.EddMatcher(edd_dict[0x80])
> @@ -86,7 +84,7 @@ class EddTestCase(mock.TestCase):
> def test_get_edd_dict_1(self):
> """ Test get_edd_dict()'s pci_dev matching. """
> from pyanaconda.storage.devicelibs import edd
> - fs = EddTestFS(edd).sda_vda()
> + fs = EddTestFS(self, edd).sda_vda()
> self.assertEqual(edd.get_edd_dict([]),
> {'sda' : 0x80,
> 'vda' : 0x81})
> @@ -97,33 +95,33 @@ class EddTestCase(mock.TestCase):
> edd.collect_mbrs = mock.Mock(return_value = {
> 'sda' : '0x000ccb01',
> 'vda' : '0x0006aef1'})
> - fs = EddTestFS(edd).sda_vda_missing_details()
> + fs = EddTestFS(self, edd).sda_vda_missing_details()
> self.assertEqual(edd.get_edd_dict([]),
> {'sda' : 0x80,
> 'vda' : 0x81})
> 
> class EddTestFS(object):
> - def __init__(self, target_module):
> + def __init__(self, test_case, target_module):
> self.fs = mock.DiskIO()
> - self.fs.take_over_module(target_module)
> + test_case.take_over_io(self.fs, target_module)
> 
> def sda_vda_missing_details(self):
> self.fs["/sys/firmware/edd/int13_dev80"] = self.fs.Dir()
> - self.fs["/sys/firmware/edd/int13_dev80/mbr_signature"] =
> "0x000ccb01"
> + self.fs["/sys/firmware/edd/int13_dev80/mbr_signature"] =
> "0x000ccb01\n"
> self.fs["/sys/firmware/edd/int13_dev81"] = self.fs.Dir()
> - self.fs["/sys/firmware/edd/int13_dev81/mbr_signature"] =
> "0x0006aef1"
> + self.fs["/sys/firmware/edd/int13_dev81/mbr_signature"] =
> "0x0006aef1\n"
> 
> def sda_vda(self):
> self.fs["/sys/firmware/edd/int13_dev80"] = self.fs.Dir()
> self.fs["/sys/firmware/edd/int13_dev80/host_bus"] = "PCI 00:01.1
> channel: 0\n"
> self.fs["/sys/firmware/edd/int13_dev80/interface"] = "ATA device: 0\n"
> - self.fs["/sys/firmware/edd/int13_dev80/mbr_signature"] =
> "0x000ccb01"
> + self.fs["/sys/firmware/edd/int13_dev80/mbr_signature"] =
> "0x000ccb01\n"
> self.fs["/sys/firmware/edd/int13_dev80/sectors"] = "2097152\n"
> 
> self.fs["/sys/firmware/edd/int13_dev81"] = self.fs.Dir()
> self.fs["/sys/firmware/edd/int13_dev81/host_bus"] = "PCI 00:05.0
> channel: 0\n"
> self.fs["/sys/firmware/edd/int13_dev81/interface"] = "SCSI id: 0 lun:
> 0\n"
> - self.fs["/sys/firmware/edd/int13_dev81/mbr_signature"] =
> "0x0006aef1"
> + self.fs["/sys/firmware/edd/int13_dev81/mbr_signature"] =
> "0x0006aef1\n"
> self.fs["/sys/firmware/edd/int13_dev81/sectors"] = "16777216\n"
> 
> self.fs["/sys/devices/pci0000:00/0000:00:01.1/host0/target0:0:0/0:0:0:0/block"]
> = self.fs.Dir()
> @@ -144,7 +142,7 @@ class EddTestFS(object):
> self.fs["/sys/firmware/edd/int13_dev80"] = self.fs.Dir()
> self.fs["/sys/firmware/edd/int13_dev80/host_bus"] = "PCIX 05:00.0
> channel: 0\n"
> self.fs["/sys/firmware/edd/int13_dev80/interface"] = "RAID
> identity_tag: 0\n"
> - self.fs["/sys/firmware/edd/int13_dev80/mbr_signature"] =
> "0x000ccb01"
> + self.fs["/sys/firmware/edd/int13_dev80/mbr_signature"] =
> "0x000ccb01\n"
> self.fs["/sys/firmware/edd/int13_dev80/sectors"] = "2097152\n"
> 
> return self.fs
> --
> 1.7.6
> 
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list redhat com
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]