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

chrome/test/mini_installer/test_installer.py

 # Copyright 2013 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """This script tests the installer with test cases specified in the config file.
 
 For each test case, it checks that the machine states after the execution of
 each command match the expected machine states. For more details, take a look at
 the design documentation at http://goo.gl/Q0rGM6
 """
@@ skipping 51 matching lines @@
     """
     return 'Test: %s' % (' -> '.join(self._test))
 
   def runTest(self):
     """Run the test case."""
     # |test| is an array of alternating state names and action names, starting
     # and ending with state names. Therefore, its length must be odd.
     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.
+    self._RunCleanCommand()

[gab, 2013/09/10 22:19:05]

This shouldn't always be run by default imo... as I remember grt saying, if you
have Chrome installed on your machine and you run this script unknowingly it
shouldn't delete your Chrome from your system.

It's ok for it to warn you though (popup or stderr).

It's also ok to have a flag that would force cleaning without prompting (and the
bots would always use that flag); but, again, it's not okay to risk deleting
Chrome from the machines of any random dev trying this script.

[grt (UTC plus 2), 2013/09/11 03:01:41]

I like the idea of:
1) having the test fail if it finds that the machine isn't in the proper state
2) having a command line argument that puts the machine in the proper state if
needed
3) having the bot configured to always use the above command line argument

grt would be sad if he ran this and it nuked his chromes. :-)

[sukolsak, 2013/09/13 09:10:21]

Done.

[sukolsak, 2013/09/13 09:10:21]

Done.

Checking whether Chrome is installed (and is installed properly) before deciding
whether to have the test fail is kind of tricky. So, I choose to always warn the
user that the script will remove Chrome, whether Chrome is currently installed
or not, and ask for confirmation.

 
     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])
 
       state = self._test[i + 1]
@@ skipping 28 matching lines @@
     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 _RunCleanCommand(self):
+    """Puts the machine in the clean state (i.e. Chrome not installed)."""
+    # TODO(sukolsak): Read the clean state from the config file and clean
+    # the machine according to it.
+    # TODO(sukolsak): Handle Chrome SxS installs.
+    command = ('python uninstall_chrome.py '
+               '--chrome-long-name="$CHROME_LONG_NAME" & '
+               'python uninstall_chrome.py '
+               '--chrome-long-name="$CHROME_LONG_NAME" --system-level')
+    resolved_command = self._path_resolver.ResolvePath(command)
+    script_dir = os.path.dirname(os.path.abspath(__file__))

[gab, 2013/09/10 22:19:05]

Since we now need |script_dir| in more than one place, how about making it a
member of InstallerTest?

[sukolsak, 2013/09/13 09:10:21]

|script_dir| doesn't seem like a state of an object to me. And we use it only
twice. What do you think?

+    subprocess.call(resolved_command, shell=True, cwd=script_dir,
+                    stderr=open(os.devnull, 'w'))

[gab, 2013/09/10 22:19:05]

So we trash all stderr output :(... why? If it's only to avoid the case where
there is no install present, perhaps uninstall_chrome.py could have an option to
silently return if no install is found instead?

[sukolsak, 2013/09/13 09:10:21]

Done.

+
 
 def MergePropertyDictionaries(current_property, new_property):
   """Merges the new property dictionary into the current property dictionary.
 
   This is different from general dictionary merging in that, in case there are
   keys with the same name, we merge values together in the first level, and we
   override earlier values in the second level. For more details, take a look at
   http://goo.gl/uE0RoR
 
   Args:
@@ skipping 90 matching lines @@
   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):
     return 1
   return 0
 
 
 if __name__ == '__main__':
   sys.exit(main())