Bug 1351010 - Return a single value from optimize functions; r=bstack,TheNavigat

This simplifies the return value of optimize functions:
 * False -- don't optimize
 * True -- optimize away
 * <taskId> -- replace with this task

Original patch by ayodeji.oyewole, with test updates by dustin.

MozReview-Commit-ID: HKoWcINVSnV

--HG--
extra : rebase_source : 1d22d61030eb2bf95d4b9d0803a40e0be96c8830
extra : source : ae32a34822c40dbf7f9129edd701b945f6c95b3a

taskcluster/taskgraph/optimize.py

@@ -76,11 +76,11 @@ def optimize_task(task, params):
     for opt in task.optimizations:
         opt_type, args = opt[0], opt[1:]
         opt_fn = _optimizations[opt_type]
-        optimized, task_id = opt_fn(task, params, *args)
-        if optimized or task_id:
-            return optimized, task_id
+        opt_result = opt_fn(task, params, *args)
+        if opt_result:
+            return opt_result
 
-    return False, None
+    return False
 
 
 def annotate_task_graph(target_task_graph, params, do_not_optimize,
@@ -118,7 +118,11 @@ def annotate_task_graph(target_task_graph, params, do_not_optimize,
             replacement_task_id = existing_tasks[label]
         # otherwise, examine the task itself (which may be an expensive operation)
         else:
-            optimized, replacement_task_id = optimize_task(task, params)
+            opt_result = optimize_task(task, params)
+
+            # use opt_result to determine values for optimized, replacement_task_id
+            optimized = bool(opt_result)
+            replacement_task_id = opt_result if opt_result and opt_result is not True else None
 
         task.optimized = optimized
         task.task_id = replacement_task_id
@@ -195,11 +199,11 @@ def opt_index_search(task, params, index_path):
             index_path,
             use_proxy=bool(os.environ.get('TASK_ID')))
 
-        return True, task_id
+        return task_id or True
     except requests.exceptions.HTTPError:
         pass
 
-    return False, None
+    return False
 
 
 @optimization('seta')
@@ -212,7 +216,7 @@ def opt_seta(task, params):
         bbb_task = True
 
         # never optimize with SETA for BBB- bug 1364421
-        return False, None
+        return False
     else:
         label = task.label
 
@@ -224,20 +228,20 @@ def opt_seta(task, params):
                          params.get('pushdate'),
                          bbb_task):
         # Always optimize away low-value tasks
-        return True, None
+        return True
     else:
-        return False, None
+        return False
 
 
 @optimization('files-changed')
 def opt_files_changed(task, params, file_patterns):
     # pushlog_id == -1 - this is the case when run from a cron.yml job
     if params.get('pushlog_id') == -1:
-        return True, None
+        return True
 
     changed = files_changed.check(params, file_patterns)
     if not changed:
         logger.debug('no files found matching a pattern in `when.files-changed` for ' +
                      task.label)
-        return True, None
-    return False, None
+        return True
+    return False

taskcluster/taskgraph/test/test_optimize.py

@@ -60,10 +60,9 @@ class TestOptimize(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
         # set up some simple optimization functions
-        optimization('no-optimize')(lambda self, params: (False, None))
-        optimization('optimize-away')(lambda self, params: (True, None))
-        optimization('optimize-to-task')(lambda self, params, task: (True, task))
-        optimization('false-with-taskid')(lambda self, params: (False, 'some-taskid'))
+        optimization('no-optimize')(lambda self, params: False)
+        optimization('optimize-away')(lambda self, params: True)
+        optimization('optimize-to-task')(lambda self, params, task: task)
 
     def make_task(self, label, optimization=None, task_def=None, optimized=None, task_id=None):
         task_def = task_def or {'sample': 'task-def'}
@@ -85,7 +84,7 @@ class TestOptimize(unittest.TestCase):
         def repl(task_id):
             return 'SLUGID' if task_id and len(task_id) == 22 else task_id
         got_annotations = {
-            t.label: (t.optimized, repl(t.task_id)) for t in graph.tasks.itervalues()
+            t.label: repl(t.task_id) or t.optimized for t in graph.tasks.itervalues()
         }
         self.assertEqual(got_annotations, annotations)
 
@@ -101,9 +100,9 @@ class TestOptimize(unittest.TestCase):
         annotate_task_graph(graph, {}, set(), graph.graph.named_links_dict(), {}, None)
         self.assert_annotations(
             graph,
-            task1=(False, None),
-            task2=(False, None),
-            task3=(False, None)
+            task1=False,
+            task2=False,
+            task3=False
         )
 
     def test_annotate_task_graph_optimize_away_dependency(self):
@@ -130,8 +129,8 @@ class TestOptimize(unittest.TestCase):
                             graph.graph.named_links_dict(), label_to_taskid, None)
         self.assert_annotations(
             graph,
-            task1=(False, None),
-            task2=(False, None)
+            task1=False,
+            task2=False
         )
         self.assertEqual
 
@@ -148,9 +147,9 @@ class TestOptimize(unittest.TestCase):
                             graph.graph.named_links_dict(), {}, None)
         self.assert_annotations(
             graph,
-            task1=(False, None),
-            task2=(True, 'taskid'),
-            task3=(True, 'taskid')
+            task1=False,
+            task2='taskid',
+            task3='taskid'
         )
 
     def test_get_subgraph_single_dep(self):