Clean the machine before running commands in the mini_installer test framework.

chrome/test/mini_installer/test_installer.py

Index: chrome/test/mini_installer/test_installer.py
diff --git a/chrome/test/mini_installer/test_installer.py b/chrome/test/mini_installer/test_installer.py
index e0614243b0ee51d3c5219d77e050f43e494f4bb3..cd7d2c5e5860cdef015a6441ab05fc231f00d542 100644
--- a/chrome/test/mini_installer/test_installer.py
+++ b/chrome/test/mini_installer/test_installer.py
@@ -69,19 +69,23 @@ class InstallerTest(unittest.TestCase):
     self.assertEqual(1, len(self._test) % 2,
                      'The length of test array must be odd')
 
-    # TODO(sukolsak): run a reset command that puts the machine in clean state.
-
     state = self._test[0]
     self._VerifyState(state)
 
     # Starting at index 1, we loop through pairs of (action, state).
     for i in range(1, len(self._test), 2):
       action = self._test[i]
-      self._RunCommand(self._config.actions[action])
+      command = self._config.actions[action]
+      resolved_command = self._path_resolver.ResolvePath(command)
+      RunCommand(resolved_command)
 
       state = self._test[i + 1]
       self._VerifyState(state)
 
+  def tearDown(self):
+    """Cleans up the machine after running the test case."""
+    RunCleanCommand(True)

[gab, 2013/09/16 13:59:58]

We should only need to do this if the test case failed (otherwise we should
always be clean since the final state is the clean state).

[sukolsak, 2013/09/16 18:19:22]

Done.

+
   def shortDescription(self):
     """Overridden from unittest.TestCase.
 
@@ -105,18 +109,41 @@ class InstallerTest(unittest.TestCase):
       # to the error message so that we know where the test fails.
       raise AssertionError("In state '%s', %s" % (state, e))
 
-  def _RunCommand(self, command):
-    """Runs the given command from the current file's directory.
 
-    Args:
-      command: A command to run. It is expanded using ResolvePath.
-    """
-    resolved_command = self._path_resolver.ResolvePath(command)
-    script_dir = os.path.dirname(os.path.abspath(__file__))
-    exit_status = subprocess.call(resolved_command, shell=True, cwd=script_dir)
-    if exit_status != 0:
-      self.fail('Command %s returned non-zero exit status %s' % (
-          resolved_command, exit_status))
+def RunCommand(command):
+  """Runs the given command from the current file's directory.
+
+  This function throws an Exception if the command returns with non-zero exit
+  status.
+
+  Args:
+    command: A command to run.
+  """
+  script_dir = os.path.dirname(os.path.abspath(__file__))
+  exit_status = subprocess.call(command, shell=True, cwd=script_dir)
+  if exit_status != 0:
+    raise Exception('Command %s returned non-zero exit status %s' % (
+        command, exit_status))
+
+
+def RunCleanCommand(force_clean):
+  """Puts the machine in the clean state (i.e. Chrome not installed).
+
+  Args:
+    force_clean: A boolean indicating whether to force cleaning existing
+        installations.
+  """
+  # TODO(sukolsak): Read the clean state from the config file and clean
+  # the machine according to it.
+  # TODO(sukolsak): Handle Chrome SxS installs.
+  commands = []
+  interactive_option = '--interactive' if not force_clean else ''
+  for chrome_long_name in ['Google Chrome', 'Chromium']:

[gab, 2013/09/16 13:59:58]

I don't like having to delete both brands; imo we should only clean the brand we
are testing. It is true however that this is required to have a fully clean
state for a single shared registry value (App Paths for chrome.exe)...

But in practice I think it's the TODO step that should clean the shared value
(i.e., it's rule being: if the uninstall was successful, I shall clean
everything remaining I know of -- this includes the shared registry value).

Cleaning the shared registry value silently is not wrong imo; only one
chrome.exe can own it, and thus any chrome.exe can't rely on it (all it does is
it allows you to enter commands like "start chrome.exe" without providing a full
path -- it's kind of like adding chrome.exe to %PATH%). Removing it even if the
tested chrome doesn't own it isn't a big deal imo.


** tl;dr; Only clean the brand you are testing here.**
(and get the long name from the PathResolver)

[sukolsak, 2013/09/16 18:19:22]

Done.

+    for level_option in ['', '--system-level']:
+      commands.append('python uninstall_chrome.py --chrome-long-name="%s" '
+                      '--no-error-if-absent %s %s' %
+                      (chrome_long_name, level_option, interactive_option))
+  RunCommand(' && '.join(commands))
 
 
 def MergePropertyDictionaries(current_property, new_property):
@@ -185,16 +212,19 @@ def ParseConfigFile(filename):
   return config
 
 
-def RunTests(mini_installer_path, config):
+def RunTests(mini_installer_path, config, force_clean):
   """Tests the installer using the given Config object.
 
   Args:
     mini_installer_path: The path to mini_installer.exe.
     config: A Config object.
+    force_clean: A boolean indicating whether to force cleaning existing
+        installations.
 
   Returns:
     True if all the tests passed, or False otherwise.
   """
+  RunCleanCommand(force_clean)
   suite = unittest.TestSuite()
   path_resolver = PathResolver(mini_installer_path)
   for test in config.tests:
@@ -211,6 +241,8 @@ def main():
                          'Release or Debug directory)')
   parser.add_option('--target', default='Release',
                     help='Build target (Release or Debug)')
+  parser.add_option('--force-clean', action='store_true', dest='force_clean',
+                    default=False, help='Force cleaning existing installations')
   options, args = parser.parse_args()
   if len(args) != 1:
     parser.error('Incorrect number of arguments.')
@@ -221,7 +253,7 @@ def main():
   assert os.path.exists(mini_installer_path), ('Could not find file %s' %
                                                mini_installer_path)
   config = ParseConfigFile(config_filename)
-  if not RunTests(mini_installer_path, config):
+  if not RunTests(mini_installer_path, config, options.force_clean):
     return 1
   return 0