[tor-commits] [ooni-probe/master] Do some important refactoring of runner and nettest

art at torproject.org art at torproject.org
Wed Nov 7 19:59:45 UTC 2012


commit 6c08785800e06fed821d5928ac558726d733bbbd
Author: Arturo Filastò <arturo at filasto.net>
Date:   Wed Nov 7 20:57:31 2012 +0100

    Do some important refactoring of runner and nettest
    * Refactor dnstamper to usage usage.Options
---
 before_i_commit.sh         |    1 +
 nettests/core/dnstamper.py |   28 ++++++++++++++++------------
 nettests/core/http_host.py |    2 ++
 ooni/nettest.py            |   40 +++++++++++++++++++++++++++++++++++-----
 ooni/runner.py             |   18 +++++++++++-------
 5 files changed, 65 insertions(+), 24 deletions(-)

diff --git a/before_i_commit.sh b/before_i_commit.sh
index 1bf96a3..e27fbd2 100755
--- a/before_i_commit.sh
+++ b/before_i_commit.sh
@@ -35,3 +35,4 @@ echo "Read through the log file and fix it."
 echo "If you are having some problems fixing some things that have to do with"
 echo "the core of OONI, let's first discuss it on IRC, or open a ticket"
 
+rm -f *yamloo
diff --git a/nettests/core/dnstamper.py b/nettests/core/dnstamper.py
index 8a59709..bc57f62 100644
--- a/nettests/core/dnstamper.py
+++ b/nettests/core/dnstamper.py
@@ -27,6 +27,15 @@ from twisted.names.error import DNSQueryRefusedError
 from ooni import nettest
 from ooni.utils import log
 
+class UsageOptions(usage.Options):
+    optParameters = [['backend', 'b', '8.8.8.8',
+                        'The OONI backend that runs the DNS resolver'],
+                     ['backendport', 'p', 53,
+                        'The port of the good DNS resolver'],
+                     ['testresolvers', 't', None,
+                        'file containing list of DNS resolvers to test against']
+                    ]
+
 class DNSTamperTest(nettest.NetTestCase):
 
     name = "DNS tamper"
@@ -38,13 +47,8 @@ class DNSTamperTest(nettest.NetTestCase):
     inputFile = ['file', 'f', None,
                  'Input file of list of hostnames to attempt to resolve']
 
-    optParameters = [['controlresolver', 'b', '8.8.8.8',
-                      'The OONI backend that runs the DNS resolver'],
-                     ['controlresolver-port', 'p', 53,
-                      'The port of the good DNS resolver'],
-                     ['testresolvers', 't', None,
-                      'file containing list of DNS resolvers to test against']
-                    ]
+    usageOptions = UsageOptions
+    requiredOptions = ['backend', 'backendport', 'file', 'testresolvers']
 
     def setUp(self):
         self.report['test_lookups'] = {}
@@ -88,7 +92,7 @@ class DNSTamperTest(nettest.NetTestCase):
             all_a.append(lookup)
 
         if resolver_address == 'control':
-            self.report['control_server'] = self.localOptions['controlresolver']
+            self.report['control_server'] = self.localOptions['backend']
             self.report['control_lookup'] = all_a
             self.control_a_lookups = a_a
         else:
@@ -160,8 +164,8 @@ class DNSTamperTest(nettest.NetTestCase):
         list_of_ds = []
         hostname = self.input
         dns_query = [dns.Query(hostname, dns.IN, dns.A)]
-        dns_server = [(self.localOptions['controlresolver'],
-                       self.localOptions['controlresolver-port'])]
+        dns_server = [(self.localOptions['backend'],
+                       self.localOptions['backendport'])]
 
         resolver = Resolver(servers=dns_server)
 
@@ -206,8 +210,8 @@ class DNSTamperTest(nettest.NetTestCase):
         """
         log.msg("Doing the reverse lookups %s" % self.input)
         list_of_ds = []
-        dns_server = [(self.localOptions['controlresolver'],
-                       self.localOptions['controlresolver-port'])]
+        dns_server = [(self.localOptions['backend'],
+                       self.localOptions['backendport'])]
 
         resolver = Resolver(servers=dns_server)
 
diff --git a/nettests/core/http_host.py b/nettests/core/http_host.py
index 07c7019..137d3b8 100644
--- a/nettests/core/http_host.py
+++ b/nettests/core/http_host.py
@@ -27,6 +27,8 @@ class HTTPHost(httpt.HTTPTest):
     author = "Arturo Filastò"
     version = 0.1
 
+    usageOptions = UsageOptions
+
     inputFile = ['urls', 'f', None, 'Urls file']
 
     def test_send_host_header(self):
diff --git a/ooni/nettest.py b/ooni/nettest.py
index dc7188d..a8d3560 100644
--- a/ooni/nettest.py
+++ b/ooni/nettest.py
@@ -115,6 +115,9 @@ class NetTestCase(unittest.TestCase):
 
     * usageOptions: a subclass of twisted.python.usage.Options for more advanced command line arguments fun.
 
+    * requiredOptions: a list containing the name of the options that are
+                       required for proper running of a test.
+
     """
     name = "I Did Not Change The Name"
     author = "Jane Doe <foo at example.com>"
@@ -128,7 +131,9 @@ class NetTestCase(unittest.TestCase):
 
     optFlags = None
     optParameters = None
+
     usageOptions = None
+    requiredOptions = []
 
     requiresRoot = False
 
@@ -149,13 +154,31 @@ class NetTestCase(unittest.TestCase):
             return unittest.TestCase.deferSetUp(self, ignored, result)
 
     def inputProcessor(self, fp):
+        """
+        You may replace this with your own custom input processor. It takes as
+        input a file descriptor so remember to close it when you are done.
+
+        This can be useful when you have some input data that is in a certain
+        format and you want to set the input attribute of the test to something
+        that you will be able to properly process.
+
+        For example you may wish to have an input processor that will allow you
+        to ignore comments in files. This can be easily achieved like so:
+
+            for x in fp.xreadlines():
+                if x.startswith("#"):
+                    continue
+                yield x.strip()
+            fp.close()
+
+        Other fun stuff is also possible.
+        """
         log.debug("Running default input processor")
-        for x in fp.readlines():
+        for x in fp.xreadlines():
             yield x.strip()
         fp.close()
 
-    def getOptions(self):
-        log.debug("Getting options for test")
+    def _processOptions(self, options=None):
         if self.inputFile:
             try:
                 assert isinstance(self.inputFile, str)
@@ -163,15 +186,22 @@ class NetTestCase(unittest.TestCase):
                 log.err(ae)
             else:
                 if os.path.isfile(self.inputFile):
-                    print self.inputFile
                     fp = open(self.inputFile)
                     self.inputs = self.inputProcessor(fp)
         elif not self.inputs[0]:
             pass
         elif self.inputFile:
             raise usage.UsageError("No input file specified!")
+
+        # XXX this is a bit hackish
+        if options:
+            for required_option in self.requiredOptions:
+                log.debug("Checking if %s is present" % required_option)
+                if not options[required_option]:
+                    raise usage.UsageError("%s not specified!" % required_option)
+
         # XXX perhaps we may want to name and version to be inside of a
-        # different object that is not called options.
+        # different method that is not called options.
         return {'inputs': self.inputs,
                 'name': self.name,
                 'version': self.version}
diff --git a/ooni/runner.py b/ooni/runner.py
index 29ed831..b4d9560 100644
--- a/ooni/runner.py
+++ b/ooni/runner.py
@@ -51,9 +51,11 @@ def processTest(obj, cmd_line_options):
             obj.optParameters.append(input_file)
 
         if obj.usageOptions:
-            log.debug("Got advanced options")
+            if input_file:
+                obj.usageOptions.optParameters.append(input_file)
             options = obj.usageOptions()
         else:
+            # XXX this as suggested by isis should be removed.
             log.debug("Got optParameters")
             class Options(usage.Options):
                 optParameters = obj.optParameters
@@ -68,10 +70,14 @@ def processTest(obj, cmd_line_options):
 
         if input_file:
             obj.inputFile = options[input_file[0]]
+
         try:
-            tmp_obj = obj()
-            tmp_obj.getOptions()
-        except usage.UsageError:
+            tmp_test_case_object = obj()
+            tmp_test_case_object._processOptions(options)
+
+        except usage.UsageError, e:
+            print "There was an error in running %s!" % tmp_test_case_object.name
+            print "%s" % e
             options.opt_help()
 
     return obj
@@ -115,9 +121,7 @@ def makeTestCases(klass, tests, method_prefix):
 def loadTestsAndOptions(classes, cmd_line_options):
     """
     Takes a list of test classes and returns their testcases and options.
-    Legacy tests will be adapted.
     """
-
     method_prefix = 'test'
     options = []
     test_cases = []
@@ -129,7 +133,7 @@ def loadTestsAndOptions(classes, cmd_line_options):
             test_cases.append(cases)
         try:
             k = klass()
-            opts = k.getOptions()
+            opts = k._processOptions()
             options.append(opts)
         except AttributeError, ae:
             options.append([])



More information about the tor-commits mailing list