commit 6c08785800e06fed821d5928ac558726d733bbbd Author: Arturo Filastò <arturo@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@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([])