cheets_[CG]TS: More cleanup and unification of the common codes.

Mainly as a preparation for separating "manual test" from "waivers" in GTS.
We already have the logic for CTS, so by unifying the code we can just utilize
it.
Also it should be good for code clarity and maintainability.

* Bundle install and waiver file reading code is moved to tradefed_test.
  The initialization code is planed in initialize() instead of setup().
* Tradefed result parsing logic also is moved to tradefed_test.
* Some unused attributes/arguments are removed.

BUG=None
TEST=cheets_CTS_N.7.1_r10.arm.CtsViewTestCases
TEST=cheets_GTS.5.0_r2.GtsPlacementTestCases
TEST=cheets_GTS.5.0_r2.GtsBackupHostTestCases
TEST=trybot

Change-Id: If800980f8523c9c312b1d5ced33f8eecaddf698e
Reviewed-on: https://chromium-review.googlesource.com/770837
Reviewed-by: Ilja H. Friedel <ihf@chromium.org>
Tested-by: Ilja H. Friedel <ihf@chromium.org>
diff --git a/server/cros/tradefed_test.py b/server/cros/tradefed_test.py
index e1cfd83..0258be5 100644
--- a/server/cros/tradefed_test.py
+++ b/server/cros/tradefed_test.py
@@ -372,18 +372,18 @@
     _BOARD_RETRY = {}
     _CHANNEL_RETRY = {'dev': 5}
 
-    def log_java_version(self):
+    def _log_java_version(self):
         """Quick sanity and spew of java version installed on the server."""
         utils.run('java', args=('-version',), ignore_status=False, verbose=True,
                   stdout_tee=utils.TEE_TO_LOGS, stderr_tee=utils.TEE_TO_LOGS)
 
-    def initialize(self, host=None, max_retry=None, warn_on_test_retry=True):
+    def initialize(self, bundle=None, uri=None, host=None, max_retry=None,
+                   warn_on_test_retry=True):
         """Sets up the tools and binary bundles for the test."""
         logging.info('Hostname: %s', host.hostname)
         self._host = host
         self._max_retry = self._get_max_retry(max_retry, self._host)
         self._install_paths = []
-        self.result_history = {}
         self._warn_on_test_retry = warn_on_test_retry
         # Tests in the lab run within individual lxc container instances.
         if utils.is_in_container():
@@ -419,6 +419,17 @@
         self._install_files(_ADB_DIR, _ADB_FILES, permission)
         self._install_files(_SDK_TOOLS_DIR, _SDK_TOOLS_FILES, permission)
 
+        # Install the tradefed bundle.
+        bundle_install_path = self._install_bundle(
+            uri or self._get_default_bundle_url(bundle))
+        self._repository = os.path.join(bundle_install_path,
+                                        self._get_tradefed_base_dir())
+        # Load waivers and manual tests so TF doesn't re-run them.
+        self._waivers = self._get_expected_failures('expectations')
+        self._manual_tests = self._get_expected_failures('manual_tests')
+        # Load modules with no tests.
+        self._notest_modules = self._get_expected_failures('notest_modules')
+
     def cleanup(self):
         """Cleans up any dirtied state."""
         # Kill any lingering adb servers.
@@ -1073,6 +1084,37 @@
             output = host.run(formatted_command, ignore_status=True)
             logging.info('END: %s\n', output)
 
+    def _run_and_parse_tradefed(self, commands):
+        """Kick off the tradefed command.
+
+        Assumes that only last entry of |commands| actually runs tests and has
+        interesting output (results, logs) for collection. Ignores all other
+        commands for this purpose.
+
+        @param commands: List of lists of command tokens.
+        @raise TestFail: when a test failure is detected.
+        @return: tuple of (tests, pass, fail, notexecuted) counts.
+        """
+        try:
+            output = self._run_tradefed(commands)
+        except:
+            self._log_java_version()
+            raise
+
+        result_destination = os.path.join(self.resultsdir,
+                                          self._get_tradefed_base_dir())
+        # Gather the global log first. Datetime parsing below can abort the test
+        # if tradefed startup had failed. Even then the global log is useful.
+        self._collect_tradefed_global_log(output, result_destination)
+        # Parse stdout to obtain datetime of the session. This is needed to
+        # locate result xml files and logs.
+        datetime_id = self._parse_tradefed_datetime_v2(output, self.summary)
+        # Collect tradefed logs for autotest.
+        self._collect_logs(datetime_id, result_destination)
+        # Result parsing must come after all other essential operations as test
+        # warnings, errors and failures can be raised here.
+        return self._parse_result_v2(output, waivers=self._waivers)
+
     def _clean_repository(self):
         """Ensures all old logs, results and plans are deleted.
 
@@ -1138,7 +1180,7 @@
         # new session_id might be more reliable. For now just assume simple
         # increment. This works if only one tradefed instance is active and
         # only a single run command is executing at any moment.
-        return session_id + 1, self._run_tradefed(commands)
+        return session_id + 1, self._run_and_parse_tradefed(commands)
 
     def _classify_results(self, total_tests, passed, failed, notexecuted,
                           waived, steps, retry_inconsistency_error):
@@ -1219,16 +1261,15 @@
                 # The list command is not required. It allows the reader to
                 # inspect the tradefed state when examining the autotest logs.
                 commands = [['list', 'results'], test_command]
-                counts = self._run_tradefed(commands)
+                counts = self._run_and_parse_tradefed(commands)
                 tests, passed, failed, notexecuted, waived = counts
-                self.result_history[steps] = counts
                 msg = 'run(t=%d, p=%d, f=%d, ne=%d, w=%d)' % counts
                 logging.info('RESULT: %s', msg)
                 self.summary += msg
-                if tests == 0 and target_module in self.notest_modules:
+                if tests == 0 and target_module in self._notest_modules:
                     logging.info('Package has no tests as expected.')
                     return
-                if tests > 0 and target_module in self.notest_modules:
+                if tests > 0 and target_module in self._notest_modules:
                     # We expected no tests, but the new bundle drop must have
                     # added some for us. Alert us to the situation.
                     raise error.TestFail('Failed: Remove module %s from '
@@ -1239,13 +1280,12 @@
                     notexecuted += 1
                     waived += 1
                     logging.warning('Skipped test %s', ' '.join(test_command))
-                elif tests == 0 and target_module not in self.notest_modules:
+                elif tests == 0 and target_module not in self._notest_modules:
                     logging.error('Did not find any tests in module. Hoping '
                                   'this is transient. Retry after reboot.')
                 if not self._consistent(tests, passed, failed, notexecuted):
                     # Try to figure out what happened. Example: b/35605415.
-                    self._run_tradefed([['list', 'results']],
-                                       collect_results=False)
+                    self._run_tradefed([['list', 'results']])
                     logging.warning('Test count inconsistent. %s',
                                     self.summary)
                 # Keep track of global count, we can't trust continue/retry.
@@ -1276,7 +1316,6 @@
                 session_id, counts = self._tradefed_retry(test_name,
                                                           session_id)
                 tests, passed, failed, notexecuted, waived = counts
-                self.result_history[steps] = counts
                 # Consistency check, did we really run as many as we thought
                 # initially?
                 if expected_tests != tests:
@@ -1299,7 +1338,6 @@
                     session_id, counts = self._tradefed_retry(test_name,
                                                               session_id)
                     tests, passed, failed, notexecuted, waived = counts
-                    self.result_history[steps] = counts
                 msg = 'retry(t=%d, p=%d, f=%d, ne=%d, w=%d)' % counts
                 logging.info('RESULT: %s', msg)
                 self.summary += ' ' + msg
diff --git a/server/site_tests/cheets_CTS_N/cheets_CTS_N.py b/server/site_tests/cheets_CTS_N/cheets_CTS_N.py
index c95c3ec..774a4f8 100644
--- a/server/site_tests/cheets_CTS_N/cheets_CTS_N.py
+++ b/server/site_tests/cheets_CTS_N/cheets_CTS_N.py
@@ -37,27 +37,11 @@
     _BOARD_RETRY = {'betty': 0}
     _CHANNEL_RETRY = {'dev': 5, 'beta': 5, 'stable': 5}
 
-    def setup(self, bundle=None, uri=None):
-        """Download and install a zipfile bundle from Google Storage.
+    def _get_default_bundle_url(self, bundle):
+        return _CTS_URI[bundle]
 
-        @param bundle: bundle name, which needs to be key of the _CTS_URI
-                       dictionary. Can be 'arm', 'x86' and undefined.
-        @param uri: URI of CTS bundle. Required if |abi| is undefined.
-        """
-        if uri:
-            self._android_cts = self._install_bundle(uri)
-        elif bundle in _CTS_URI:
-            self._android_cts = self._install_bundle(_CTS_URI[bundle])
-
-        self._repository = os.path.join(self._android_cts, 'android-cts')
-        self._cts_tradefed = os.path.join(self._repository, 'tools',
-                                          'cts-tradefed')
-        logging.info('CTS-tradefed path: %s', self._cts_tradefed)
-        # Load waivers and manual tests so TF doesn't re-run them.
-        self._waivers = self._get_expected_failures('expectations')
-        self._manual_tests = self._get_expected_failures('manual_tests')
-        # Load modules with no tests.
-        self.notest_modules = self._get_expected_failures('notest_modules')
+    def _get_tradefed_base_dir(self):
+        return 'android-cts'
 
     def _tradefed_run_command(self,
                               module=None,
@@ -138,58 +122,28 @@
                 sum(1 for abi in abilist if abi.startswith(prefix)))
         return self._timeoutfactor
 
-    def _run_tradefed(self, commands, datetime_id=None, collect_results=True):
-        """Kick off CTS."""
-        return self._run_cts_tradefed(commands, datetime_id, collect_results)
+    def _run_tradefed(self, commands):
+        """Kick off CTS.
 
-    def _run_cts_tradefed(self,
-                          commands,
-                          datetime_id=None,
-                          collect_results=True):
-        """Runs tradefed, collects logs and returns the result counts.
-
-        Assumes that only last entry of |commands| actually runs tests and has
-        interesting output (results, logs) for collection. Ignores all other
-        commands for this purpose.
-
-        @param commands: List of lists of command tokens.
+        @param commands: the command(s) to pass to CTS.
         @param datetime_id: For 'continue' datetime of previous run is known.
-                            Knowing it makes collecting logs more robust.
-        @param collect_results: Skip result collection if False.
-        @return: tuple of (tests, pass, fail, notexecuted) counts.
+        @return: The result object from utils.run.
         """
+        cts_tradefed = os.path.join(self._repository, 'tools', 'cts-tradefed')
         for command in commands:
-            # Assume only last command actually runs tests and has interesting
-            # output (results, logs) for collection.
             logging.info('RUN: ./cts-tradefed %s', ' '.join(command))
-            try:
-                output = self._run(
-                    self._cts_tradefed,
-                    args=tuple(command),
-                    timeout=self._timeout * self._get_timeout_factor(),
-                    verbose=True,
-                    ignore_status=False,
-                    # Make sure to tee tradefed stdout/stderr to autotest logs
-                    # continuously during the test run.
-                    stdout_tee=utils.TEE_TO_LOGS,
-                    stderr_tee=utils.TEE_TO_LOGS)
-            except Exception:
-                self.log_java_version()
-                raise
+            output = self._run(
+                cts_tradefed,
+                args=tuple(command),
+                timeout=self._timeout * self._get_timeout_factor(),
+                verbose=True,
+                ignore_status=False,
+                # Make sure to tee tradefed stdout/stderr to autotest logs
+                # continuously during the test run.
+                stdout_tee=utils.TEE_TO_LOGS,
+                stderr_tee=utils.TEE_TO_LOGS)
             logging.info('END: ./cts-tradefed %s\n', ' '.join(command))
-        if not collect_results:
-            return None
-        result_destination = os.path.join(self.resultsdir, 'android-cts')
-        # Gather the global log first. Datetime parsing below can abort the test
-        # if tradefed startup had failed. Even then the global log is useful.
-        self._collect_tradefed_global_log(output, result_destination)
-        if not datetime_id:
-            # Parse stdout to obtain datetime of the session. This is needed to
-            # locate result xml files and logs.
-            datetime_id = self._parse_tradefed_datetime_v2(output, self.summary)
-        # Collect tradefed logs for autotest.
-        self._collect_logs(datetime_id, result_destination)
-        return self._parse_result_v2(output, waivers=self._waivers)
+        return output
 
     def _should_skip_test(self):
         """Some tests are expected to fail and are skipped."""
@@ -284,6 +238,8 @@
         # Retries depend on channel.
         self._timeoutfactor = None
 
+        # TODO(kinaba): Move this logic to tradefed_test so that cheets_GTS can
+        # deal with manual tests.
         if not retry_manual_tests:
             self._waivers.update(self._manual_tests)
 
diff --git a/server/site_tests/cheets_GTS/cheets_GTS.py b/server/site_tests/cheets_GTS/cheets_GTS.py
index 888d3d8..f644b91 100644
--- a/server/site_tests/cheets_GTS/cheets_GTS.py
+++ b/server/site_tests/cheets_GTS/cheets_GTS.py
@@ -25,20 +25,12 @@
     """Sets up tradefed to run GTS tests."""
     version = 1
 
+    def _get_default_bundle_url(self, bundle):
+        return _PARTNER_GTS_LOCATION
 
-    def setup(self, uri=None):
-        """Set up GTS bundle from Google Storage.
 
-        @param uri: The location to pull the GTS bundle from.
-        """
-        if uri:
-            self._android_gts = self._install_bundle(uri)
-        else:
-            self._android_gts = self._install_bundle(_PARTNER_GTS_LOCATION)
-
-        self._repository = os.path.join(self._android_gts, 'android-gts')
-        self.waivers = self._get_expected_failures('expectations')
-        self.notest_modules = self._get_expected_failures('notest_modules')
+    def _get_tradefed_base_dir(self):
+        return 'android-gts'
 
 
     def _tradefed_run_command(self, target_module=None, plan=None,
@@ -57,53 +49,26 @@
         return args
 
 
-    def _run_tradefed(self, commands, datetime_id=None, collect_results=True):
-        """Kick off GTS."""
-        return self._run_gts_tradefed(commands, datetime_id, collect_results)
-
-
-    def _run_gts_tradefed(self, commands, datetime_id=None,
-                          collect_results=True):
-        """This tests runs the GTS tradefed binary and collects results.
+    def _run_tradefed(self, commands):
+        """Kick off GTS.
 
         @param commands: the command(s) to pass to GTS.
-        @param datetime_id: For 'continue' datetime of previous run is known.
-        @collect_results: skip result collection if false.
-        @raise TestFail: when a test failure is detected.
+        @return: The result object from utils.run.
         """
         gts_tradefed = os.path.join(self._repository, 'tools', 'gts-tradefed')
-        logging.info('GTS-tradefed path: %s', gts_tradefed)
-        # Run GTS via tradefed and obtain stdout, sterr as output.
         with tradefed_test.adb_keepalive(self._get_adb_target(),
                                          self._install_paths):
-            try:
-                for command in commands:
-                    output = self._run(gts_tradefed,
-                                       args=command,
-                                       verbose=True,
-                                       # Tee tradefed stdout/stderr to logs
-                                       # continuously during the test run.
-                                       stdout_tee=utils.TEE_TO_LOGS,
-                                       stderr_tee=utils.TEE_TO_LOGS)
-            except Exception:
-                self.log_java_version()
-                raise
-            if not collect_results:
-                return None
-        result_destination = os.path.join(self.resultsdir, 'android-gts')
-
-        # Gather the global log first. Datetime parsing below can abort the test
-        # if tradefed startup had failed. Even then the global log is useful.
-        self._collect_tradefed_global_log(output, result_destination)
-
-        # Parse stdout to obtain datetime IDs of directories into which tradefed
-        # wrote result xml files and logs.
-        datetime_id = self._parse_tradefed_datetime_v2(output)
-        self._collect_logs(datetime_id, result_destination)
-
-        # Result parsing must come after all other essential operations as test
-        # warnings, errors and failures can be raised here.
-        return self._parse_result_v2(output, waivers=self.waivers)
+            for command in commands:
+                logging.info('RUN: ./gts-tradefed %s', ' '.join(command))
+                output = self._run(gts_tradefed,
+                                   args=command,
+                                   verbose=True,
+                                   # Tee tradefed stdout/stderr to logs
+                                   # continuously during the test run.
+                                   stdout_tee=utils.TEE_TO_LOGS,
+                                   stderr_tee=utils.TEE_TO_LOGS)
+                logging.info('END: ./gts-tradefed %s\n', ' '.join(command))
+        return output
 
 
     def run_once(self, target_package=None, gts_tradefed_args=None):