[tor-commits] [ooni-probe/master] Refactoring of NetTestCase and NetTest with partial unittesting

isis at torproject.org isis at torproject.org
Sun Mar 10 01:57:01 UTC 2013


commit 376715a8350a0a52c57a2fa6f8d68b5d37eafb8d
Author: aagbsn <aagbsn at extc.org>
Date:   Sun Jan 13 00:56:20 2013 +0000

    Refactoring of NetTestCase and NetTest with partial unittesting
---
 ooni/nettest.py       |  123 ++++++++++++++-----------------------------------
 tests/test_nettest.py |   92 +++++++++++++++++++++++++++++++------
 2 files changed, 113 insertions(+), 102 deletions(-)

diff --git a/ooni/nettest.py b/ooni/nettest.py
index 511c3da..5c2e8cf 100644
--- a/ooni/nettest.py
+++ b/ooni/nettest.py
@@ -4,7 +4,7 @@ from twisted.trial.runner import filenameToModule
 from twisted.python import usage, reflect
 
 from ooni.tasks import Measurement
-from ooni.utils import log
+from ooni.utils import log, checkForRoot, NotRootError
 
 from inspect import getmembers
 from StringIO import StringIO
@@ -13,23 +13,20 @@ class NetTest(object):
     director = None
     method_prefix = 'test'
 
-    def __init__(self, net_test_file, inputs, options, report):
+    def __init__(self, net_test_file, options, report):
         """
         net_test_file:
             is a file object containing the test to be run.
 
-        inputs:
-            is a generator containing the inputs to the net test.
-
         options:
             is a dict containing the options to be passed to the net test.
         """
-        self.test_cases = self.loadNetTest(net_test_file)
-        self.inputs = inputs
         self.options = options
-
         self.report = report
 
+        self.test_cases = self.loadNetTest(net_test_file)
+        self.setUpNetTestCases()
+
     def start(self):
         """
         Start tests and generate measurements.
@@ -115,85 +112,30 @@ class NetTest(object):
         This is a generator that yields measurements and sets their timeout
         value and their netTest attribute.
         """
-        for test_input in self.inputs:
-            for test_class, test_method in self.test_cases:
-                measurement = Measurement(test_class, test_method, test_input)
+        for test_class, test_method in self.test_cases:
+            for test_input in test_class.inputs:
+                measurement = Measurement(test_class, test_method,
+                        test_input, self)
                 measurement.netTest = self
                 yield measurement
 
-    def processTestCasesOptions(self):
-        self.options #XXX is this cmd_line_options?
-
-        # get set of unique classes
+    def setUpNetTestCases(self):
+        """
+        Call processTest and processOptions methods of each NetTestCase
+        """
         test_classes = set([])
         for test_class, test_method in self.test_cases:
             test_classes.add(test_class)
 
-        #XXX where should the options bound to a test_class get stashed?
-        for test_class in test_classes:
-            options = self._processOptions()
-
-    #XXX: is options passed to init the same as cmd_line_options???
-    def _processTest(self, nettest_test_case, cmd_line_options):
-        """
-        Process the parameters and :class:`twisted.python.usage.Options` of a
-        :class:`ooni.nettest.Nettest`.
-
-        :param obj:
-            An uninstantiated old test, which should be a subclass of
-            :class:`ooni.plugoo.tests.OONITest`.
-
-        :param cmd_line_options:
-            A configured and instantiated :class:`twisted.python.usage.Options`
-            class.
-
-        """
-        obj = nettest_test_case
-        if not hasattr(obj.usageOptions, 'optParameters'):
-            obj.usageOptions.optParameters = []
-
-        if obj.inputFile:
-            obj.usageOptions.optParameters.append(obj.inputFile)
-
-        if obj.baseParameters:
-            for parameter in obj.baseParameters:
-                obj.usageOptions.optParameters.append(parameter)
-
-        if obj.baseFlags:
-            if not hasattr(obj.usageOptions, 'optFlags'):
-                obj.usageOptions.optFlags = []
-            for flag in obj.baseFlags:
-                obj.usageOptions.optFlags.append(flag)
-
-        options = obj.usageOptions()
+        for klass in test_classes:
+            klass.localOptions = self.options
 
-        options.parseOptions(cmd_line_options['subargs'])
-        obj.localOptions = options
-
-        if obj.inputFile:
-            obj.inputFilename = options[obj.inputFile[0]]
-
-        try:
-            log.debug("processing options")
-            tmp_test_case_object = obj()
-            tmp_test_case_object._checkRequiredOptions()
-
-        except usage.UsageError, e:
-            test_name = tmp_test_case_object.name
-            log.err("There was an error in running %s!" % test_name)
-            log.err("%s" % e)
-            options.opt_help()
-            raise usage.UsageError("Error in parsing command line args for %s" % test_name)
-
-        # who checks for root?
-        if obj.requiresRoot:
-            try:
+            test_instance = klass()
+            if test_instance.requiresRoot:
                 checkForRoot()
-            except NotRootError:
-                log.err("%s requires root to run" % obj.name)
-                sys.exit(1)
+            test_instance._checkRequiredOptions()
 
-        return obj
+            klass.inputs = test_instance.getInputProcessor()
 
 class NetTestCase(object):
     """
@@ -317,16 +259,16 @@ class NetTestCase(object):
         else:
             pass
 
-    def _checkRequiredOptions(self):
-        for required_option in self.requiredOptions:
-            log.debug("Checking if %s is present" % required_option)
-            if not self.localOptions[required_option]:
-                raise usage.UsageError("%s not specified!" % required_option)
+    def getInputProcessor(self):
+        """
+        This method must be called afterr
+        """
+        if self.inputFile:
+            self.inputFilename = self.localOptions[self.inputFile[0]]
 
-    def _processOptions(self):
-        if self.inputFilename:
             inputProcessor = self.inputProcessor
             inputFilename = self.inputFilename
+
             class inputProcessorIterator(object):
                 """
                 Here we convert the input processor generator into an iterator
@@ -334,11 +276,16 @@ class NetTestCase(object):
                 """
                 def __iter__(self):
                     return inputProcessor(inputFilename)
-            self.inputs = inputProcessorIterator()
 
-        return {'inputs': self.inputs,
-                'name': self.name, 'version': self.version
-               }
+            return inputProcessorIterator()
+
+        return iter(())
+
+    def _checkRequiredOptions(self):
+        for required_option in self.requiredOptions:
+            log.debug("Checking if %s is present" % required_option)
+            if not self.localOptions[required_option]:
+                raise usage.UsageError("%s not specified!" % required_option)
 
     def __repr__(self):
         return "<%s inputs=%s>" % (self.__class__, self.inputs)
diff --git a/tests/test_nettest.py b/tests/test_nettest.py
index 6ef214c..0c91a9e 100644
--- a/tests/test_nettest.py
+++ b/tests/test_nettest.py
@@ -8,15 +8,17 @@ from twisted.internet import defer, reactor
 from ooni.nettest import NetTest, InvalidOption, MissingRequiredOption
 from ooni.nettest import FailureToLoadNetTest
 from ooni.tasks import BaseTask
+from ooni.utils import NotRootError
 
 net_test_string = """
 from twisted.python import usage
 from ooni.nettest import NetTestCase
 
 class UsageOptions(usage.Options):
-    optParameters = [['spam', 's', 'ham']]
+    optParameters = [['spam', 's', None, 'ham']]
 
 class DummyTestCase(NetTestCase):
+
     usageOptions = UsageOptions
 
     def test_a(self):
@@ -30,6 +32,26 @@ net_test_root_required = net_test_string+"""
     requiresRoot = True
 """
 
+net_test_string_with_file = """
+from twisted.python import usage
+from ooni.nettest import NetTestCase
+
+class UsageOptions(usage.Options):
+    optParameters = [['spam', 's', None, 'ham']]
+
+class DummyTestCase(NetTestCase):
+    inputFile = ['file', 'f', None, 'The input File']
+
+    usageOptions = UsageOptions
+
+    def test_a(self):
+        self.report['bar'] = 'bar'
+
+    def test_b(self):
+        self.report['foo'] = 'foo'
+"""
+
+
 #XXX you should actually implement this
 net_test_with_required_option = net_test_string
 
@@ -60,7 +82,7 @@ class DummyMeasurementFailOnce(BaseTask):
 
 class DummyNetTest(NetTest):
     def __init__(self, num_measurements=1):
-        NetTest.__init__(self, StringIO(net_test_string), dummyInputs, dummyOptions)
+        NetTest.__init__(self, StringIO(net_test_string), dummyOptions)
         self.num_measurements = num_measurements
     def generateMeasurements(self):
         for i in range(self.num_measurements):
@@ -77,6 +99,11 @@ class DummyReporter(object):
         pass
 
 class TestNetTest(unittest.TestCase):
+    def setUp(self):
+        with open('dummyInputFile.txt', 'w') as f:
+            for i in range(10):
+                f.write("%s\n" % i)
+
     def assertCallable(self, thing):
         self.assertIn('__call__', dir(thing))
 
@@ -90,7 +117,7 @@ class TestNetTest(unittest.TestCase):
             f.write(net_test_string)
         f.close()
 
-        net_test_from_file = NetTest(net_test_file, dummyInputs,
+        net_test_from_file = NetTest(net_test_file,
                 dummyOptions, DummyReporter())
 
         test_methods = set()
@@ -111,7 +138,7 @@ class TestNetTest(unittest.TestCase):
         generated.
         """
         net_test_from_string = NetTest(StringIO(net_test_string),
-                dummyInputs, dummyOptions, DummyReporter())
+                dummyOptions, DummyReporter())
 
         test_methods = set()
         for test_class, test_method in net_test_from_string.test_cases:
@@ -123,9 +150,13 @@ class TestNetTest(unittest.TestCase):
 
         self.assertEqual(set(['test_a', 'test_b']), test_methods)
 
-    def test_load_with_option(self):
-        self.assertIsInstance(NetTest(StringIO(net_test_string),
-                    dummyInputs, dummyOptions, None), NetTest)
+    def dd_test_load_with_option(self):
+        net_test = NetTest(StringIO(net_test_string),
+                dummyOptions, None)
+
+        self.assertIsNotNone(net_test.usageOptions)
+        self.assertIsNotNone(net_test.usageOptions.optParameters)
+        self.assertIsInstance(net_test, NetTest)
 
     #def test_load_with_invalid_option(self):
     #    #XXX: raises TypeError??
@@ -134,7 +165,7 @@ class TestNetTest(unittest.TestCase):
 
     def test_load_with_required_option(self):
         self.assertIsInstance(NetTest(StringIO(net_test_with_required_option),
-                dummyInputs, dummyOptionsWithRequiredOptions, None), NetTest)
+                dummyOptionsWithRequiredOptions, None), NetTest)
 
     #def test_load_with_missing_required_option(self):
     #    #XXX: raises TypeError
@@ -142,15 +173,48 @@ class TestNetTest(unittest.TestCase):
     #            NetTest(StringIO(net_test_with_required_option), dummyInputs,
     #                dummyOptions, None))
 
-    def test_require_root_succeed(self):
-        #XXX: make root succeed
-        NetTest(StringIO(net_test_root_required),
-                dummyInputs, dummyOptions, None)
+
+    def test_net_test_inputs(self):
+        dummyOptionsWithFile = dict(dummyOptions)
+        dummyOptionsWithFile['file'] = 'dummyInputFile.txt'
+
+        net_test = NetTest(StringIO(net_test_string_with_file),
+            dummyOptionsWithFile, None)
+
+        for test_class, test_method in net_test.test_cases:
+            self.assertEqual(len(list(test_class.inputs)), 10)
+
+    def test_setup_local_options_in_test_cases(self):
+        net_test = NetTest(StringIO(net_test_string),
+            dummyOptions, None)
+
+        for test_class, test_method in net_test.test_cases:
+            self.assertEqual(test_class.localOptions, dummyOptions)
+
+    def test_generate_measurements_size(self):
+        dummyOptionsWithFile = dict(dummyOptions)
+        dummyOptionsWithFile['file'] = 'dummyInputFile.txt'
+
+        net_test = NetTest(StringIO(net_test_string_with_file),
+            dummyOptionsWithFile, None)
+
+        measurements = list(net_test.generateMeasurements())
+        self.assertEqual(len(measuremenets), 20)
+
+
+    def dd_test_require_root_succeed(self):
+        n = NetTest(StringIO(net_test_root_required),
+                dummyOptions, None)
+        for test_class, method in n.test_cases:
+            self.assertTrue(test_class.requiresRoot)
 
     def test_require_root_failed(self):
         #XXX: make root fail
-        NetTest(StringIO(net_test_root_required),
-                dummyInputs, dummyOptions, None)
+        try:
+            NetTest(StringIO(net_test_root_required),
+                    dummyOptions, None)
+        except NotRootError:
+            pass
 
     #def test_create_report_succeed(self):
     #    pass





More information about the tor-commits mailing list