[tor-commits] [ooni-probe/master] Implement fix for #493
art at torproject.org
art at torproject.org
Mon May 23 11:16:08 UTC 2016
commit b19cf6800c43c936a25ae452f85967b5be4c12d6
Author: Arturo Filastò <arturo at filasto.net>
Date: Tue May 3 19:18:37 2016 +0200
Implement fix for #493
* Use a class factory to generate NetTestCase subclasses with injected
localOptions.
* Major reworking of how localOptions are handled.
* Fixes to the NetTestCase object lifecycle to resolve issues with concurrent
tests running.
---
ooni/deck.py | 73 ++++----
ooni/director.py | 12 +-
ooni/errors.py | 2 +
ooni/nettest.py | 394 ++++++++++++++++++++++++--------------------
ooni/oonicli.py | 5 +-
ooni/reporter.py | 3 +-
ooni/tests/test_deck.py | 56 ++++++-
ooni/tests/test_director.py | 43 +++++
ooni/tests/test_nettest.py | 43 ++---
ooni/tests/test_oonicli.py | 7 +-
10 files changed, 370 insertions(+), 268 deletions(-)
diff --git a/ooni/deck.py b/ooni/deck.py
index 4049103..4b6d894 100644
--- a/ooni/deck.py
+++ b/ooni/deck.py
@@ -136,6 +136,7 @@ class Deck(InputFile):
log.msg("Skipping...")
continue
net_test_loader = NetTestLoader(test['options']['subargs'],
+ annotations=test['options'].get('annotations', {}),
test_file=nettest_path)
if test['options']['collector']:
net_test_loader.collector = test['options']['collector']
@@ -143,23 +144,13 @@ class Deck(InputFile):
def insert(self, net_test_loader):
""" Add a NetTestLoader to this test deck """
-
- def has_test_helper(missing_option):
- for rth in net_test_loader.requiredTestHelpers:
- if missing_option == rth['option']:
- return True
- return False
-
try:
net_test_loader.checkOptions()
if net_test_loader.requiresTor:
self.requiresTor = True
- except e.MissingRequiredOption as missing_options:
+ except e.MissingTestHelper:
if not self.bouncer:
raise
- for missing_option in missing_options.message:
- if not has_test_helper(missing_option):
- raise
self.requiresTor = True
if net_test_loader.collector and net_test_loader.collector.startswith('https://'):
@@ -192,26 +183,25 @@ class Deck(InputFile):
requires_collector = False
for net_test_loader in self.netTestLoaders:
nettest = {
- 'name': net_test_loader.testDetails['test_name'],
- 'version': net_test_loader.testDetails['test_version'],
+ 'name': net_test_loader.testName,
+ 'version': net_test_loader.testVersion,
'test-helpers': [],
'input-hashes': [x['hash'] for x in net_test_loader.inputFiles]
}
if not net_test_loader.collector and not self.no_collector:
requires_collector = True
- for th in net_test_loader.requiredTestHelpers:
- # {'name':'', 'option':'', 'test_class':''}
- if th['test_class'].localOptions[th['option']]:
- continue
- nettest['test-helpers'].append(th['name'])
+ if len(net_test_loader.missingTestHelpers) > 0:
requires_test_helpers = True
+ nettest['test-helpers'] += map(lambda x: x[1],
+ net_test_loader.missingTestHelpers)
required_nettests.append(nettest)
if not requires_test_helpers and not requires_collector:
defer.returnValue(None)
+ log.debug("Looking up {}".format(required_nettests))
response = yield self.oonibclient.lookupTestCollector(required_nettests)
provided_net_tests = response['net-tests']
@@ -227,17 +217,18 @@ class Deck(InputFile):
return net_test['collector'], net_test['test-helpers']
for net_test_loader in self.netTestLoaders:
- log.msg("Setting collector and test helpers for %s" % net_test_loader.testDetails['test_name'])
+ log.msg("Setting collector and test helpers for %s" %
+ net_test_loader.testName)
collector, test_helpers = \
- find_collector_and_test_helpers(net_test_loader.testDetails['test_name'],
- net_test_loader.testDetails['test_version'],
+ find_collector_and_test_helpers(net_test_loader.testName,
+ net_test_loader.testVersion,
net_test_loader.inputFiles)
- for th in net_test_loader.requiredTestHelpers:
- if not th['test_class'].localOptions[th['option']]:
- th['test_class'].localOptions[th['option']] = test_helpers[th['name']].encode('utf-8')
- net_test_loader.testHelpers[th['option']] = th['test_class'].localOptions[th['option']]
+ for option, name in net_test_loader.missingTestHelpers:
+ test_helper_address = test_helpers[name].encode('utf-8')
+ net_test_loader.localOptions[option] = test_helper_address
+ net_test_loader.testHelpers[option] = test_helper_address
if not net_test_loader.collector:
net_test_loader.collector = collector.encode('utf-8')
@@ -252,11 +243,8 @@ class Deck(InputFile):
if not net_test_loader.collector and not self.no_collector:
requires_collector.append(net_test_loader)
- for th in net_test_loader.requiredTestHelpers:
- # {'name':'', 'option':'', 'test_class':''}
- if th['test_class'].localOptions[th['option']]:
- continue
- required_test_helpers.append(th['name'])
+ required_test_helpers += map(lambda x: x[1],
+ net_test_loader.missingTestHelpers)
if not required_test_helpers and not requires_collector:
defer.returnValue(None)
@@ -265,33 +253,34 @@ class Deck(InputFile):
for net_test_loader in self.netTestLoaders:
log.msg("Setting collector and test helpers for %s" %
- net_test_loader.testDetails['test_name'])
+ net_test_loader.testName)
# Only set the collector if the no collector has been specified
# from the command line or via the test deck.
- if not net_test_loader.requiredTestHelpers and \
+ if len(net_test_loader.missingTestHelpers) == 0 and \
net_test_loader in requires_collector:
log.msg("Using the default collector: %s" %
response['default']['collector'])
net_test_loader.collector = response['default']['collector'].encode('utf-8')
continue
- for th in net_test_loader.requiredTestHelpers:
- # Only set helpers which are not already specified
- if th['name'] not in required_test_helpers:
- continue
- test_helper = response[th['name']]
- log.msg("Using this helper: %s" % test_helper)
- th['test_class'].localOptions[th['option']] = test_helper['address'].encode('utf-8')
+ for option, name in net_test_loader.missingTestHelpers:
+ test_helper_address = response[name]['address'].encode('utf-8')
+ test_helper_collector = \
+ response[name]['collector'].encode('utf-8')
+
+ log.msg("Using this helper: %s" % test_helper_address)
+ net_test_loader.localOptions[option] = test_helper_address
+ net_test_loader.testHelpers[option] = test_helper_address
if net_test_loader in requires_collector:
- net_test_loader.collector = test_helper['collector'].encode('utf-8')
+ net_test_loader.collector = test_helper_collector
@defer.inlineCallbacks
def fetchAndVerifyNetTestInput(self, net_test_loader):
""" fetch and verify a single NetTest's inputs """
log.debug("Fetching and verifying inputs")
for i in net_test_loader.inputFiles:
- if 'url' in i:
+ if i['url']:
log.debug("Downloading %s" % i['url'])
self.oonibclient.address = i['address']
@@ -305,4 +294,4 @@ class Deck(InputFile):
except AssertionError:
raise e.UnableToLoadDeckInput
- i['test_class'].localOptions[i['key']] = input_file.cached_file
+ i['test_options'][i['key']] = input_file.cached_file
diff --git a/ooni/director.py b/ooni/director.py
index a60b91e..02619c2 100644
--- a/ooni/director.py
+++ b/ooni/director.py
@@ -241,21 +241,19 @@ class Director(object):
net_test_loader:
an instance of :class:ooni.nettest.NetTestLoader
"""
- # Here we set the test details again since the geoip lookups may
- # not have already been done and probe_asn and probe_ip
- # are not set.
- net_test_loader.setTestDetails()
+ test_details = net_test_loader.getTestDetails()
+ test_cases = net_test_loader.getTestCases()
if self.allTestsDone.called:
self.allTestsDone = defer.Deferred()
if config.privacy.includepcap:
- self.startSniffing(net_test_loader.testDetails)
- report = Report(net_test_loader.testDetails, report_filename,
+ self.startSniffing(test_details)
+ report = Report(test_details, report_filename,
self.reportEntryManager, collector_address,
no_yamloo)
- net_test = NetTest(net_test_loader, report)
+ net_test = NetTest(test_cases, test_details, report)
net_test.director = self
yield net_test.report.open()
diff --git a/ooni/errors.py b/ooni/errors.py
index f98a09f..0412b50 100644
--- a/ooni/errors.py
+++ b/ooni/errors.py
@@ -195,6 +195,8 @@ class MissingRequiredOption(Exception):
def __str__(self):
return ','.join(self.message)
+class MissingTestHelper(MissingRequiredOption):
+ pass
class OONIUsageError(usage.UsageError):
def __init__(self, net_test_loader):
diff --git a/ooni/nettest.py b/ooni/nettest.py
index 9645400..074e8c2 100644
--- a/ooni/nettest.py
+++ b/ooni/nettest.py
@@ -8,6 +8,7 @@ from twisted.internet import defer
from twisted.trial.runner import filenameToModule
from twisted.python import usage, reflect
+from ooni import __version__ as ooniprobe_version
from ooni import otime
from ooni.tasks import Measurement
from ooni.utils import log, sanitize_options, randomStr
@@ -128,140 +129,198 @@ def getNetTestInformation(net_test_file):
return information
+def usageOptionsFactory(test_name, test_version):
+
+ class UsageOptions(usage.Options):
+ optParameters = []
+ optFlags = []
+
+ synopsis = "{} {} [options]".format(
+ os.path.basename(sys.argv[0]),
+ test_name
+ )
+
+ def opt_version(self):
+ """
+ Display the net_test version and exit.
+ """
+ print "{} version: {}".format(test_name, test_version)
+ sys.exit(0)
+
+ return UsageOptions
+
+def netTestCaseFactory(test_class, local_options):
+ class NetTestCaseWithLocalOptions(test_class):
+ localOptions = local_options
+ return NetTestCaseWithLocalOptions
+
+ONION_INPUT_REGEXP = re.compile("(httpo://[a-z0-9]{16}\.onion)/input/(["
+ "a-z0-9]{64})$")
+
class NetTestLoader(object):
method_prefix = 'test'
collector = None
yamloo = True
- requiresTor = False
- reportID = None
- def __init__(self, options, test_file=None, test_string=None):
- self.onionInputRegex = re.compile(
- "(httpo://[a-z0-9]{16}\.onion)/input/([a-z0-9]{64})$")
+ def __init__(self, options, test_file=None, test_string=None,
+ annotations={}):
self.options = options
- self.testCases = []
- self.annotations = {}
+ self.annotations = annotations
+
+ self.requiresTor = False
+
+ self.testName = ""
+ self.testVersion = ""
+ self.reportId = None
+
+ self.testHelpers = {}
+ self.missingTestHelpers = []
+ self.usageOptions = None
+ self.inputFiles = []
+
+ self._testCases = []
+ self.localOptions = None
if test_file:
self.loadNetTestFile(test_file)
elif test_string:
self.loadNetTestString(test_string)
- @property
- def requiredTestHelpers(self):
- required_test_helpers = []
- if not self.testCases:
- return required_test_helpers
-
- for test_class, test_methods in self.testCases:
- for option, name in test_class.requiredTestHelpers.items():
- required_test_helpers.append({
- 'name': name,
- 'option': option,
- 'test_class': test_class
- })
- return required_test_helpers
-
- @property
- def inputFiles(self):
- input_files = []
- if not self.testCases:
- return input_files
-
- for test_class, test_methods in self.testCases:
- if test_class.inputFile:
- key = test_class.inputFile[0]
- filename = test_class.localOptions[key]
- if not filename:
- continue
- input_file = {
- 'key': key,
- 'test_class': test_class
- }
- m = self.onionInputRegex.match(filename)
- if m:
- input_file['url'] = filename
- input_file['address'] = m.group(1)
- input_file['hash'] = m.group(2)
- else:
- input_file['filename'] = filename
- try:
- with open(filename) as f:
- h = sha256()
- for l in f:
- h.update(l)
- except:
- raise e.InvalidInputFile(filename)
- input_file['hash'] = h.hexdigest()
- input_files.append(input_file)
-
- return input_files
-
- def setTestDetails(self):
- from ooni import __version__ as software_version
-
- input_file_hashes = []
- for input_file in self.inputFiles:
- input_file_hashes.append(input_file['hash'])
-
- options = sanitize_options(self.options)
- self.testDetails = {
- 'test_start_time': otime.timestampNowLongUTC(),
+ def getTestDetails(self):
+ return {
'probe_asn': config.probe_ip.geodata['asn'],
'probe_cc': config.probe_ip.geodata['countrycode'],
'probe_ip': config.probe_ip.geodata['ip'],
'probe_city': config.probe_ip.geodata['city'],
+ 'software_name': 'ooniprobe',
+ 'software_version': ooniprobe_version,
+ 'options': sanitize_options(self.options),
+ 'annotations': self.annotations,
+ 'data_format_version': '0.2.0',
'test_name': self.testName,
'test_version': self.testVersion,
- 'software_name': 'ooniprobe',
- 'software_version': software_version,
- 'options': options,
- 'input_hashes': input_file_hashes,
- 'report_id': self.reportID,
'test_helpers': self.testHelpers,
- 'annotations': self.annotations,
- 'data_format_version': '0.2.0'
+ 'test_start_time': otime.timestampNowLongUTC(),
+ 'input_hashes': [input_file['hash']
+ for input_file in self.inputFiles],
+ 'report_id': self.reportId
}
- def _parseNetTestOptions(self, klass):
+ def getTestCases(self):
"""
- Helper method to assemble the options into a single UsageOptions object
+ Specialises the test_classes to include the local options.
+ :return:
"""
- usage_options = klass.usageOptions
+ test_cases = []
+ for test_class, test_method in self._testCases:
+ test_cases.append((netTestCaseFactory(test_class,
+ self.localOptions),
+ test_method))
+ return test_cases
+
+ def _accumulateInputFiles(self, test_class):
+ if not test_class.inputFile:
+ return
- if not hasattr(usage_options, 'optParameters'):
- usage_options.optParameters = []
+ key = test_class.inputFile[0]
+ filename = self.localOptions[key]
+ if not filename:
+ return
+
+ input_file = {
+ 'key': key,
+ 'test_options': self.localOptions,
+ 'hash': None,
+
+ 'url': None,
+ 'address': None,
+
+ 'filename': None
+ }
+ m = ONION_INPUT_REGEXP.match(filename)
+ if m:
+ input_file['url'] = filename
+ input_file['address'] = m.group(1)
+ input_file['hash'] = m.group(2)
else:
- for parameter in usage_options.optParameters:
+ input_file['filename'] = filename
+ try:
+ with open(filename) as f:
+ h = sha256()
+ for l in f:
+ h.update(l)
+ except:
+ raise e.InvalidInputFile(filename)
+ input_file['hash'] = h.hexdigest()
+ self.inputFiles.append(input_file)
+
+ def _accumulateTestOptions(self, test_class):
+ """
+ Accumulate the optParameters and optFlags for the NetTestCase class
+ into the usageOptions of the NetTestLoader.
+ """
+ if getattr(test_class.usageOptions, 'optParameters', None):
+ for parameter in test_class.usageOptions.optParameters:
+ # XXX should look into if this is still necessary, seems like
+ # something left over from a bug in some nettest.
+ # In theory optParameters should always have a length of 4.
if len(parameter) == 5:
parameter.pop()
+ self.usageOptions.optParameters.append(parameter)
- if klass.inputFile:
- usage_options.optParameters.append(klass.inputFile)
+ if getattr(test_class, 'inputFile', None):
+ self.usageOptions.optParameters.append(test_class.inputFile)
- if klass.baseParameters:
- for parameter in klass.baseParameters:
- usage_options.optParameters.append(parameter)
+ if getattr(test_class, 'baseParameters', None):
+ for parameter in test_class.baseParameters:
+ self.usageOptions.optParameters.append(parameter)
- if klass.baseFlags:
- if not hasattr(usage_options, 'optFlags'):
- usage_options.optFlags = []
- for flag in klass.baseFlags:
- usage_options.optFlags.append(flag)
+ if getattr(test_class, 'baseFlags', None):
+ for flag in test_class.baseFlags:
+ self.usageOptions.optFlags.append(flag)
- return usage_options
-
- @property
- def usageOptions(self):
- usage_options = None
- for test_class, test_method in self.testCases:
- if not usage_options:
- usage_options = self._parseNetTestOptions(test_class)
- else:
- if usage_options != test_class.usageOptions:
- raise e.IncoherentOptions(usage_options.__name__,
- test_class.usageOptions.__name__)
- return usage_options
+ def parseLocalOptions(self):
+ """
+ Parses the localOptions for the NetTestLoader.
+ """
+ self.localOptions = self.usageOptions()
+ try:
+ self.localOptions.parseOptions(self.options)
+ except usage.UsageError:
+ tb = sys.exc_info()[2]
+ raise e.OONIUsageError(self), None, tb
+
+ def _checkTestClassOptions(self, test_class):
+ if test_class.requiresRoot and not hasRawSocketPermission():
+ raise e.InsufficientPrivileges
+ if test_class.requiresTor:
+ self.requiresTor = True
+ self._checkRequiredOptions(test_class)
+ self._setTestHelpers(test_class)
+ test_instance = netTestCaseFactory(test_class, self.localOptions)()
+ test_instance.requirements()
+
+ def _setTestHelpers(self, test_class):
+ for option, name in test_class.requiredTestHelpers.items():
+ if self.localOptions.get(option, None):
+ self.testHelpers[option] = self.localOptions[option]
+
+ def _checkRequiredOptions(self, test_class):
+ missing_options = []
+ for required_option in test_class.requiredOptions:
+ log.debug("Checking if %s is present" % required_option)
+ if required_option not in self.localOptions or \
+ self.localOptions[required_option] is None:
+ missing_options.append(required_option)
+ missing_test_helpers = [opt in test_class.requiredTestHelpers.keys()
+ for opt in missing_options]
+ if len(missing_test_helpers) and all(missing_test_helpers):
+ self.missingTestHelpers = map(lambda x:
+ (x, test_class.requiredTestHelpers[x]),
+ missing_options)
+ raise e.MissingTestHelper(missing_options, test_class)
+ elif missing_options:
+ raise e.MissingRequiredOption(missing_options, test_class)
def loadNetTestString(self, net_test_string):
"""
@@ -280,12 +339,12 @@ class NetTestLoader(object):
test_cases = []
exec net_test_file_object.read() in ns
for item in ns.itervalues():
- test_cases.extend(self._get_test_methods(item))
+ test_cases.extend(self._getTestMethods(item))
if not test_cases:
raise e.NoTestCasesFound
- self.setupTestCases(test_cases)
+ self._setupTestCases(test_cases)
def loadNetTestFile(self, net_test_file):
"""
@@ -294,27 +353,26 @@ class NetTestLoader(object):
test_cases = []
module = filenameToModule(net_test_file)
for __, item in getmembers(module):
- test_cases.extend(self._get_test_methods(item))
+ test_cases.extend(self._getTestMethods(item))
if not test_cases:
raise e.NoTestCasesFound
- self.setupTestCases(test_cases)
+ self._setupTestCases(test_cases)
- def setupTestCases(self, test_cases):
+ def _setupTestCases(self, test_cases):
"""
Creates all the necessary test_cases (a list of tuples containing the
NetTestCase (test_class, test_method))
example:
- [(test_classA, test_method1),
- (test_classA, test_method2),
- (test_classA, test_method3),
- (test_classA, test_method4),
- (test_classA, test_method5),
-
- (test_classB, test_method1),
- (test_classB, test_method2)]
+ [(test_classA, [test_method1,
+ test_method2,
+ test_method3,
+ test_method4,
+ test_method5]),
+ (test_classB, [test_method1,
+ test_method2])]
Note: the inputs must be valid for test_classA and test_classB.
@@ -323,46 +381,37 @@ class NetTestLoader(object):
generate the test_cases.
"""
test_class, _ = test_cases[0]
- self.testVersion = test_class.version
self.testName = test_class_name_to_name(test_class.name)
- self.testCases = test_cases
- self.testClasses = set([])
- self.testHelpers = {}
+ self.testVersion = test_class.version
+ self._testCases = test_cases
- if config.reports.unique_id is True and not self.reportID:
- self.reportID = randomStr(64)
+ self.usageOptions = usageOptionsFactory(self.testName,
+ self.testVersion)
- for test_class, test_method in self.testCases:
- self.testClasses.add(test_class)
+ if config.reports.unique_id is True:
+ self.reportId = randomStr(64)
+
+ for test_class, test_methods in self._testCases:
+ self._accumulateTestOptions(test_class)
def checkOptions(self):
- """
- Call processTest and processOptions methods of each NetTestCase
- """
- for klass in self.testClasses:
- options = self.usageOptions()
+ self.parseLocalOptions()
+ test_options_exc = None
+ usage_options = self._testCases[0][0].usageOptions
+ for test_class, test_methods in self._testCases:
try:
- options.parseOptions(self.options)
- except usage.UsageError:
- tb = sys.exc_info()[2]
- raise e.OONIUsageError(self), None, tb
-
- if options:
- klass.localOptions = options
- # XXX this class all needs to be refactored and this is kind of a
- # hack.
- self.setTestDetails()
-
- test_instance = klass()
- if test_instance.requiresRoot and not hasRawSocketPermission():
- raise e.InsufficientPrivileges
- if test_instance.requiresTor:
- self.requiresTor = True
- test_instance.requirements()
- test_instance._checkRequiredOptions()
- test_instance._checkValidOptions()
-
- def _get_test_methods(self, item):
+ self._accumulateInputFiles(test_class)
+ self._checkTestClassOptions(test_class)
+ if usage_options != test_class.usageOptions:
+ raise e.IncoherentOptions(usage_options.__name__,
+ test_class.usageOptions.__name__)
+ except Exception as exc:
+ test_options_exc = exc
+
+ if test_options_exc is not None:
+ raise test_options_exc
+
+ def _getTestMethods(self, item):
"""
Look for test_ methods in subclasses of NetTestCase
"""
@@ -432,7 +481,7 @@ class NetTestState(object):
class NetTest(object):
director = None
- def __init__(self, net_test_loader, report):
+ def __init__(self, test_cases, test_details, report):
"""
net_test_loader:
an instance of :class:ooni.nettest.NetTestLoader containing
@@ -442,9 +491,11 @@ class NetTest(object):
an instance of :class:ooni.reporter.Reporter
"""
self.report = report
- self.testCases = net_test_loader.testCases
- self.testClasses = net_test_loader.testClasses
- self.testDetails = net_test_loader.testDetails
+
+ self.testDetails = test_details
+ self.testCases = test_cases
+
+ self.testInstances = []
self.summary = {}
@@ -459,11 +510,18 @@ class NetTest(object):
def __str__(self):
return ' '.join(tc.name for tc, _ in self.testCases)
+ def uniqueClasses(self):
+ classes = []
+ for test_class, test_method in self.testCases:
+ if test_class not in classes:
+ classes.append(test_class)
+ return classes
+
def doneNetTest(self, result):
if self.summary:
print "Summary for %s" % self.testDetails['test_name']
print "------------" + "-"*len(self.testDetails['test_name'])
- for test_class in self.testClasses:
+ for test_class in self.uniqueClasses():
test_instance = test_class()
test_instance.displaySummary(self.summary)
if self.testDetails["report_id"]:
@@ -510,12 +568,10 @@ class NetTest(object):
@defer.inlineCallbacks
def initializeInputProcessor(self):
- for test_class, _ in self.testCases:
+ for test_class, test_method in self.testCases:
test_class.inputs = yield defer.maybeDeferred(
test_class().getInputProcessor
)
- if not test_class.inputs:
- test_class.inputs = [None]
def generateMeasurements(self):
"""
@@ -531,7 +587,7 @@ class NetTest(object):
test_instance._setUp()
test_instance.summary = self.summary
for method in test_methods:
- log.debug("Running %s %s" % (test_class, method))
+ log.debug("Running %s %s" % (test_instance, method))
measurement = self.makeMeasurement(
test_instance,
method,
@@ -631,8 +687,6 @@ class NetTestCase(object):
inputFile = None
inputFilename = None
- report = {}
-
usageOptions = usage.Options
optParameters = None
@@ -766,23 +820,7 @@ class NetTestCase(object):
if self.inputs:
return self.inputs
- return None
-
- def _checkValidOptions(self):
- for option in self.localOptions:
- if option not in self.usageOptions():
- if not self.inputFile or option not in self.inputFile:
- raise e.InvalidOption
-
- def _checkRequiredOptions(self):
- missing_options = []
- for required_option in self.requiredOptions:
- log.debug("Checking if %s is present" % required_option)
- if required_option not in self.localOptions or \
- self.localOptions[required_option] is None:
- missing_options.append(required_option)
- if missing_options:
- raise e.MissingRequiredOption(missing_options, self)
+ return [None]
def __repr__(self):
return "<%s inputs=%s>" % (self.__class__, self.inputs)
diff --git a/ooni/oonicli.py b/ooni/oonicli.py
index 9327afc..e2d78e1 100644
--- a/ooni/oonicli.py
+++ b/ooni/oonicli.py
@@ -261,7 +261,8 @@ def createDeck(global_options, url=None):
if any(global_options['subargs']):
args = global_options['subargs'] + args
net_test_loader = NetTestLoader(args,
- test_file=test_file)
+ test_file=test_file,
+ annotations=global_options['annotations'])
if global_options['collector']:
net_test_loader.collector = global_options['collector']
deck.insert(net_test_loader)
@@ -328,8 +329,6 @@ def runTestWithDirector(director, global_options, url=None, start_tor=True):
collector_address = setupCollector(global_options,
net_test_loader.collector)
- net_test_loader.annotations = global_options['annotations']
-
yield director.startNetTest(net_test_loader,
global_options['reportfile'],
collector_address,
diff --git a/ooni/reporter.py b/ooni/reporter.py
index 2ee8468..63d6f15 100644
--- a/ooni/reporter.py
+++ b/ooni/reporter.py
@@ -626,10 +626,9 @@ class Report(object):
self.collector_address)
def created(report_id):
- self.reportID = report_id
- self.test_details['report_id'] = report_id
if not self.oonib_reporter:
return
+ self.test_details['report_id'] = report_id
return self.report_log.created(self.report_filename,
self.collector_address,
report_id)
diff --git a/ooni/tests/test_deck.py b/ooni/tests/test_deck.py
index d82c0eb..7b423af 100644
--- a/ooni/tests/test_deck.py
+++ b/ooni/tests/test_deck.py
@@ -44,6 +44,32 @@ class BaseTestCase(unittest.TestCase):
test_file: manipulation/http_invalid_request_line
testdeck: null
"""
+ self.dummy_deck_content_with_many_tests = """- options:
+ collector: null
+ help: 0
+ logfile: null
+ no-default-reporter: 0
+ parallelism: null
+ pcapfile: null
+ reportfile: null
+ resume: 0
+ subargs: [-b, "1.1.1.1"]
+ test_file: manipulation/http_invalid_request_line
+ testdeck: null
+- options:
+ collector: null
+ help: 0
+ logfile: null
+ no-default-reporter: 0
+ parallelism: null
+ pcapfile: null
+ reportfile: null
+ resume: 0
+ subargs: [-b, "2.2.2.2"]
+ test_file: manipulation/http_invalid_request_line
+ testdeck: null
+"""
+
class TestInputFile(BaseTestCase):
@@ -127,10 +153,32 @@ class TestDeck(BaseTestCase):
deck.bouncer = "httpo://foo.onion"
deck.oonibclient = MockOONIBClient()
deck.loadDeck(self.deck_file)
+
+ self.assertEqual(len(deck.netTestLoaders[0].missingTestHelpers), 1)
+
yield deck.lookupTestHelpers()
- assert deck.netTestLoaders[0].collector == 'httpo://thirteenchars1234.onion'
+ self.assertEqual(deck.netTestLoaders[0].collector,
+ 'httpo://thirteenchars1234.onion')
+
+ self.assertEqual(deck.netTestLoaders[0].localOptions['backend'],
+ '127.0.0.1')
+
+
+ def test_deck_with_many_tests(self):
+ os.remove(self.deck_file)
+ deck_hash = sha256(self.dummy_deck_content_with_many_tests).hexdigest()
+ self.deck_file = os.path.join(self.cwd, deck_hash)
+ with open(self.deck_file, 'w+') as f:
+ f.write(self.dummy_deck_content_with_many_tests)
+ deck = Deck(decks_directory=".")
+ deck.loadDeck(self.deck_file)
- required_test_helpers = deck.netTestLoaders[0].requiredTestHelpers
- assert len(required_test_helpers) == 1
- assert required_test_helpers[0]['test_class'].localOptions['backend'] == '127.0.0.1'
+ self.assertEqual(
+ deck.netTestLoaders[0].localOptions['backend'],
+ '1.1.1.1'
+ )
+ self.assertEqual(
+ deck.netTestLoaders[1].localOptions['backend'],
+ '2.2.2.2'
+ )
diff --git a/ooni/tests/test_director.py b/ooni/tests/test_director.py
index 61d504c..5875ccb 100644
--- a/ooni/tests/test_director.py
+++ b/ooni/tests/test_director.py
@@ -2,6 +2,7 @@ from mock import patch, MagicMock
from ooni.settings import config
from ooni.director import Director
+from ooni.nettest import NetTestLoader
from ooni.tests.bases import ConfigTestCase
from twisted.internet import defer
@@ -9,6 +10,31 @@ from twisted.trial import unittest
from txtorcon import TorControlProtocol
+test_failing_twice = """
+from twisted.internet import defer, reactor
+from ooni.nettest import NetTestCase
+
+class TestFailingTwice(NetTestCase):
+ inputs = ["spam-{}".format(idx) for idx in range(50)]
+
+ def setUp(self):
+ self.summary[self.input] = self.summary.get(self.input, 0)
+
+ def test_a(self):
+ run_count = self.summary[self.input]
+ delay = float(self.input.split("-")[1])/1000
+ d = defer.Deferred()
+ def callback():
+ self.summary[self.input] += 1
+ if run_count < 3:
+ d.errback(Exception("Failing"))
+ else:
+ d.callback(self.summary[self.input])
+
+ reactor.callLater(delay, callback)
+ return d
+"""
+
proto = MagicMock()
proto.tor_protocol = TorControlProtocol()
@@ -52,6 +78,23 @@ class TestDirector(ConfigTestCase):
return director_start_tor()
+ def test_run_test_fails_twice(self):
+ finished = defer.Deferred()
+
+ def net_test_done(net_test):
+ summary_items = net_test.summary.items()
+ self.assertEqual(len(summary_items), 50)
+ for input_name, run_count in summary_items:
+ self.assertEqual(run_count, 3)
+ finished.callback(None)
+
+ net_test_loader = NetTestLoader(('spam','ham'))
+ net_test_loader.loadNetTestString(test_failing_twice)
+ director = Director()
+ director.netTestDone = net_test_done
+ director.startNetTest(net_test_loader, None, no_yamloo=True)
+ return finished
+
class TestStartSniffing(unittest.TestCase):
def setUp(self):
diff --git a/ooni/tests/test_nettest.py b/ooni/tests/test_nettest.py
index e93111b..1108f11 100644
--- a/ooni/tests/test_nettest.py
+++ b/ooni/tests/test_nettest.py
@@ -186,13 +186,6 @@ class TestNetTest(unittest.TestCase):
uniq_test_methods.add(test_method)
self.assertEqual(set(['test_a', 'test_b']), uniq_test_methods)
- def verifyClasses(self, test_cases, control_classes):
- actual_classes = set()
- for test_class, test_methods in test_cases:
- actual_classes.add(test_class.__name__)
-
- self.assertEqual(actual_classes, control_classes)
-
def test_load_net_test_from_file(self):
"""
Given a file verify that the net test cases are properly
@@ -206,7 +199,7 @@ class TestNetTest(unittest.TestCase):
ntl = NetTestLoader(dummyArgs)
ntl.loadNetTestFile(net_test_file)
- self.verifyMethods(ntl.testCases)
+ self.verifyMethods(ntl.getTestCases())
os.unlink(net_test_file)
def test_load_net_test_from_str(self):
@@ -217,24 +210,21 @@ class TestNetTest(unittest.TestCase):
ntl = NetTestLoader(dummyArgs)
ntl.loadNetTestString(net_test_string)
- self.verifyMethods(ntl.testCases)
+ self.verifyMethods(ntl.getTestCases())
def test_load_net_test_multiple(self):
ntl = NetTestLoader(dummyArgs)
ntl.loadNetTestString(double_net_test_string)
-
- self.verifyMethods(ntl.testCases)
- self.verifyClasses(ntl.testCases, set(('DummyTestCaseA', 'DummyTestCaseB')))
-
+ test_cases = ntl.getTestCases()
+ self.verifyMethods(test_cases)
ntl.checkOptions()
def test_load_net_test_multiple_different_options(self):
ntl = NetTestLoader(dummyArgs)
ntl.loadNetTestString(double_different_options_net_test_string)
- self.verifyMethods(ntl.testCases)
- self.verifyClasses(ntl.testCases, set(('DummyTestCaseA', 'DummyTestCaseB')))
-
+ test_cases = ntl.getTestCases()
+ self.verifyMethods(test_cases)
self.assertRaises(IncoherentOptions, ntl.checkOptions)
def test_load_with_option(self):
@@ -242,7 +232,7 @@ class TestNetTest(unittest.TestCase):
ntl.loadNetTestString(net_test_string)
self.assertIsInstance(ntl, NetTestLoader)
- for test_klass, test_meth in ntl.testCases:
+ for test_klass, test_meth in ntl.getTestCases():
for option in dummyOptions.keys():
self.assertIn(option, test_klass.usageOptions())
@@ -266,34 +256,29 @@ class TestNetTest(unittest.TestCase):
def test_net_test_inputs(self):
ntl = NetTestLoader(dummyArgsWithFile)
ntl.loadNetTestString(net_test_string_with_file)
-
ntl.checkOptions()
- nt = NetTest(ntl, None)
+ nt = NetTest(ntl.getTestCases(), ntl.getTestDetails(), None)
nt.initializeInputProcessor()
# XXX: if you use the same test_class twice you will have consumed all
# of its inputs!
tested = set([])
- for test_class, test_method in ntl.testCases:
- if test_class not in tested:
- tested.update([test_class])
- self.assertEqual(len(list(test_class.inputs)), 10)
+ for test_instance, test_method, inputs in nt.testInstances:
+ self.assertEqual(len(list(inputs)), 10)
def test_setup_local_options_in_test_cases(self):
ntl = NetTestLoader(dummyArgs)
ntl.loadNetTestString(net_test_string)
ntl.checkOptions()
-
- for test_class, test_method in ntl.testCases:
- self.assertEqual(test_class.localOptions, dummyOptions)
+ self.assertEqual(dict(ntl.localOptions), dummyOptions)
def test_generate_measurements_size(self):
ntl = NetTestLoader(dummyArgsWithFile)
ntl.loadNetTestString(net_test_string_with_file)
-
ntl.checkOptions()
- net_test = NetTest(ntl, None)
+
+ net_test = NetTest(ntl.getTestCases(), ntl.getTestDetails(), None)
net_test.initializeInputProcessor()
measurements = list(net_test.generateMeasurements())
@@ -321,7 +306,7 @@ class TestNetTest(unittest.TestCase):
ntl = NetTestLoader(dummyArgs)
ntl.loadNetTestString(net_test_root_required)
- for test_class, method in ntl.testCases:
+ for test_class, methods in ntl.getTestCases():
self.assertTrue(test_class.requiresRoot)
diff --git a/ooni/tests/test_oonicli.py b/ooni/tests/test_oonicli.py
index f5c8545..16e3f77 100644
--- a/ooni/tests/test_oonicli.py
+++ b/ooni/tests/test_oonicli.py
@@ -62,9 +62,7 @@ class TestRunDirector(ConfigTestCase):
super(TestRunDirector, self).setUp()
if not is_internet_connected():
self.skipTest("You must be connected to the internet to run this test")
- elif not hasRawSocketPermission():
- self.skipTest("You must run this test as root or have the capabilities "
- "cap_net_admin,cap_net_raw+eip")
+
config.tor.socks_port = 9050
config.tor.control_port = None
self.filenames = ['example-input.txt']
@@ -165,6 +163,9 @@ class TestRunDirector(ConfigTestCase):
@defer.inlineCallbacks
def test_sniffing_activated(self):
+ if not hasRawSocketPermission():
+ self.skipTest("You must run this test as root or have the "
+ "capabilities cap_net_admin,cap_net_raw+eip")
self.skipTest("Not properly set packet capture?")
filename = os.path.abspath('test_report.pcap')
self.filenames.append(filename)
More information about the tor-commits
mailing list