[tor-commits] [ooni-probe/master] Ignore global options that are None or set to zero.

[art at torproject.org]

commit 196fc17c2d62a8eb37e95e55f0dedaa2c53284f5
Author: Arturo Filastò <arturo at filasto.net>
Date:   Mon Aug 29 09:39:14 2016 +0200

    Ignore global options that are None or set to zero.
    
    * Always pop values from task_data to ensure the options are always clean.
    
    This closes #592
---
 ooni/deck/deck.py       | 16 ++++++++++------
 ooni/tests/test_deck.py | 10 +++++++++-
 ooni/ui/cli.py          |  4 ++--
 3 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/ooni/deck/deck.py b/ooni/deck/deck.py
index 037c5d6..e4c35c4 100644
--- a/ooni/deck/deck.py
+++ b/ooni/deck/deck.py
@@ -317,12 +317,16 @@ class DeckTask(object):
 
         self._load(data)
 
-    def _get_option(self, name, task_data, default=None):
+    def _pop_option(self, name, task_data, default=None):
         try:
-            return self.global_options[name]
+            value = self.global_options[name]
+            if value in [None, 0]:
+                raise KeyError
         except KeyError:
-            return task_data.pop(name,
-                                 self.parent_metadata.get(name, default))
+            value = task_data.pop(name,
+                                  self.parent_metadata.get(name, default))
+        task_data.pop(name, None)
+        return value
 
     def _load_ooni(self, task_data):
         required_keys = ["test_name"]
@@ -334,8 +338,8 @@ class DeckTask(object):
         nettest_path = nettest_to_path(task_data.pop("test_name"),
                                        self._arbitrary_paths)
 
-        annotations = self._get_option('annotations', task_data, {})
-        collector_address = self._get_option('collector', task_data, None)
+        annotations = self._pop_option('annotations', task_data, {})
+        collector_address = self._pop_option('collector', task_data, None)
 
         try:
             self.output_path = self.global_options['reportfile']
diff --git a/ooni/tests/test_deck.py b/ooni/tests/test_deck.py
index 1bc6097..b2d05a6 100644
--- a/ooni/tests/test_deck.py
+++ b/ooni/tests/test_deck.py
@@ -14,7 +14,7 @@ from hashlib import sha256
 from ooni import errors
 from ooni.deck.store import input_store
 from ooni.deck.backend import lookup_collector_and_test_helpers
-from ooni.deck.deck import nettest_to_path, NGDeck
+from ooni.deck.deck import nettest_to_path, NGDeck, options_to_args
 from ooni.deck.legacy import convert_legacy_deck
 from ooni.tests.bases import ConfigTestCase
 from ooni.tests.mocks import MockBouncerClient, MockCollectorClient
@@ -330,3 +330,11 @@ class TestNGDeck(ConfigTestCase):
             "manipulation/http_header_field_manipulation",
             "blocking/web_connectivity"
         ])
+        tasks = map(lambda task: task['ooni'], ng_deck['tasks'])
+        self.assertEqual(
+            tasks[2]['f'],
+            '/path/to/citizenlab-urls-global.txt')
+
+    def test_options_to_args(self):
+        args = options_to_args({"f": "foobar.txt", "bar": None, "help": 0})
+        print(args)
diff --git a/ooni/ui/cli.py b/ooni/ui/cli.py
index 61f254b..3ad9605 100644
--- a/ooni/ui/cli.py
+++ b/ooni/ui/cli.py
@@ -338,8 +338,8 @@ def createDeck(global_options, url=None):
         raise SystemExit(3)
 
     except errors.OONIUsageError as e:
-        log.err(e)
-        print e.net_test_loader.usageOptions().getUsage()
+        log.exception(e)
+        map(log.msg, e.net_test_loader.usageOptions().getUsage().split("\n"))
         raise SystemExit(4)
 
     except errors.HTTPSCollectorUnsupported: