commit da51f6d069ac55ebf430ebb0adfa6a70110fda68
Author: Arturo Filastò <arturo(a)filasto.net>
Date: Tue Apr 12 17:09:51 2016 +0200
Clean up how we handle the reportId and remove unused functions
---
ooni/deck.py | 52 ++++-------------------------------
ooni/director.py | 8 +-----
ooni/reporter.py | 66 +++++++++++++++++++--------------------------
ooni/tasks.py | 3 ---
ooni/tests/mocks.py | 19 +++++++++++++
ooni/tests/test_deck.py | 4 +--
ooni/tests/test_reporter.py | 2 +-
7 files changed, 55 insertions(+), 99 deletions(-)
diff --git a/ooni/deck.py b/ooni/deck.py
index 4b6d894..b13385d 100644
--- a/ooni/deck.py
+++ b/ooni/deck.py
@@ -171,10 +171,10 @@ class Deck(InputFile):
if self.bouncer:
log.msg("Looking up collector and test helpers")
- yield self.lookupCollector()
+ yield self.lookupCollectorAndTestHelpers()
@defer.inlineCallbacks
- def lookupCollector(self):
+ def lookupCollectorAndTestHelpers(self):
self.oonibclient.address = self.bouncer
required_nettests = []
@@ -221,9 +221,9 @@ class Deck(InputFile):
net_test_loader.testName)
collector, test_helpers = \
- find_collector_and_test_helpers(net_test_loader.testName,
- net_test_loader.testVersion,
- net_test_loader.inputFiles)
+ find_collector_and_test_helpers(test_name=net_test_loader.testName,
+ test_version=net_test_loader.testVersion,
+ input_files=net_test_loader.inputFiles)
for option, name in net_test_loader.missingTestHelpers:
test_helper_address = test_helpers[name].encode('utf-8')
@@ -234,48 +234,6 @@ class Deck(InputFile):
net_test_loader.collector = collector.encode('utf-8')
@defer.inlineCallbacks
- def lookupTestHelpers(self):
- self.oonibclient.address = self.bouncer
-
- required_test_helpers = []
- requires_collector = []
- for net_test_loader in self.netTestLoaders:
- if not net_test_loader.collector and not self.no_collector:
- requires_collector.append(net_test_loader)
-
- required_test_helpers += map(lambda x: x[1],
- net_test_loader.missingTestHelpers)
-
- if not required_test_helpers and not requires_collector:
- defer.returnValue(None)
-
- response = yield self.oonibclient.lookupTestHelpers(required_test_helpers)
-
- for net_test_loader in self.netTestLoaders:
- log.msg("Setting collector and test helpers for %s" %
- 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 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 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
-
- @defer.inlineCallbacks
def fetchAndVerifyNetTestInput(self, net_test_loader):
""" fetch and verify a single NetTest's inputs """
log.debug("Fetching and verifying inputs")
diff --git a/ooni/director.py b/ooni/director.py
index 13a4503..64b5a98 100644
--- a/ooni/director.py
+++ b/ooni/director.py
@@ -253,16 +253,10 @@ class Director(object):
self.reportEntryManager, collector_address,
no_yamloo)
+ yield report.open()
net_test = NetTest(test_cases, test_details, report)
net_test.director = self
- yield net_test.report.open()
-
- # XXX this needs some serious refactoring
- net_test_loader.reportID = report.reportID
- net_test.reportID = report.reportID
- net_test.testDetails['report_id'] = report.reportID
-
yield net_test.initializeInputProcessor()
try:
self.activeNetTests.append(net_test)
diff --git a/ooni/reporter.py b/ooni/reporter.py
index 4318fd5..aa6f5ee 100644
--- a/ooni/reporter.py
+++ b/ooni/reporter.py
@@ -163,7 +163,10 @@ class YAMLReporter(OReporter):
if not os.path.isdir(report_destination):
raise errors.InvalidDestination
- report_filename = generate_filename(test_details, filename=report_filename, prefix='report', extension='yamloo')
+ report_filename = generate_filename(test_details,
+ filename=report_filename,
+ prefix='report',
+ extension='yamloo')
report_path = os.path.join(self.reportDestination, report_filename)
@@ -239,7 +242,7 @@ class OONIBReporter(OReporter):
self.collectorAddress = collector_address
self.validateCollectorAddress()
- self.reportID = None
+ self.reportId = None
self.supportedFormats = ["yaml"]
if self.collectorAddress.startswith('https://'):
@@ -302,7 +305,7 @@ class OONIBReporter(OReporter):
def writeReportEntry(self, entry):
log.debug("Writing report with OONIB reporter")
- url = self.collectorAddress + '/report/' + self.reportID
+ url = self.collectorAddress + '/report/' + self.reportId
if "json" in self.supportedFormats:
serialisation_format = 'json'
@@ -314,7 +317,7 @@ class OONIBReporter(OReporter):
'content': self.serializeEntry(entry, serialisation_format)
}
- log.debug("Updating report with id %s (%s)" % (self.reportID, url))
+ log.debug("Updating report with id %s (%s)" % (self.reportId, url))
request_json = json.dumps(request)
log.debug("Sending %s" % request_json)
@@ -414,7 +417,7 @@ class OONIBReporter(OReporter):
"different collector.")
raise errors.OONIBReportCreationError
- self.reportID = parsed_response['report_id']
+ self.reportId = parsed_response['report_id']
self.backendVersion = parsed_response['backend_version']
self.supportedFormats = parsed_response.get('supported_formats', ["yaml"])
@@ -423,7 +426,7 @@ class OONIBReporter(OReporter):
defer.returnValue(parsed_response['report_id'])
def finish(self):
- url = self.collectorAddress + '/report/' + self.reportID + '/close'
+ url = self.collectorAddress + '/report/' + self.reportId + '/close'
log.debug("Closing the report %s" % url)
return self.agent.request("POST", str(url))
@@ -538,6 +541,7 @@ class OONIBReportLog(object):
'collector': collector_address,
'report_id': report_id
}
+ return report_id
def created(self, report_file, collector_address, report_id):
return self.run(self._created, report_file,
@@ -576,7 +580,7 @@ class OONIBReportLog(object):
class Report(object):
- reportID = None
+ reportId = None
def __init__(self, test_details, report_filename,
reportEntryManager, collector_address=None,
@@ -605,19 +609,13 @@ class Report(object):
"""
self.test_details = test_details
self.collector_address = collector_address
+ self.report_filename = report_filename
self.report_log = OONIBReportLog()
self.yaml_reporter = None
- self.report_filename = None
- if not no_yamloo:
- self.yaml_reporter = YAMLReporter(test_details, report_filename=report_filename)
- self.report_filename = self.yaml_reporter.report_path
-
self.oonib_reporter = None
- if collector_address:
- self.oonib_reporter = OONIBReporter(test_details,
- collector_address)
+ self.no_yamloo = no_yamloo
self.done = defer.Deferred()
self.reportEntryManager = reportEntryManager
@@ -641,36 +639,26 @@ class Report(object):
d.addCallback(created)
return d
+ @defer.inlineCallbacks
def open(self):
"""
This will create all the reports that need to be created and fires the
created callback of the reporter whose report got created.
"""
- d = defer.Deferred()
- deferreds = []
-
- def yaml_report_failed(failure):
- d.errback(failure)
-
- def all_reports_openned(result):
- if not d.called:
- d.callback(None)
-
- if self.oonib_reporter:
- deferreds.append(self.open_oonib_reporter())
- else:
- if self.yaml_reporter:
- deferreds.append(self.report_log.not_created(self.report_filename))
-
- if self.yaml_reporter:
- yaml_report_created = \
- defer.maybeDeferred(self.yaml_reporter.createReport)
- yaml_report_created.addErrback(yaml_report_failed)
-
- dl = defer.DeferredList(deferreds)
- dl.addCallback(all_reports_openned)
+ if self.collector_address:
+ self.oonib_reporter = OONIBReporter(self.net_test_details,
+ self.collector_address)
+ self.net_test_details['report_id'] = yield self.open_oonib_reporter()
+
+ if not self.no_yamloo:
+ self.yaml_reporter = YAMLReporter(self.net_test_details,
+ report_filename=self.report_filename)
+ self.report_filename = self.yaml_reporter.report_path
+ if not self.oonib_reporter:
+ yield self.report_log.not_created(self.report_filename)
+ yield defer.maybeDeferred(self.yaml_reporter.createReport)
- return d
+ defer.returnValue(self.reportId)
def write(self, measurement):
"""
diff --git a/ooni/tasks.py b/ooni/tasks.py
index 2c4f07d..72e211f 100644
--- a/ooni/tasks.py
+++ b/ooni/tasks.py
@@ -115,9 +115,6 @@ class Measurement(TaskWithTimeout):
if 'input' not in self.testInstance.report.keys():
self.testInstance.report['input'] = test_input
- if 'test_start_time' not in self.testInstance.report.keys():
- start_time = otime.epochToNewTimestamp(self.testInstance._start_time)
- self.testInstance.report['test_start_time'] = start_time
self.testInstance.setUp()
diff --git a/ooni/tests/mocks.py b/ooni/tests/mocks.py
index 42f548e..4f51f32 100644
--- a/ooni/tests/mocks.py
+++ b/ooni/tests/mocks.py
@@ -203,3 +203,22 @@ class MockOONIBClient(object):
'collector': 'httpo://thirteenchars1234.onion'
}
return defer.succeed(ret)
+
+ def lookupTestCollector(self, net_tests):
+ ret = {
+ 'net-tests': [
+ ]
+ }
+ for net_test in net_tests:
+ test_helpers ={}
+ for test_helper in net_test['test-helpers']:
+ test_helpers[test_helper] = '127.0.0.1'
+
+ ret['net-tests'].append({
+ 'name': net_test['name'],
+ 'version': net_test['version'],
+ 'input-hashes': net_test['input-hashes'],
+ 'collector': 'httpo://thirteenchars1234.onion',
+ 'test-helpers': test_helpers
+ })
+ return defer.succeed(ret)
diff --git a/ooni/tests/test_deck.py b/ooni/tests/test_deck.py
index 7b423af..2c3bc04 100644
--- a/ooni/tests/test_deck.py
+++ b/ooni/tests/test_deck.py
@@ -148,7 +148,7 @@ class TestDeck(BaseTestCase):
deck.verify()
@defer.inlineCallbacks
- def test_lookuptest_helpers(self):
+ def test_lookup_test_helpers_and_collector(self):
deck = Deck(decks_directory=".")
deck.bouncer = "httpo://foo.onion"
deck.oonibclient = MockOONIBClient()
@@ -156,7 +156,7 @@ class TestDeck(BaseTestCase):
self.assertEqual(len(deck.netTestLoaders[0].missingTestHelpers), 1)
- yield deck.lookupTestHelpers()
+ yield deck.lookupCollectorAndTestHelpers()
self.assertEqual(deck.netTestLoaders[0].collector,
'httpo://thirteenchars1234.onion')
diff --git a/ooni/tests/test_reporter.py b/ooni/tests/test_reporter.py
index 9774449..b4e7592 100644
--- a/ooni/tests/test_reporter.py
+++ b/ooni/tests/test_reporter.py
@@ -92,7 +92,7 @@ class TestOONIBReporter(unittest.TestCase):
def test_create_report(self):
self.mock_response = oonib_new_report_message
yield self.oonib_reporter.createReport()
- assert self.oonib_reporter.reportID == oonib_new_report_message[
+ assert self.oonib_reporter.reportId == oonib_new_report_message[
'report_id']
@defer.inlineCallbacks