Submitting nirnimesh's changes.

LGTMed by Elliot Glaysher.
See http://codereview.chromium.org/27079



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

3 files changed, 87 insertions, 45 deletions

diff --git a/tools/purify/common.py b/tools/purify/common.py
index 25b552f..9bd55ae 100644
--- a/tools/purify/common.py
+++ b/tools/purify/common.py
@@ -128,8 +128,9 @@ def RunSubprocess(proc, timeout=0, detach=False):
   elif not detach:
     for line in p.stdout.readlines():
       _print_line(line, False)
-    logging.info("flushing stdout")
-    p.stdout.flush()
+    if sys.platform != 'darwin':   # stdout flush fails on Mac
+      logging.info("flushing stdout")
+      p.stdout.flush()
 
   logging.info("collecting result code")
   result = p.poll()
@@ -331,6 +332,3 @@ class Rational(object):
             except:
               logging.warning("unable to delete file %s: %s" % (file, 
                               sys.exc_info()[0]))
-
-
-
diff --git a/tools/valgrind/chrome_tests.py b/tools/valgrind/chrome_tests.py
index 0292f14..b261c0e 100755
--- a/tools/valgrind/chrome_tests.py
+++ b/tools/valgrind/chrome_tests.py
@@ -75,7 +75,7 @@ class ChromeTests:
     # an absolute Windows-style path
     self._source_dir = utility.GetAbsolutePath(self._source_dir)
     valgrind_test = os.path.join(script_dir, "valgrind_test.py")
-    self._command_preamble = ["python", valgrind_test, "--echo_to_stdout",
+    self._command_preamble = ["python", valgrind_test,
                               "--source_dir=%s" % (self._source_dir)]
 
   def _DefaultCommand(self, module, exe=None):
@@ -354,4 +354,3 @@ def _main(argv):
 if __name__ == "__main__":
   ret = _main(sys.argv)
   sys.exit(ret)
-
diff --git a/tools/valgrind/valgrind_test.py b/tools/valgrind/valgrind_test.py
index c279fca..9075a0e 100755
--- a/tools/valgrind/valgrind_test.py
+++ b/tools/valgrind/valgrind_test.py
@@ -14,12 +14,8 @@ import glob
 import logging
 import optparse
 import os
-import re
 import shutil
 import sys
-import time
-
-import google.path_utils
 
 import common
 
@@ -27,18 +23,26 @@ import valgrind_analyze
 
 rmtree = shutil.rmtree
 
-class Valgrind():
+class Valgrind(object):
+
+  """Abstract class for running Valgrind.
+
+  Always subclass this and implement ValgrindCommand() with platform specific
+  stuff.
+  """
+
   TMP_DIR = "valgrind.tmp"
 
   def __init__(self):
     self._suppressions_files = []
+    # If we have a valgrind.tmp directory, we failed to cleanup last time.
+    if os.path.exists(self.TMP_DIR):
+      shutil.rmtree(self.TMP_DIR)
+    os.mkdir(self.TMP_DIR)
 
   def CreateOptionParser(self):
     self._parser = optparse.OptionParser("usage: %prog [options] <program to "
                                          "test>")
-    self._parser.add_option("-e", "--echo_to_stdout",
-                      dest="echo_to_stdout", action="store_true", default=False,
-                      help="echo purify output to standard output")
     self._parser.add_option("-t", "--timeout",
                       dest="timeout", metavar="TIMEOUT", default=10000,
                       help="timeout in seconds for the run (default 10000)")
@@ -65,37 +69,17 @@ class Valgrind():
   def Setup(self):
     return self.ParseArgv()
 
+  def ValgrindCommand(self):
+    """Get the valgrind command to run."""
+    raise RuntimeError, "Never use Valgrind directly. Always subclass and " \
+                        "implement ValgrindCommand() at least"
+
   def Execute(self):
     ''' Execute the app to be tested after successful instrumentation.
     Full execution command-line provided by subclassers via proc.'''
     logging.info("starting execution...")
-    # note that self._args begins with the exe to be run
-    # TODO(erg): We probably want to get a version of valgrind that supports
-    # the "--track-origins" option...
-    proc = ["valgrind", "--smc-check=all", "--leak-check=full",
-            "--num-callers=30"]
 
-    # Either generate suppressions or load them.
-    if self._generate_suppressions:
-      proc += ["--gen-suppressions=all"]
-    else:
-      proc += ["--xml=yes"]
-
-    suppression_count = 0
-    for suppression_file in self._suppressions:
-      if os.path.exists(suppression_file):
-        suppression_count += 1
-        proc += ["--suppressions=%s" % suppression_file]
-
-    if not suppression_count:
-      logging.warning("WARNING: NOT USING SUPPRESSIONS!")
-
-    proc += ["--log-file=" + self.TMP_DIR + "/valgrind.%p"] + self._args
-
-    # If we have a valgrind.tmp directory, we failed to cleanup last time.
-    if os.path.exists(self.TMP_DIR):
-      shutil.rmtree(self.TMP_DIR)
-    os.mkdir(self.TMP_DIR)
+    proc = self.ValgrindCommand()
     common.RunSubprocess(proc, self._timeout)
 
     # Always return true, even if running the subprocess failed. We depend on
@@ -151,10 +135,71 @@ class Valgrind():
     return retcode
 
 
+class ValgrindLinux(Valgrind):
 
-if __name__ == "__main__":
-  valgrind = Valgrind()
-  retcode = valgrind.Main()
-  sys.exit(retcode)
+  """Valgrind on Linux."""
 
+  def __init__(self):
+    Valgrind.__init__(self)
 
+  def ValgrindCommand(self):
+    """Get the valgrind command to run."""
+    # note that self._args begins with the exe to be run
+    # TODO(erg): We probably want to get a version of valgrind that supports
+    # the "--track-origins" option...
+    proc = ["valgrind", "--smc-check=all", "--leak-check=full",
+            "--num-callers=30"]
+
+    # Either generate suppressions or load them.
+    if self._generate_suppressions:
+      proc += ["--gen-suppressions=all"]
+    else:
+      proc += ["--xml=yes"]
+
+    suppression_count = 0
+    for suppression_file in self._suppressions:
+      if os.path.exists(suppression_file):
+        suppression_count += 1
+        proc += ["--suppressions=%s" % suppression_file]
+
+    if not suppression_count:
+      logging.warning("WARNING: NOT USING SUPPRESSIONS!")
+
+    proc += ["--log-file=" + self.TMP_DIR + "/valgrind.%p"] + self._args
+    return proc
+
+
+class ValgrindMac(Valgrind):
+
+  """Valgrind on Mac OS X.
+
+  Valgrind on OS X does not support suppressions (yet).
+  """
+
+  def __init__(self):
+    Valgrind.__init__(self)
+
+  def ValgrindCommand(self):
+    """Get the valgrind command to run."""
+    proc = ["valgrind", "--leak-check=full"]
+    proc += ["--log-file=" + self.TMP_DIR + "/valgrind.%p"] + self._args
+    return proc
+
+  def Analyze(self):
+    # TODO(nirnimesh): Implement analysis later. Valgrind on Mac is new so
+    # analysis might not be useful until we have stable output from valgring
+    return 0
+
+
+if __name__ == "__main__":
+  if sys.platform == 'darwin': # Mac
+    valgrind = ValgrindMac()
+    retcode = valgrind.Main()
+    sys.exit(retcode)
+  elif sys.platform == 'linux2': # Linux
+    valgrind = ValgrindLinux()
+    retcode = valgrind.Main()
+    sys.exit(retcode)
+  else:
+    logging.error("Unknown platform: %s" % sys.platform)
+    sys.exit(1)