From 1e36446a0aa97404fdfda943cffc258db0c5a012 Mon Sep 17 00:00:00 2001 From: Zachary Turner Date: Tue, 22 Jul 2014 16:19:29 +0000 Subject: [PATCH] Make the test runner understand Windows command shell execution. Currently, the test runner makes the assumption that it will run commands through /bin/sh. This is obviously not true on Windows, so this patch abstracts this logic out somewhat. Instead of having the caller build the command string himself, the caller will now pass in argument list of the form [[a, b], [c, d], ...] which will get converted into a string of the form a b; c d or a b && c d, depending on the platform. Reviewed by: Todd Fiala Differential Revision: http://reviews.llvm.org/D4590 git-svn-id: https://llvm.org/svn/llvm-project/lldb/trunk@213669 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../TestDataFormatterSkipSummary.py | 2 +- .../TestSingleQuoteInFilename.py | 2 +- test/lang/cpp/virtual/TestVirtual.py | 2 +- test/lldbtest.py | 28 +++++---- test/plugins/builder_base.py | 59 +++++++------------ test/plugins/builder_darwin.py | 18 ++---- test/source-manager/TestSourceManager.py | 4 +- 7 files changed, 48 insertions(+), 67 deletions(-) diff --git a/test/functionalities/data-formatter/data-formatter-skip-summary/TestDataFormatterSkipSummary.py b/test/functionalities/data-formatter/data-formatter-skip-summary/TestDataFormatterSkipSummary.py index b59f9f072b..4612c43c17 100644 --- a/test/functionalities/data-formatter/data-formatter-skip-summary/TestDataFormatterSkipSummary.py +++ b/test/functionalities/data-formatter/data-formatter-skip-summary/TestDataFormatterSkipSummary.py @@ -135,7 +135,7 @@ def cleanup(): # Skip the following tests if the condition is met. if self.getCompiler().endswith('gcc') and not self.getCompiler().endswith('llvm-gcc'): import re - gcc_version_output = system([lldbutil.which(self.getCompiler()), "-v"])[1] + gcc_version_output = system([[lldbutil.which(self.getCompiler()), "-v"]])[1] #print "my output:", gcc_version_output for line in gcc_version_output.split(os.linesep): m = re.search('\(Apple Inc\. build ([0-9]+)\)', line) diff --git a/test/functionalities/single-quote-in-filename-to-lldb/TestSingleQuoteInFilename.py b/test/functionalities/single-quote-in-filename-to-lldb/TestSingleQuoteInFilename.py index ccd6cbcb35..8ecf52353b 100644 --- a/test/functionalities/single-quote-in-filename-to-lldb/TestSingleQuoteInFilename.py +++ b/test/functionalities/single-quote-in-filename-to-lldb/TestSingleQuoteInFilename.py @@ -26,7 +26,7 @@ def test_lldb_invocation_with_single_quote_in_filename(self): """Test that 'lldb my_file_name' works where my_file_name is a string with a single quote char in it.""" import pexpect self.buildDefault() - system(["/bin/sh", "-c", "cp a.out \"%s\"" % self.myexe]) + system([["cp", "a.out", "\"%s\"" % self.myexe]]) # The default lldb prompt. prompt = "(lldb) " diff --git a/test/lang/cpp/virtual/TestVirtual.py b/test/lang/cpp/virtual/TestVirtual.py index 4c1fff15e9..49310ade4d 100644 --- a/test/lang/cpp/virtual/TestVirtual.py +++ b/test/lang/cpp/virtual/TestVirtual.py @@ -45,7 +45,7 @@ def virtual_madness_test(self): # First, capture the golden output emitted by the oracle, i.e., the # series of printf statements. - go = system("./a.out", sender=self)[0] + go = system([["./a.out"]], sender=self)[0] # This golden list contains a list of "my_expr = 'value' pairs extracted # from the golden output. gl = [] diff --git a/test/lldbtest.py b/test/lldbtest.py index f719f20cf5..470c4b26e5 100644 --- a/test/lldbtest.py +++ b/test/lldbtest.py @@ -233,7 +233,7 @@ def __exit__(self, type, value, tb): # From 2.7's subprocess.check_output() convenience function. # Return a tuple (stdoutdata, stderrdata). -def system(*popenargs, **kwargs): +def system(commands, **kwargs): r"""Run an os command with arguments and return its output as a byte string. If the exit code was non-zero it raises a CalledProcessError. The @@ -257,20 +257,25 @@ def system(*popenargs, **kwargs): # Assign the sender object to variable 'test' and remove it from kwargs. test = kwargs.pop('sender', None) + separator = None + separator = " && " if os.name == "nt" else "; " + # [['make', 'clean', 'foo'], ['make', 'foo']] -> ['make clean foo', 'make foo'] + commandList = [' '.join(x) for x in commands] + # ['make clean foo', 'make foo'] -> 'make clean foo; make foo' + shellCommand = separator.join(commandList) + if 'stdout' in kwargs: raise ValueError('stdout argument not allowed, it will be overridden.') - process = Popen(stdout=PIPE, stderr=PIPE, *popenargs, **kwargs) + if 'shell' in kwargs and kwargs['shell']==False: + raise ValueError('shell=False not allowed') + process = Popen(shellCommand, stdout=PIPE, stderr=PIPE, shell=True, **kwargs) pid = process.pid output, error = process.communicate() retcode = process.poll() with recording(test, traceAlways) as sbuf: - if isinstance(popenargs, types.StringTypes): - args = [popenargs] - else: - args = list(popenargs) print >> sbuf - print >> sbuf, "os command:", args + print >> sbuf, "os command:", shellCommand print >> sbuf, "with pid:", pid print >> sbuf, "stdout:", output print >> sbuf, "stderr:", error @@ -280,7 +285,7 @@ def system(*popenargs, **kwargs): if retcode: cmd = kwargs.get("args") if cmd is None: - cmd = popenargs[0] + cmd = shellCommand raise CalledProcessError(retcode, cmd) return (output, error) @@ -963,8 +968,8 @@ def tearDown(self): # This is for the case of directly spawning 'lldb' and interacting with it # using pexpect. - import pexpect if self.child and self.child.isalive(): + import pexpect with recording(self, traceAlways) as sbuf: print >> sbuf, "tearing down the child process...." try: @@ -1103,9 +1108,6 @@ def dumpSessionInfo(self): else: benchmarks = False - # This records the compiler version used for the test. - system([self.getCompiler(), "-v"], sender=self) - dname = os.path.join(os.environ["LLDB_TEST"], os.environ["LLDB_SESSION_DIRNAME"]) if not os.path.isdir(dname): @@ -1144,7 +1146,7 @@ def getCompilerVersion(self): version = 'unknown' compiler = self.getCompiler() - version_output = system([which(compiler), "-v"])[1] + version_output = system([[which(compiler), "-v"]])[1] for line in version_output.split(os.linesep): m = re.search('version ([0-9\.]+)', line) if m: diff --git a/test/plugins/builder_base.py b/test/plugins/builder_base.py index c6caa4022f..0ecce3d18d 100644 --- a/test/plugins/builder_base.py +++ b/test/plugins/builder_base.py @@ -32,18 +32,18 @@ def getArchFlag(): elif "gcc" in compiler: archflag = "-m" elif "clang" in compiler: - archflag = "-arch " + archflag = "-arch" else: archflag = None - return (" ARCHFLAG=" + archflag) if archflag else "" + return ("ARCHFLAG=" + archflag) if archflag else "" def getMake(): """Returns the name for GNU make""" if platform.system() == "FreeBSD": - return "gmake " + return "gmake" else: - return "make " + return "make" def getArchSpec(architecture): """ @@ -54,8 +54,7 @@ def getArchSpec(architecture): if not arch and "ARCH" in os.environ: arch = os.environ["ARCH"] - # Note the leading space character. - return (" ARCH=" + arch) if arch else "" + return ("ARCH=" + arch) if arch else "" def getCCSpec(compiler): """ @@ -65,9 +64,10 @@ def getCCSpec(compiler): cc = compiler if compiler else None if not cc and "CC" in os.environ: cc = os.environ["CC"] - - # Note the leading space character. - return (" CC=" + cc) if cc else "" + if cc: + return "CC=\"%s\"" % cc + else: + return "" def getCmdLine(d): """ @@ -81,44 +81,29 @@ def getCmdLine(d): cmdline = " ".join(["%s='%s'" % (k, v) for k, v in d.items()]) - # Note the leading space character. - return " " + cmdline + return cmdline def buildDefault(sender=None, architecture=None, compiler=None, dictionary=None, clean=True): """Build the binaries the default way.""" + commands = [] if clean: - lldbtest.system(["/bin/sh", "-c", - getMake() + "clean" + getCmdLine(dictionary) + ";" - + getMake() - + getArchSpec(architecture) + getCCSpec(compiler) - + getCmdLine(dictionary)], - sender=sender) - else: - lldbtest.system(["/bin/sh", "-c", - getMake() + getArchSpec(architecture) + getCCSpec(compiler) - + getCmdLine(dictionary)], - sender=sender) + commands.append([getMake(), "clean", getCmdLine(dictionary)]) + commands.append([getMake(), getArchSpec(architecture), getCCSpec(compiler), getCmdLine(dictionary)]) + + lldbtest.system(commands, sender=sender) # True signifies that we can handle building default. return True def buildDwarf(sender=None, architecture=None, compiler=None, dictionary=None, clean=True): """Build the binaries with dwarf debug info.""" + commands = [] if clean: - lldbtest.system(["/bin/sh", "-c", - getMake() + "clean" + getCmdLine(dictionary) - + ";" + getMake() + "MAKE_DSYM=NO" - + getArchSpec(architecture) + getCCSpec(compiler) - + getCmdLine(dictionary)], - sender=sender) - else: - lldbtest.system(["/bin/sh", "-c", - getMake() + "MAKE_DSYM=NO" - + getArchSpec(architecture) + getCCSpec(compiler) - + getCmdLine(dictionary)], - sender=sender) + commands.append([getMake(), "clean", getCmdLine(dictionary)]) + commands.append([getMake(), "MAKE_DSYM=NO", getArchSpec(architecture), getCCSpec(compiler), getCmdLine(dictionary)]) + lldbtest.system(commands, sender=sender) # True signifies that we can handle building dwarf. return True @@ -126,10 +111,10 @@ def cleanup(sender=None, dictionary=None): """Perform a platform-specific cleanup after the test.""" #import traceback #traceback.print_stack() + commands = [] if os.path.isfile("Makefile"): - lldbtest.system(["/bin/sh", "-c", - getMake() + "clean" + getCmdLine(dictionary)], - sender=sender) + commands.append([getMake(), "clean", getCmdLine(dictionary)]) + lldbtest.system(commands, sender=sender) # True signifies that we can handle cleanup. return True diff --git a/test/plugins/builder_darwin.py b/test/plugins/builder_darwin.py index 018de8f031..ac5d8aaf69 100644 --- a/test/plugins/builder_darwin.py +++ b/test/plugins/builder_darwin.py @@ -7,19 +7,13 @@ def buildDsym(sender=None, architecture=None, compiler=None, dictionary=None, clean=True): """Build the binaries with dsym debug info.""" + commands = [] + if clean: - lldbtest.system(["/bin/sh", "-c", - "make clean" + getCmdLine(dictionary) - + "; make MAKE_DSYM=YES" - + getArchSpec(architecture) + getCCSpec(compiler) - + getCmdLine(dictionary)], - sender=sender) - else: - lldbtest.system(["/bin/sh", "-c", - "make MAKE_DSYM=YES" - + getArchSpec(architecture) + getCCSpec(compiler) - + getCmdLine(dictionary)], - sender=sender) + commands.append(["make", "clean", getCmdLine(dictionary)]) + commands.append(["make", "MAKE_DSYM=YES", getArchSpec(architecture), getCCSpec(compiler), getCmdLine(dictionary)]) + + lldbtest.system(commands, sender=sender) # True signifies that we can handle building dsym. return True diff --git a/test/source-manager/TestSourceManager.py b/test/source-manager/TestSourceManager.py index bcb73721ff..c4634b848d 100644 --- a/test/source-manager/TestSourceManager.py +++ b/test/source-manager/TestSourceManager.py @@ -92,8 +92,8 @@ def move_and_then_display_source(self): os.rename(main_c, main_c_hidden) if self.TraceOn(): - system(["ls"]) - system(["ls", "hidden"]) + system([["ls"]]) + system([["ls", "hidden"]]) # Restore main.c after the test. self.addTearDownHook(lambda: os.rename(main_c_hidden, main_c))