Bug 1352599 - Follow-up: Fix Python lint errors and tests. r=rillian draft
authorNick Alexander <nalexander@mozilla.com>
Thu, 12 Oct 2017 14:28:31 -0700
changeset 680316 173c6845f712f4f51ba6a489557482655d7cfe90
parent 680304 9af705e7ab64106b49b84ae091aeaca13200d357
child 735818 4b7e17452d9be6e0fbe0ddf7e9c320a0b01aeb67
push id84457
push usernalexander@mozilla.com
push dateFri, 13 Oct 2017 20:29:17 +0000
reviewersrillian
bugs1352599
milestone58.0a1
Bug 1352599 - Follow-up: Fix Python lint errors and tests. r=rillian This required to extend the fragile test sandbox yet further, but it's all following the established pattern. MozReview-Commit-ID: DzNP45JeAsM
build/moz.configure/java.configure
python/mozbuild/mozbuild/test/configure/common.py
python/mozbuild/mozbuild/test/configure/test_checks_configure.py
--- a/build/moz.configure/java.configure
+++ b/build/moz.configure/java.configure
@@ -62,55 +62,62 @@ def javac_version(javac):
         version = Version(output.split(' ')[-1])
         if version < '1.8':
             die('javac 1.8 or higher is required (found %s). '
                 'Check the JAVA_HOME environment variable.' % version)
         return version
     except subprocess.CalledProcessError as e:
         die('Failed to get javac version: %s', e.output)
 
+
 # Proguard detection
 # ========================================================
-@dependable
+@depends('--help')
 @imports('os')
-def proguard_jar_default():
+def proguard_jar_default(_):
     # By default, look for proguard.jar in the location to which `mach
     # bootstrap` or `mach artifact toolchain` will install Proguard.
-    mozbuild_state_dir = os.environ.get('MOZBUILD_STATE_PATH',
-                                        os.path.expanduser(os.path.join('~', '.mozbuild')))
+    default = os.path.expanduser(os.path.join('~', '.mozbuild'))
+    mozbuild_state_dir = os.environ.get('MOZBUILD_STATE_PATH', default)
     return os.path.join(mozbuild_state_dir, 'proguard', 'lib', 'proguard.jar')
 
+
 # Proguard is really required; we provide a good error message when
 # validating.
-option(env='PROGUARD_JAR', nargs=1, default=proguard_jar_default, help='Path to proguard.jar')
+option(env='PROGUARD_JAR', nargs=1, default=proguard_jar_default,
+       help='Path to proguard.jar')
+
 
 @depends(java, 'PROGUARD_JAR')
 @checking('for proguard.jar version')
-@imports('os')
+# Do not change, this is fragile!  This form works with the test
+# configure sandbox.
+@imports(_from='os', _import='path')
 @imports('subprocess')
-# Not Python 3 compatible, but neither is the subprocess invocation.
-@imports(_from='exceptions', _import='Exception')
 def valid_proguard(java, proguard_jar):
-    if not proguard_jar or not os.path.isfile(proguard_jar[0]):
-        die('proguard.jar 5.3.3 or higher is required (looked for %s). '
+    if not proguard_jar or not path.isfile(proguard_jar[0]):
+        die('proguard.jar 5.3.3 or higher is required (looked for {}). '
             'Run |mach artifact install --from-build proguard-jar| or add '
-            '`export PROGUARD_JAR=/path/to/proguard.jar` to your mozconfig.' % proguard_jar[0])
+            '`export PROGUARD_JAR=/path/to/proguard.jar` to your mozconfig.'
+            .format(proguard_jar[0]))
 
     try:
         output = subprocess.check_output([java, '-jar', proguard_jar[0]])
         # Exit code zero shouldn't happen.
-        die('Expected `java -jar {}` to fail (with version in output) but got exit code 0'
+        die('Expected `java -jar {}` to fail (with version in output) '
+            'but got exit code 0'
             .format(proguard_jar[0]))
 
     except subprocess.CalledProcessError as e:
         # Exit code is non zero and output is like
         # ProGuard, version 5.3.3
         # Usage: java proguard.ProGuard [options ...]
         output = e.output
 
-    version = Version(e.output.splitlines()[0].split(' ')[-1])
+    version = Version(output.splitlines()[0].split(' ')[-1])
     if version < '5.3.3':
         die('proguard.jar 5.3.3 or higher is required (found %s). '
             'Run |mach bootstrap| to upgrade. ' % version)
 
     return proguard_jar[0]
 
+
 set_config('PROGUARD_JAR', valid_proguard)
--- a/python/mozbuild/mozbuild/test/configure/common.py
+++ b/python/mozbuild/mozbuild/test/configure/common.py
@@ -37,17 +37,16 @@ def ensure_exe_extension(path):
     return path
 
 
 class ConfigureTestVFS(object):
     def __init__(self, paths):
         self._paths = set(mozpath.abspath(p) for p in paths)
 
     def exists(self, path):
-        path = mozpath.abspath(path)
         if path in self._paths:
             return True
         if mozpath.basedir(path, [topsrcdir, topobjdir]):
             return os.path.exists(path)
         return False
 
     def isfile(self, path):
         path = mozpath.abspath(path)
@@ -121,16 +120,25 @@ class ConfigureTestSandbox(ConfigureSand
             return ReadOnlyNamespace(
                 CalledProcessError=subprocess.CalledProcessError,
                 check_output=self.check_output,
                 PIPE=subprocess.PIPE,
                 STDOUT=subprocess.STDOUT,
                 Popen=self.Popen,
             )
 
+        if what == 'os.path':
+            return self.imported_os.path
+
+        if what == 'os.path.exists':
+            return self.imported_os.path.exists
+
+        if what == 'os.path.isfile':
+            return self.imported_os.path.isfile
+
         if what == 'os.environ':
             return self._environ
 
         if what == 'ctypes.wintypes':
             return ReadOnlyNamespace(
                 LPCWSTR=0,
                 LPWSTR=1,
                 DWORD=2,
--- a/python/mozbuild/mozbuild/test/configure/test_checks_configure.py
+++ b/python/mozbuild/mozbuild/test/configure/test_checks_configure.py
@@ -476,199 +476,258 @@ class TestChecksConfigure(unittest.TestC
             checking for a... 
             DEBUG: a: Trying known-a
             ERROR: Paths provided to find_program must be a list of strings, not %r
         ''' % mozpath.dirname(self.OTHER_A)))
 
     def test_java_tool_checks(self):
         includes = ('util.configure', 'checks.configure', 'java.configure')
 
-        def mock_valid_javac(_, args):
-            if len(args) == 1 and args[0] == '-version':
-                return 0, '1.8', ''
-            self.fail("Unexpected arguments to mock_valid_javac: %s" % args)
-
         # A valid set of tools in a standard location.
         java = mozpath.abspath('/usr/bin/java')
         javah = mozpath.abspath('/usr/bin/javah')
         javac = mozpath.abspath('/usr/bin/javac')
         jar = mozpath.abspath('/usr/bin/jar')
         jarsigner = mozpath.abspath('/usr/bin/jarsigner')
         keytool = mozpath.abspath('/usr/bin/keytool')
+        proguard_jar = mozpath.abspath('/path/to/proguard.jar')
+        old_proguard_jar = mozpath.abspath('/path/to/old_proguard.jar')
+
+        def mock_valid_java(_, args):
+            # Yield valid proguard.jar output with a version based on the given path.
+            stdout = \
+                 'ProGuard, version {version}' + \
+                 'Usage: java proguard.ProGuard [options ...]'
+            args = tuple(args)
+            if args == ('-jar', proguard_jar):
+                return 1, stdout.format(version="5.3.3"), ''
+            elif args == ('-jar', old_proguard_jar):
+                return 1, stdout.format(version="4.2"), ''
+            self.fail("Unexpected arguments to mock_valid_java: %s" % args)
+
+        def mock_valid_javac(_, args):
+            if len(args) == 1 and args[0] == '-version':
+                return 0, '1.8', ''
+            self.fail("Unexpected arguments to mock_valid_javac: %s" % args)
 
         paths = {
-            java: None,
+            java: mock_valid_java,
             javah: None,
             javac: mock_valid_javac,
             jar: None,
             jarsigner: None,
             keytool: None,
+            proguard_jar: mock_valid_java,
         }
 
-        config, out, status = self.get_result(includes=includes, extra_paths=paths)
+        config, out, status = self.get_result(includes=includes, extra_paths=paths,
+                                              environ={
+                                                  'PROGUARD_JAR': proguard_jar,
+                                              })
+        self.assertEqual(out, textwrap.dedent('''\
+             checking for java... %s
+             checking for javah... %s
+             checking for jar... %s
+             checking for jarsigner... %s
+             checking for keytool... %s
+             checking for javac... %s
+             checking for javac version... 1.8
+             checking for proguard.jar version... %s
+        ''' % (java, javah, jar, jarsigner, keytool, javac, proguard_jar)))
         self.assertEqual(status, 0)
         self.assertEqual(config, {
             'JAVA': java,
             'JAVAH': javah,
             'JAVAC': javac,
             'JAR': jar,
             'JARSIGNER': jarsigner,
             'KEYTOOL': keytool,
+            'PROGUARD_JAR': proguard_jar,
         })
-        self.assertEqual(out, textwrap.dedent('''\
-             checking for java... %s
-             checking for javah... %s
-             checking for jar... %s
-             checking for jarsigner... %s
-             checking for keytool... %s
-             checking for javac... %s
-             checking for javac version... 1.8
-        ''' % (java, javah, jar, jarsigner, keytool, javac)))
 
         # An alternative valid set of tools referred to by JAVA_HOME.
         alt_java = mozpath.abspath('/usr/local/bin/java')
         alt_javah = mozpath.abspath('/usr/local/bin/javah')
         alt_javac = mozpath.abspath('/usr/local/bin/javac')
         alt_jar = mozpath.abspath('/usr/local/bin/jar')
         alt_jarsigner = mozpath.abspath('/usr/local/bin/jarsigner')
         alt_keytool = mozpath.abspath('/usr/local/bin/keytool')
         alt_java_home = mozpath.dirname(mozpath.dirname(alt_java))
 
         paths.update({
-            alt_java: None,
+            alt_java: mock_valid_java,
             alt_javah: None,
             alt_javac: mock_valid_javac,
             alt_jar: None,
             alt_jarsigner: None,
             alt_keytool: None,
         })
 
         config, out, status = self.get_result(includes=includes,
                                               extra_paths=paths,
                                               environ={
                                                   'JAVA_HOME': alt_java_home,
-                                                  'PATH': mozpath.dirname(java)
+                                                  'PATH': mozpath.dirname(java),
+                                                  'PROGUARD_JAR': proguard_jar,
                                               })
-        self.assertEqual(status, 0)
-        self.assertEqual(config, {
-            'JAVA': alt_java,
-            'JAVAH': alt_javah,
-            'JAVAC': alt_javac,
-            'JAR': alt_jar,
-            'JARSIGNER': alt_jarsigner,
-            'KEYTOOL': alt_keytool,
-        })
         self.assertEqual(out, textwrap.dedent('''\
              checking for java... %s
              checking for javah... %s
              checking for jar... %s
              checking for jarsigner... %s
              checking for keytool... %s
              checking for javac... %s
              checking for javac version... 1.8
+             checking for proguard.jar version... %s
         ''' % (alt_java, alt_javah, alt_jar, alt_jarsigner,
-               alt_keytool, alt_javac)))
+               alt_keytool, alt_javac, proguard_jar)))
+        self.assertEqual(status, 0)
+        self.assertEqual(config, {
+            'JAVA': alt_java,
+            'JAVAH': alt_javah,
+            'JAVAC': alt_javac,
+            'JAR': alt_jar,
+            'JARSIGNER': alt_jarsigner,
+            'KEYTOOL': alt_keytool,
+            'PROGUARD_JAR': proguard_jar,
+        })
 
         # We can use --with-java-bin-path instead of JAVA_HOME to similar
         # effect.
         config, out, status = self.get_result(
             args=['--with-java-bin-path=%s' % mozpath.dirname(alt_java)],
             includes=includes,
             extra_paths=paths,
             environ={
-                'PATH': mozpath.dirname(java)
+                'PATH': mozpath.dirname(java),
+                'PROGUARD_JAR': proguard_jar,
             })
+        self.assertEqual(out, textwrap.dedent('''\
+             checking for java... %s
+             checking for javah... %s
+             checking for jar... %s
+             checking for jarsigner... %s
+             checking for keytool... %s
+             checking for javac... %s
+             checking for javac version... 1.8
+             checking for proguard.jar version... %s
+        ''' % (alt_java, alt_javah, alt_jar, alt_jarsigner,
+               alt_keytool, alt_javac, proguard_jar)))
         self.assertEqual(status, 0)
         self.assertEqual(config, {
             'JAVA': alt_java,
             'JAVAH': alt_javah,
             'JAVAC': alt_javac,
             'JAR': alt_jar,
             'JARSIGNER': alt_jarsigner,
             'KEYTOOL': alt_keytool,
+            'PROGUARD_JAR': proguard_jar,
         })
-        self.assertEqual(out, textwrap.dedent('''\
-             checking for java... %s
-             checking for javah... %s
-             checking for jar... %s
-             checking for jarsigner... %s
-             checking for keytool... %s
-             checking for javac... %s
-             checking for javac version... 1.8
-        ''' % (alt_java, alt_javah, alt_jar, alt_jarsigner,
-               alt_keytool, alt_javac)))
 
         # If --with-java-bin-path and JAVA_HOME are both set,
         # --with-java-bin-path takes precedence.
         config, out, status = self.get_result(
             args=['--with-java-bin-path=%s' % mozpath.dirname(alt_java)],
             includes=includes,
             extra_paths=paths,
             environ={
                 'PATH': mozpath.dirname(java),
                 'JAVA_HOME': mozpath.dirname(mozpath.dirname(java)),
+                'PROGUARD_JAR': proguard_jar,
             })
+        self.assertEqual(out, textwrap.dedent('''\
+             checking for java... %s
+             checking for javah... %s
+             checking for jar... %s
+             checking for jarsigner... %s
+             checking for keytool... %s
+             checking for javac... %s
+             checking for javac version... 1.8
+             checking for proguard.jar version... %s
+        ''' % (alt_java, alt_javah, alt_jar, alt_jarsigner,
+               alt_keytool, alt_javac, proguard_jar)))
         self.assertEqual(status, 0)
         self.assertEqual(config, {
             'JAVA': alt_java,
             'JAVAH': alt_javah,
             'JAVAC': alt_javac,
             'JAR': alt_jar,
             'JARSIGNER': alt_jarsigner,
             'KEYTOOL': alt_keytool,
+            'PROGUARD_JAR': proguard_jar,
         })
+
+        def mock_old_javac(_, args):
+            if len(args) == 1 and args[0] == '-version':
+                return 0, '1.6.9', ''
+            self.fail("Unexpected arguments to mock_old_javac: %s" % args)
+
+        # An old proguard JAR is fatal.
+        config, out, status = self.get_result(includes=includes,
+                                              extra_paths=paths,
+                                              environ={
+                                                  'PATH': mozpath.dirname(java),
+                                                  'PROGUARD_JAR': old_proguard_jar,
+                                              })
         self.assertEqual(out, textwrap.dedent('''\
              checking for java... %s
              checking for javah... %s
              checking for jar... %s
              checking for jarsigner... %s
              checking for keytool... %s
              checking for javac... %s
              checking for javac version... 1.8
-        ''' % (alt_java, alt_javah, alt_jar, alt_jarsigner,
-               alt_keytool, alt_javac)))
-
-        def mock_old_javac(_, args):
-            if len(args) == 1 and args[0] == '-version':
-                return 0, '1.6.9', ''
-            self.fail("Unexpected arguments to mock_old_javac: %s" % args)
-
-        # An old javac is fatal.
-        paths[javac] = mock_old_javac
-        config, out, status = self.get_result(includes=includes,
-                                              extra_paths=paths,
-                                              environ={
-                                                  'PATH': mozpath.dirname(java)
-                                              })
+             checking for proguard.jar version... 
+             ERROR: proguard.jar 5.3.3 or higher is required (looked for %s). Run |mach artifact install --from-build proguard-jar| or add `export PROGUARD_JAR=/path/to/proguard.jar` to your mozconfig.
+        ''' % (java, javah, jar, jarsigner, keytool, javac, old_proguard_jar)))
         self.assertEqual(status, 1)
         self.assertEqual(config, {
             'JAVA': java,
             'JAVAH': javah,
             'JAVAC': javac,
             'JAR': jar,
             'JARSIGNER': jarsigner,
             'KEYTOOL': keytool,
         })
+
+        # An old javac is fatal.
+        paths[javac] = mock_old_javac
+        config, out, status = self.get_result(includes=includes,
+                                              extra_paths=paths,
+                                              environ={
+                                                  'PATH': mozpath.dirname(java),
+                                                  'PROGUARD_JAR': proguard_jar,
+                                              })
         self.assertEqual(out, textwrap.dedent('''\
              checking for java... %s
              checking for javah... %s
              checking for jar... %s
              checking for jarsigner... %s
              checking for keytool... %s
              checking for javac... %s
              checking for javac version... 
              ERROR: javac 1.8 or higher is required (found 1.6.9). Check the JAVA_HOME environment variable.
         ''' % (java, javah, jar, jarsigner, keytool, javac)))
+        self.assertEqual(status, 1)
+        self.assertEqual(config, {
+            'JAVA': java,
+            'JAVAH': javah,
+            'JAVAC': javac,
+            'JAR': jar,
+            'JARSIGNER': jarsigner,
+            'KEYTOOL': keytool,
+        })
 
         # Any missing tool is fatal when these checks run.
         del paths[jarsigner]
         config, out, status = self.get_result(includes=includes,
                                               extra_paths=paths,
                                               environ={
-                                                  'PATH': mozpath.dirname(java)
+                                                  'PATH': mozpath.dirname(java),
+                                                  'PROGUARD_JAR': proguard_jar,
                                               })
         self.assertEqual(status, 1)
         self.assertEqual(config, {
             'JAVA': java,
             'JAVAH': javah,
             'JAR': jar,
             'JARSIGNER': ':',
         })