[NaCl SDK] Fix bug when running naclsdk in path with spaces.

I had to separate the change to naclsdk.bat into a separate CL, landed in r158040. This is just the test and cleanup now.

Also add test.
Also cleanup test output for test_auto_update_sdktools.py.
Also rename test_auto_update_sdktools.py -> test_sdktools.py.

BUG=144403
R=noelallen@chromium.org


Review URL: https://chromiumcodereview.appspot.com/10913308

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@158218 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 59 insertions, 25 deletions

diff --git a/native_client_sdk/src/build_tools/build_updater.py b/native_client_sdk/src/build_tools/build_updater.py
index 8283790..c28b254 100755
--- a/native_client_sdk/src/build_tools/build_updater.py
+++ b/native_client_sdk/src/build_tools/build_updater.py
@@ -155,9 +155,13 @@ def main(args):
   parser = optparse.OptionParser()
   parser.add_option('-o', '--out', help='output directory',
       dest='out_dir', default='out')
+  parser.add_option('-r', '--revision', help='revision number of this updater',
+      dest='revision', default=None)
   options, args = parser.parse_args(args[1:])
 
-  BuildUpdater(options.out_dir)
+  if options.revision:
+    options.revision = int(options.revision)
+  BuildUpdater(options.out_dir, options.revision)
 
 
 if __name__ == '__main__':
diff --git a/native_client_sdk/src/build_tools/tests/test_all.py b/native_client_sdk/src/build_tools/tests/test_all.py
index 945f02b..5a4a19a 100755
--- a/native_client_sdk/src/build_tools/tests/test_all.py
+++ b/native_client_sdk/src/build_tools/tests/test_all.py
@@ -7,7 +7,7 @@ import sys
 import unittest
 
 TEST_MODULES = [
-    'test_auto_update_sdktools',
+    'test_sdktools',
     'test_update_nacl_manifest'
 ]
 
diff --git a/native_client_sdk/src/build_tools/tests/test_auto_update_sdktools.py b/native_client_sdk/src/build_tools/tests/test_sdktools.py
index 0f0e325..9f1b017 100755
--- a/native_client_sdk/src/build_tools/tests/test_auto_update_sdktools.py
+++ b/native_client_sdk/src/build_tools/tests/test_sdktools.py
@@ -13,35 +13,40 @@ import unittest
 
 SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
 BUILD_TOOLS_DIR = os.path.dirname(SCRIPT_DIR)
+TOOLS_DIR = os.path.join(os.path.dirname(BUILD_TOOLS_DIR), 'tools')
 
-sys.path.append(BUILD_TOOLS_DIR)
-import buildbot_common
+sys.path.extend([BUILD_TOOLS_DIR, TOOLS_DIR])
 import build_utils
-import build_updater
 import getos
 import manifest_util
+import oshelpers
 
 
 MANIFEST_BASENAME = 'naclsdk_manifest2.json'
 
+# Attribute '' defined outside __init__
+# pylint: disable=W0201
 
-class TestAutoUpdateSdkTools(unittest.TestCase):
-  def setUp(self):
-    self.basedir = tempfile.mkdtemp()
+class SdkToolsTestCase(unittest.TestCase):
+  def tearDown(self):
+    if self.server:
+      self.server.Shutdown()
+    oshelpers.Remove(['-rf', self.basedir])
+
+  def SetupDefault(self):
+    self.SetupWithBaseDirPrefix('sdktools')
+
+  def SetupWithBaseDirPrefix(self, basedir_prefix):
+    self.basedir = tempfile.mkdtemp(prefix=basedir_prefix)
     # We have to make sure that we build our updaters with a version that is at
     # least as large as the version in the sdk_tools bundle. If not, update
     # tests may fail because the "current" version (according to the sdk_cache)
     # is greater than the version we are attempting to update to.
     self.current_revision = self._GetSdkToolsBundleRevision()
-    build_updater.BuildUpdater(self.basedir, self.current_revision)
+    self._BuildUpdater(self.basedir, self.current_revision)
     self._LoadCacheManifest()
     self.server = test_server.LocalHTTPServer(self.basedir)
 
-  def tearDown(self):
-    if self.server:
-      self.server.Shutdown()
-    buildbot_common.RemoveDir(self.basedir)
-
   def _GetSdkToolsBundleRevision(self):
     """Get the sdk_tools bundle revision.
     We get this from the checked-in path; this is the same file that
@@ -68,6 +73,16 @@ class TestAutoUpdateSdkTools(unittest.TestCase):
     with open(os.path.join(self.basedir, MANIFEST_BASENAME), 'w') as stream:
       stream.write(self.manifest.GetDataAsString())
 
+  def _BuildUpdater(self, out_dir, revision=None):
+    build_updater_py = os.path.join(BUILD_TOOLS_DIR, 'build_updater.py')
+    cmd = [sys.executable, build_updater_py, '-o', out_dir]
+    if revision:
+      cmd.extend(['-r', str(revision)])
+
+    process = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+    _, _ = process.communicate()
+    self.assertEqual(process.returncode, 0)
+
   def _BuildUpdaterArchive(self, rel_path, revision):
     """Build a new sdk_tools bundle.
 
@@ -78,7 +93,7 @@ class TestAutoUpdateSdkTools(unittest.TestCase):
       A manifest_util.Archive() that points to this new bundle on the local
       server.
     """
-    build_updater.BuildUpdater(os.path.join(self.basedir, rel_path), revision)
+    self._BuildUpdater(os.path.join(self.basedir, rel_path), revision)
 
     new_sdk_tools_tgz = os.path.join(self.basedir, rel_path, 'sdk_tools.tgz')
     with open(new_sdk_tools_tgz, 'rb') as sdk_tools_stream:
@@ -108,18 +123,27 @@ class TestAutoUpdateSdkTools(unittest.TestCase):
     self.assertTrue(match is not None)
     return int(match.group(1))
 
+
+class TestSdkTools(SdkToolsTestCase):
+  def testPathHasSpaces(self):
+    """Test that running naclsdk from a path with spaces works."""
+    self.SetupWithBaseDirPrefix('sdk tools')
+    self._WriteManifest()
+    self._RunAndExtractRevision()
+
+
+class TestAutoUpdateSdkTools(SdkToolsTestCase):
+  def setUp(self):
+    self.SetupDefault()
+
   def testNoUpdate(self):
-    """If the current revision is the newest, the shell script will run
-    normally.
-    """
+    """Test that running naclsdk with current revision does nothing."""
     self._WriteManifest()
     revision = self._RunAndExtractRevision()
     self.assertEqual(revision, self.current_revision)
 
   def testUpdate(self):
-    """Create a new bundle with a bumped revision number.
-    When we run the shell script, we should see the new revision number.
-    """
+    """Test that running naclsdk with a new revision will auto-update."""
     new_revision = self.current_revision + 1
     archive = self._BuildUpdaterArchive('new', new_revision)
     self.sdk_tools_bundle.AddArchive(archive)
@@ -130,7 +154,9 @@ class TestAutoUpdateSdkTools(unittest.TestCase):
     self.assertEqual(revision, new_revision)
 
   def testManualUpdateIsIgnored(self):
-    """If the sdk_tools bundle was updated normally (i.e. the old way), it would
+    """Test that attempting to manually update sdk_tools is ignored.
+
+    If the sdk_tools bundle was updated normally (i.e. the old way), it would
     leave a sdk_tools_update folder that would then be copied over on a
     subsequent run. This test ensures that there is no folder made.
     """
diff --git a/native_client_sdk/src/build_tools/tests/test_server.py b/native_client_sdk/src/build_tools/tests/test_server.py
index 3fd791b..2ce0ef1 100644
--- a/native_client_sdk/src/build_tools/tests/test_server.py
+++ b/native_client_sdk/src/build_tools/tests/test_server.py
@@ -4,6 +4,7 @@
 
 import multiprocessing
 import os
+import SimpleHTTPServer
 
 
 class LocalHTTPServer(object):
@@ -37,6 +38,11 @@ class LocalHTTPServer(object):
     return 'http://localhost:%d/%s' % (self.port, rel_url)
 
 
+class QuietHTTPRequestHandler(SimpleHTTPServer.SimpleHTTPRequestHandler):
+  def log_message(self, msg_format, *args):
+    pass
+
+
 def _HTTPServerProcess(conn, serve_dir):
   """Run a local httpserver with a randomly-chosen port.
 
@@ -50,11 +56,9 @@ def _HTTPServerProcess(conn, serve_dir):
        http://localhost:<port>/path/to/filename.
   """
   import BaseHTTPServer
-  import SimpleHTTPServer
 
   os.chdir(serve_dir)
-  httpd = BaseHTTPServer.HTTPServer(('', 0),
-                                    SimpleHTTPServer.SimpleHTTPRequestHandler)
+  httpd = BaseHTTPServer.HTTPServer(('', 0), QuietHTTPRequestHandler)
   conn.send(httpd.server_address[1])  # the chosen port number
   httpd.timeout = 0.5  # seconds
   running = True