[autotest] Fix suite dependencies.

Suite dependencies were not adding the dependencies to the tests that
they were scheduling, it only impacted reimaging decisions.  This adds
the labels specified in suite_dependencies to the test's DEPENDENCIES
when scheduling them.

There's also a change snuck in here to change suite_dependencies to a
list of strings, as it's incredibly frustrating trying to
programmatically manipulate a comma seperated string.

BUG=chromium:279670
TEST=unit, Ran a suite with suite dependencies.  Dependencies showed up
on scheduled test.

Change-Id: I289036499e940487592ec21e6fdbf9eb24fc4f4e
Reviewed-on: https://chromium-review.googlesource.com/167144
Tested-by: Alexander Miller <milleral@chromium.org>
Reviewed-by: Dan Shi <dshi@chromium.org>
Reviewed-by: Aviv Keshet <akeshet@chromium.org>
Commit-Queue: Alexander Miller <milleral@chromium.org>
diff --git a/server/cros/dynamic_suite/dynamic_suite.py b/server/cros/dynamic_suite/dynamic_suite.py
index 6bb65ba..6da6799 100644
--- a/server/cros/dynamic_suite/dynamic_suite.py
+++ b/server/cros/dynamic_suite/dynamic_suite.py
@@ -325,7 +325,7 @@
                  file_experimental_bugs=False, max_runtime_mins=24*60,
                  firmware_reimage=False,
                  try_job_timeout_mins=DEFAULT_TRY_JOB_TIMEOUT_MINS,
-                 suite_dependencies=None,
+                 suite_dependencies=[],
                  reimage_type=constants.REIMAGE_TYPE_OS,
                  bug_template={}, devserver_url=None, **dargs):
         """
@@ -363,10 +363,12 @@
                                   reimage_type.)
         @param try_job_timeout_mins: Max time in mins we allow a try job to run
                                      before timing out.
-        @param suite_dependencies: A string with a comma separated list of suite
-                                   level dependencies, which act just like test
+        @param suite_dependencies: A list of strings of suite level
+                                   dependencies, which act just like test
                                    dependencies and are appended to each test's
                                    set of dependencies at job creation time.
+                                   A string of comma seperated labels is
+                                   accepted for backwards compatibility.
         @param reimage_type: A string identifying the type of reimaging that
                              should be done before running tests.
         @param bug_template: A template dictionary specifying the default bug
@@ -407,8 +409,12 @@
         self.max_runtime_mins = max_runtime_mins
         self.firmware_reimage = firmware_reimage
         self.try_job_timeout_mins = try_job_timeout_mins
-        self.suite_dependencies = suite_dependencies
         self.reimage_type = reimage_type
+        if isinstance(suite_dependencies, str):
+            self.suite_dependencies = [dep.strip(' ') for dep
+                                       in suite_dependencies.split(',')]
+        else:
+            self.suite_dependencies = suite_dependencies
         self.bug_template = bug_template
 
 
@@ -554,14 +560,9 @@
 
     dep_dict = all_dependencies.get(suite_spec.name, {'': []})
 
-    # Parse the suite_dependency string into a list of dependency labels,
-    # then append this list of suite dependencies to all individual job
-    # dependency lists.
     if suite_spec.suite_dependencies:
-        suite_deplist = [deplabel.strip(' ') for deplabel in
-                         suite_spec.suite_dependencies.split(',')]
         for deplist in dep_dict.values():
-            deplist.extend(suite_deplist)
+            deplist.extend(suite_spec.suite_dependencies)
 
     return dep_dict
 
@@ -612,7 +613,7 @@
         version_prefix=reimager.version_prefix,
         file_bugs=spec.file_bugs,
         file_experimental_bugs=spec.file_experimental_bugs,
-        suite_job_id=suite_job_id)
+        suite_job_id=suite_job_id, extra_deps=spec.suite_dependencies)
 
     # Now we get to asychronously schedule tests.
     suite.schedule(spec.job.record_entry, spec.add_experimental)
diff --git a/server/cros/dynamic_suite/dynamic_suite_unittest.py b/server/cros/dynamic_suite/dynamic_suite_unittest.py
index 880cc1d..a4ec73b 100755
--- a/server/cros/dynamic_suite/dynamic_suite_unittest.py
+++ b/server/cros/dynamic_suite/dynamic_suite_unittest.py
@@ -36,7 +36,7 @@
                        'skip_reimage': True,
                        'check_hosts': False,
                        'add_experimental': False,
-                       'suite_dependencies': 'test_dep'}
+                       'suite_dependencies': ['test_dep']}
 
 
 
@@ -133,7 +133,7 @@
         self.assertEquals(spec.skip_reimage, False)
         self.assertEquals(spec.add_experimental, True)
         self.assertEquals(spec.devserver, mock_ds)
-        self.assertEquals(spec.suite_dependencies, None)
+        self.assertEquals(spec.suite_dependencies, [])
 
 
     def testReimageWithBadDependencies(self):
diff --git a/server/cros/dynamic_suite/suite.py b/server/cros/dynamic_suite/suite.py
index b3cf6bf..b07d5c6 100644
--- a/server/cros/dynamic_suite/suite.py
+++ b/server/cros/dynamic_suite/suite.py
@@ -230,7 +230,7 @@
                  tko=None, pool=None, results_dir=None, max_runtime_mins=24*60,
                  version_prefix=constants.VERSION_PREFIX,
                  file_bugs=False, file_experimental_bugs=False,
-                 suite_job_id=None, ignore_deps=False):
+                 suite_job_id=None, ignore_deps=False, extra_deps=[]):
         """
         Constructor
 
@@ -257,6 +257,8 @@
         @param ignore_deps: True if jobs should ignore the DEPENDENCIES
                             attribute and skip applying of dependency labels.
                             (Default:False)
+        @param extra_deps: A list of strings which are the extra DEPENDENCIES
+                           to add to each test being scheduled.
         """
         def combined_predicate(test):
             #pylint: disable-msg=C0111
@@ -286,6 +288,7 @@
         self._file_experimental_bugs = file_experimental_bugs
         self._suite_job_id = suite_job_id
         self._ignore_deps = ignore_deps
+        self._extra_deps = extra_deps
 
 
     @property
@@ -327,6 +330,8 @@
         cros_label = self._version_prefix + self._build
         job_deps.append(cros_label)
 
+        if self._extra_deps:
+            job_deps.extend(self._extra_deps)
         if self._pool:
             job_deps.append(self._pool)
 
diff --git a/server/cros/dynamic_suite/suite_unittest.py b/server/cros/dynamic_suite/suite_unittest.py
index 5fd46a0..9ccdefa 100644
--- a/server/cros/dynamic_suite/suite_unittest.py
+++ b/server/cros/dynamic_suite/suite_unittest.py
@@ -208,7 +208,7 @@
 
     def expect_job_scheduling(self, recorder, add_experimental,
                               tests_to_skip=[], ignore_deps=False,
-                              raises=False):
+                              raises=False, suite_deps=[]):
         """Expect jobs to be scheduled for 'tests' in |self.files|.
 
         @param add_experimental: expect jobs for experimental tests as well.
@@ -228,6 +228,8 @@
             if not ignore_deps:
                 dependencies.extend(test.dependencies)
             dependencies.append(constants.VERSION_PREFIX + self._BUILD)
+            if suite_deps:
+                dependencies.extend(suite_deps)
             job_mock = self.afe.create_job(
                 control_file=test.text,
                 name=mox.And(mox.StrContains(self._BUILD),
@@ -311,6 +313,20 @@
         suite.schedule(recorder.record_entry, add_experimental=True)
 
 
+    def testSuiteDependencies(self):
+        """Should add suite dependencies to tests scheduled."""
+        self.mock_control_file_parsing()
+        recorder = self.mox.CreateMock(base_job.base_job)
+        self.expect_job_scheduling(recorder, add_experimental=False,
+                                   suite_deps=['extra'])
+
+        self.mox.ReplayAll()
+        suite = Suite.create_from_name(self._TAG, self._BUILD, self._BOARD,
+                                       self.devserver, extra_deps=['extra'],
+                                       afe=self.afe, tko=self.tko)
+        suite.schedule(recorder.record_entry, add_experimental=False)
+
+
     def _createSuiteWithMockedTestsAndControlFiles(self, file_bugs=False):
         """Create a Suite, using mocked tests and control file contents.