[tor-commits] [ooni-probe/master] Clean up how we handle the reportId and remove unused functions

art at torproject.org art at torproject.org
Mon May 30 16:28:32 UTC 2016


commit da51f6d069ac55ebf430ebb0adfa6a70110fda68
Author: Arturo Filastò <arturo at 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





More information about the tor-commits mailing list