Revert "[code-health] Run mini_installer tests using python3"

This reverts commit 30b9722.

Reason for revert: broke continuous builds.

Original change's description:
> [code-health] Run mini_installer tests using python3
>
> Bug: 941669
> Cq-Include-Trybots: luci.chrome.try:win-chrome,win64-chrome
> Change-Id: Ifcad5bb5a157c90c591fa77ed5260c7f91749ab0
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3561093
> Reviewed-by: Ben Pastene <bpastene@chromium.org>
> Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
> Reviewed-by: Greg Thompson <grt@chromium.org>
> Commit-Queue: Fabrice de Gans <fdegans@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#989048}

Bug: 941669
Change-Id: Id0b280092c1979aa0eb71d228eed97c10af9549f
Cq-Include-Trybots: luci.chrome.try:win-chrome,win64-chrome
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3574563
Auto-Submit: Greg Thompson <grt@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Fabian Sommer <fabiansommer@chromium.org>
Commit-Queue: Fabian Sommer <fabiansommer@chromium.org>
Owners-Override: Fabian Sommer <fabiansommer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#989353}

chrome/test/mini_installer/BUILD.gn

@@ -65,6 +65,7 @@ if (is_win) {
       "//third_party/catapult/third_party/typ",
     ]
     data = [
+             "//third_party/webdriver/pylib/",
              "//testing/scripts/",
              "//third_party/depot_tools/download_from_google_storage.py",
              "//third_party/depot_tools/gsutil.py",

chrome/test/mini_installer/PRESUBMIT.py

@@ -11,9 +11,7 @@
 
 
 def CommonChecks(input_api, output_api):
-    return input_api.canned_checks.RunPylint(input_api,
-                                             output_api,
-                                             version='2.7')
+    return input_api.canned_checks.RunPylint(input_api, output_api)
 
 
 def CheckChangeOnUpload(input_api, output_api):

chrome/test/mini_installer/chrome_helper.py

@@ -99,7 +99,7 @@ def GetBrowserProcess(chrome_processes):
         |chrome_processes|.
         """
         # Find the one whose parent isn't a chrome.exe process.
-        for process in chrome_processes.values():
+        for process in chrome_processes.itervalues():
             try:
                 if get_process_ppid(process) not in chrome_processes:
                     return process
@@ -113,7 +113,7 @@ def GetBrowserProcess(chrome_processes):
         process = GetBrowserProcess(chrome_processes)
         if not process:
             # Pick any process to wait on if no top-level parent was found.
-            process = next(chrome_processes.values())
+            process = next(chrome_processes.itervalues())
         if process.is_running():
             LOGGER.info('Waiting on %s for %s %s processes to exit' %
                         (str(process), len(chrome_processes),

chrome/test/mini_installer/config/config.config

@@ -215,15 +215,15 @@
   ],
   "actions": [
     ["test_chrome_with_chromedriver_user",
-     "\"$PYTHON_INTERPRETER\" test_chrome_with_chromedriver.py --chromedriver-path=\"$CHROMEDRIVER_PATH\" $QUIET $OUTPUT_DIR \"$LOCAL_APPDATA\\$CHROME_DIR\\Application\\chrome.exe\""],
+     "python test_chrome_with_chromedriver.py --chromedriver-path=\"$CHROMEDRIVER_PATH\" $QUIET $OUTPUT_DIR \"$LOCAL_APPDATA\\$CHROME_DIR\\Application\\chrome.exe\""],
     ["test_chrome_with_chromedriver_system",
-     "\"$PYTHON_INTERPRETER\" test_chrome_with_chromedriver.py --chromedriver-path=\"$CHROMEDRIVER_PATH\" $QUIET $OUTPUT_DIR \"$PROGRAM_FILES\\$CHROME_DIR\\Application\\chrome.exe\""],
+     "python test_chrome_with_chromedriver.py --chromedriver-path=\"$CHROMEDRIVER_PATH\" $QUIET $OUTPUT_DIR \"$PROGRAM_FILES\\$CHROME_DIR\\Application\\chrome.exe\""],
     ["test_chrome_with_chromedriver_beta",
-     "\"$PYTHON_INTERPRETER\" test_chrome_with_chromedriver.py --chromedriver-path=\"$CHROMEDRIVER_PATH\" $QUIET $OUTPUT_DIR \"$LOCAL_APPDATA\\$CHROME_DIR_BETA\\Application\\chrome.exe\""],
+     "python test_chrome_with_chromedriver.py --chromedriver-path=\"$CHROMEDRIVER_PATH\" $QUIET $OUTPUT_DIR \"$LOCAL_APPDATA\\$CHROME_DIR_BETA\\Application\\chrome.exe\""],
     ["test_chrome_with_chromedriver_canary",
-     "\"$PYTHON_INTERPRETER\" test_chrome_with_chromedriver.py --chromedriver-path=\"$CHROMEDRIVER_PATH\" $QUIET $OUTPUT_DIR \"$LOCAL_APPDATA\\$CHROME_DIR_SXS\\Application\\chrome.exe\""],
+     "python test_chrome_with_chromedriver.py --chromedriver-path=\"$CHROMEDRIVER_PATH\" $QUIET $OUTPUT_DIR \"$LOCAL_APPDATA\\$CHROME_DIR_SXS\\Application\\chrome.exe\""],
     ["test_chrome_with_chromedriver_dev",
-     "\"$PYTHON_INTERPRETER\" test_chrome_with_chromedriver.py --chromedriver-path=\"$CHROMEDRIVER_PATH\" $QUIET $OUTPUT_DIR \"$LOCAL_APPDATA\\$CHROME_DIR_DEV\\Application\\chrome.exe\""],
+     "python test_chrome_with_chromedriver.py --chromedriver-path=\"$CHROMEDRIVER_PATH\" $QUIET $OUTPUT_DIR \"$LOCAL_APPDATA\\$CHROME_DIR_DEV\\Application\\chrome.exe\""],
     ["install_chrome_beta",
      "\"$MINI_INSTALLER\" --chrome-beta \"$LOG_FILE\" --verbose-logging --do-not-launch-chrome"],
     ["install_chrome_canary",
@@ -245,27 +245,27 @@
     ["kill_user_chrome",
      "reg.exe delete \"HKEY_CURRENT_USER\\$CHROME_UPDATE_REGISTRY_SUBKEY\" /v pv /f /reg:32"],
     ["launch_chrome_canary",
-     "\"$PYTHON_INTERPRETER\" launch_chrome.py \"$LOCAL_APPDATA\\$CHROME_DIR_SXS\\Application\\chrome.exe\""],
+     "python launch_chrome.py \"$LOCAL_APPDATA\\$CHROME_DIR_SXS\\Application\\chrome.exe\""],
     ["launch_chrome_system",
-     "\"$PYTHON_INTERPRETER\" launch_chrome.py \"$PROGRAM_FILES\\$CHROME_DIR\\Application\\chrome.exe\""],
+     "python launch_chrome.py \"$PROGRAM_FILES\\$CHROME_DIR\\Application\\chrome.exe\""],
     ["launch_chrome_user",
-     "\"$PYTHON_INTERPRETER\" launch_chrome.py \"$LOCAL_APPDATA\\$CHROME_DIR\\Application\\chrome.exe\""],
+     "python launch_chrome.py \"$LOCAL_APPDATA\\$CHROME_DIR\\Application\\chrome.exe\""],
     ["quit_chrome_canary",
-     "\"$PYTHON_INTERPRETER\" quit_chrome.py \"$LOCAL_APPDATA\\$CHROME_DIR_SXS\\Application\\chrome.exe\""],
+     "python quit_chrome.py \"$LOCAL_APPDATA\\$CHROME_DIR_SXS\\Application\\chrome.exe\""],
     ["quit_chrome_system",
-     "\"$PYTHON_INTERPRETER\" quit_chrome.py \"$PROGRAM_FILES\\$CHROME_DIR\\Application\\chrome.exe\""],
+     "python quit_chrome.py \"$PROGRAM_FILES\\$CHROME_DIR\\Application\\chrome.exe\""],
     ["quit_chrome_user",
-     "\"$PYTHON_INTERPRETER\" quit_chrome.py \"$LOCAL_APPDATA\\$CHROME_DIR\\Application\\chrome.exe\""],
+     "python quit_chrome.py \"$LOCAL_APPDATA\\$CHROME_DIR\\Application\\chrome.exe\""],
     ["uninstall_chrome_beta",
-     "\"$PYTHON_INTERPRETER\" uninstall_chrome.py \"$LOG_FILE\" --chrome-long-name=\"$CHROME_LONG_NAME_BETA\""],
+     "python uninstall_chrome.py \"$LOG_FILE\" --chrome-long-name=\"$CHROME_LONG_NAME_BETA\""],
     ["uninstall_chrome_canary",
-     "\"$PYTHON_INTERPRETER\" uninstall_chrome.py \"$LOG_FILE\" --chrome-long-name=\"$CHROME_LONG_NAME_SXS\""],
+     "python uninstall_chrome.py \"$LOG_FILE\" --chrome-long-name=\"$CHROME_LONG_NAME_SXS\""],
     ["uninstall_chrome_dev",
-     "\"$PYTHON_INTERPRETER\" uninstall_chrome.py \"$LOG_FILE\" --chrome-long-name=\"$CHROME_LONG_NAME_DEV\""],
+     "python uninstall_chrome.py \"$LOG_FILE\" --chrome-long-name=\"$CHROME_LONG_NAME_DEV\""],
     ["uninstall_chrome_system",
-     "\"$PYTHON_INTERPRETER\" uninstall_chrome.py \"$LOG_FILE\" --chrome-long-name=\"$CHROME_LONG_NAME\" --system-level"],
+     "python uninstall_chrome.py \"$LOG_FILE\" --chrome-long-name=\"$CHROME_LONG_NAME\" --system-level"],
     ["uninstall_chrome_user",
-     "\"$PYTHON_INTERPRETER\" uninstall_chrome.py \"$LOG_FILE\" --chrome-long-name=\"$CHROME_LONG_NAME\""],
+     "python uninstall_chrome.py \"$LOG_FILE\" --chrome-long-name=\"$CHROME_LONG_NAME\""],
     ["update_chrome_canary",
      "\"$MINI_INSTALLER\" --chrome-sxs \"$LOG_FILE\" --verbose-logging --do-not-launch-chrome"],
     ["update_chrome_system",

chrome/test/mini_installer/create_zip.py

@@ -46,9 +46,9 @@
 THIS_DIR = os.path.abspath(os.path.dirname(os.path.abspath(__file__)))
 SRC_DIR = os.path.join(THIS_DIR, '..', '..', '..')
 SELENIUM_PATH = os.path.abspath(
-    os.path.join(SRC_DIR, 'third_party', 'webdriver', 'pylib'))
+    os.path.join(SRC_DIR, r'third_party', 'webdriver', 'pylib'))
 TYP_PATH = os.path.abspath(
-    os.path.join(SRC_DIR, 'third_party', 'catapult', 'third_party', 'typ'))
+    os.path.join(SRC_DIR, r'third_party', 'catapult', 'third_party', 'typ'))
 BLOCKLIST = ['', '.pyc', '.gn', '.gni', '.txt', '.bat']
 
 

chrome/test/mini_installer/installer_test.py

@@ -9,6 +9,7 @@
 driven by typ.
 """
 
+import argparse
 import contextlib
 import json
 import logging
@@ -18,9 +19,10 @@
 import sys
 import tempfile
 import traceback
+import typ
 import unittest
 import win32api
-from win32comext.shell import shell, shellcon
+from win32com.shell import shell, shellcon
 
 from argument_parser import ArgumentParser
 import property_walker
@@ -38,7 +40,7 @@
 _force_clean = not RUNNING_LOCALLY
 
 
-class Config:
+class Config(object):
     """Describes the machine states, actions, and test cases.
 
     Attributes:
@@ -71,7 +73,7 @@ def __init__(self, method_name):
             method_name: The name of this test.
         """
         assert InstallerTest._config, 'module _initialize() not yet called'
-        super().__init__(method_name)
+        super(InstallerTest, self).__init__(method_name)
         self._name = method_name[5:]
         self._test = InstallerTest._config.traversals[self._name]
         self._clean_on_teardown = True
@@ -164,7 +166,7 @@ def _VerifyState(self, state):
         except AssertionError as e:
             # If an AssertionError occurs, we intercept it and add the state
             # name to the error message so that we know where the test fails.
-            raise AssertionError("In state '%s', %s" % (state, str(e))) from e
+            raise AssertionError("In state '%s', %s" % (state, e))
 
 
 def RunCommand(command, variable_expander):
@@ -193,8 +195,6 @@ def RunCommand(command, variable_expander):
         proc = subprocess.Popen(expanded_command,
                                 shell=True,
                                 cwd=script_dir,
-                                text=True,
-                                encoding='utf-8',
                                 stdout=subprocess.PIPE,
                                 stderr=subprocess.PIPE)
         stdout, stderr = proc.communicate()
@@ -235,11 +235,10 @@ def RunCleanCommand(force_clean, clean_state, variable_expander):
     # Attempt to run each installed product's uninstaller.
     interactive_option = '--interactive' if not force_clean else ''
     for product_name, product_switch in data:
-        command = (
-            '%s uninstall_chrome.py '
-            '--chrome-long-name="%s" '
-            '--no-error-if-absent %s %s' %
-            (sys.executable, product_name, product_switch, interactive_option))
+        command = ('python uninstall_chrome.py '
+                   '--chrome-long-name="%s" '
+                   '--no-error-if-absent %s %s' %
+                   (product_name, product_switch, interactive_option))
         try:
             RunCommand(command, variable_expander)
         except:  # pylint: disable=bare-except
@@ -267,15 +266,16 @@ def MergePropertyDictionaries(current_property, new_property):
         current_property: The property dictionary to be modified.
         new_property: The new property dictionary.
     """
-    for key, value in new_property.items():
+    for key, value in new_property.iteritems():
         if key not in current_property:
             current_property[key] = value
         else:
             assert (isinstance(current_property[key], dict)
                     and isinstance(value, dict))
             # This merges two dictionaries together. In case there are keys with
             # the same name, the latter will override the former.
-            current_property[key].update(value)
+            current_property[key] = dict(current_property[key].items() +
+                                         value.items())
 
 
 def FilterConditionalElem(elem, condition_name, variable_expander):
@@ -340,10 +340,9 @@ def ParseConfigFile(filename, variable_expander):
     config.tests = config_data['tests']
     # Drop conditional tests that should not be run in the current
     # configuration.
-    config.tests = list(
-        filter(
-            lambda t: FilterConditionalElem(t, 'condition', variable_expander),
-            config.tests))
+    config.tests = filter(
+        lambda t: FilterConditionalElem(t, 'condition', variable_expander),
+        config.tests)
     for state_name, state_property_filenames in config_data['states']:
         config.states[state_name] = ParsePropertyFiles(
             directory, state_property_filenames, variable_expander)
@@ -454,15 +453,24 @@ def GetAbsoluteConfigPath(path):
     return os.path.abspath(path)
 
 
+# Until we're using Python 3.8 or newer, we must use tearDownModule to restore
+# the tmp dir. It would be cleaner to remove _temp_dir_manager and instead do
+# something along the lines of this after __enter__():
+# unittest.addModuleCleanup(_temp_dir_manager.__exit__, None, None, None)
+_temp_dir_manager = None
+
+
 def setUpModule():
     # Make sure that TMP and Chrome's installation directory are on the same
     # drive to work around https://crbug.com/700809. (CSIDL_PROGRAM_FILESX86 is
     # valid for both 32 and 64-bit apps running on 32 or 64-bit Windows.)
     drive = os.path.splitdrive(
         shell.SHGetFolderPath(0, shellcon.CSIDL_PROGRAM_FILESX86, None, 0))[0]
+    global _temp_dir_manager
     _temp_dir_manager = ConfigureTempOnDrive(drive)
-    _temp_dir_manager.__enter__()  # pylint: disable=no-member
-    unittest.addModuleCleanup(_temp_dir_manager.__exit__, None, None, None)  # pylint: disable=no-member
+    _temp_dir_manager.__enter__()
+    # Switch to this once we're using Python 3.8 or newer:
+    # unittest.addModuleCleanup(_temp_dir_manager.__exit__, None, None, None)
 
     # The last state in any test's traversal is the "clean" state, so use it to
     # drive the initial cleanup operation.
@@ -473,11 +481,18 @@ def setUpModule():
         RunCleanCommand(_force_clean, clean_state,
                         InstallerTest._variable_expander)
     except:
-        _temp_dir_manager.__exit__(None, None, None)  # pylint: disable=no-member
+        _temp_dir_manager.__exit__(None, None, None)
         _temp_dir_manager = None
         raise
 
 
+def tearDownModule():
+    global _temp_dir_manager
+    if _temp_dir_manager:
+        _temp_dir_manager.__exit__(None, None, None)
+        _temp_dir_manager = None
+
+
 def _initialize():
     """Initializes the InstallerTest class.
 

chrome/test/mini_installer/launch_chrome.py

@@ -1,4 +1,3 @@
-#!/usr/bin/env python3
 # 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.

chrome/test/mini_installer/property_walker.py

@@ -57,9 +57,9 @@ def _Walk(operations, continue_on_error, property_dict, variable_expander):
         property_dict: A property dictionary mapping type names to expectations.
         variable_expander: A VariableExpander.
     """
-    for type_name, expectations in property_dict.items():
+    for type_name, expectations in property_dict.iteritems():
         operation = operations[type_name]
-        for expectation_name, expectation_dict in expectations.items():
+        for expectation_name, expectation_dict in expectations.iteritems():
             # Skip over expectations with conditions that aren't satisfied.
             if 'condition' in expectation_dict:
                 condition = variable_expander.Expand(

chrome/test/mini_installer/quit_chrome.py

@@ -1,4 +1,3 @@
-#!/usr/bin/env python3
 # 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.