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..5071ecb0f7d00fe8d7fb64f9faef08066b7a337c 100644
--- a/chrome/test/mini_installer/test_installer.py
+++ b/chrome/test/mini_installer/test_installer.py
@@ -52,6 +52,7 @@ class InstallerTest(unittest.TestCase):
     self._test = test
     self._config = config
     self._path_resolver = path_resolver
+    self._was_successful = False
 
   def __str__(self):
     """Returns a string representing the test case.
@@ -69,19 +70,24 @@ 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])
+      RunCommand(self._config.actions[action], self._path_resolver)
 
       state = self._test[i + 1]
       self._VerifyState(state)
 
+    self._was_successful = True

[gab, 2013/09/16 18:33:45]

Add a comment stating why the test is successful if it made it here (i.e.,
RunCommand throws an exception on failure?).

[sukolsak, 2013/09/16 22:02:02]

Done.

+
+  def tearDown(self):
+    """Cleans up the machine if the test case fails or raises an exception."""

[gab, 2013/09/16 18:33:45]

Isn't "fail" and "raised an exception" the same thing? I'd cut this comment
after "fails."

[sukolsak, 2013/09/16 22:02:02]

Done. Python unittest makes a distinction between failures and errors. It counts
a test as failure if the test raises an AssertionError and as error if it raises
any other exceptions. But this also makes sense if we regard anything that is
not successful as failure.

+    if not self._was_successful:

[gab, 2013/09/16 18:41:22]

Rename |_was_successful| to something like |clean_on_teardown| since that's its
only purpose (and I think it's better that it not be interpreted too generally).

[sukolsak, 2013/09/16 22:02:02]

Done.

+      RunCleanCommand(True, self._path_resolver)
+
   def shortDescription(self):
     """Overridden from unittest.TestCase.
 
@@ -105,18 +111,44 @@ 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, path_resolver):
+  """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. It is expanded using ResolvePath.
+    path_resolver: A PathResolver object.
+  """
+  resolved_command = 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:
+    raise Exception('Command %s returned non-zero exit status %s' % (

[gab, 2013/09/16 18:41:22]

Why go from self.fail to throwing an exception?

Maybe we already talked about this, but I don't see a hint in the CL description
to why this was changed.

[sukolsak, 2013/09/16 22:02:02]

Because RunCommand is now a global function. I changed it so that
RunCleanCommand could be called outside the InstallerTest class. What self.fail
does is actually just throwing an Exception.

[gab, 2013/09/16 22:15:50]

Ah I see, man python is so subtle for that, unindent by 2 spaces and all of a
sudden you have a global function...! Sorry I hadn't noticed that.

+        command, exit_status))
+
+
+def RunCleanCommand(force_clean, path_resolver):
+  """Puts the machine in the clean state (i.e. Chrome not installed).
+
+  Args:
+    force_clean: A boolean indicating whether to force cleaning existing
+        installations.
+    path_resolver: A PathResolver object.
+  """
+  # 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 level_option in ['', '--system-level']:
+    commands.append('python uninstall_chrome.py '
+                    '--chrome-long-name="$CHROME_LONG_NAME" '
+                    '--no-error-if-absent %s %s' %
+                    (level_option, interactive_option))
+  RunCommand(' && '.join(commands), path_resolver)
 
 
 def MergePropertyDictionaries(current_property, new_property):
@@ -185,18 +217,21 @@ 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.
   """
   suite = unittest.TestSuite()
   path_resolver = PathResolver(mini_installer_path)
+  RunCleanCommand(force_clean, path_resolver)
   for test in config.tests:
     suite.addTest(InstallerTest(test, config, path_resolver))
   result = unittest.TextTestRunner(verbosity=2).run(suite)
@@ -211,6 +246,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 +258,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