commit 376715a8350a0a52c57a2fa6f8d68b5d37eafb8d Author: aagbsn aagbsn@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