[Lldb-commits] [lldb] r233284 - Tear down tests in reverse order from setting them up.

Zachary Turner zturner at google.com
Thu Mar 26 09:43:25 PDT 2015


Author: zturner
Date: Thu Mar 26 11:43:25 2015
New Revision: 233284

URL: http://llvm.org/viewvc/llvm-project?rev=233284&view=rev
Log:
Tear down tests in reverse order from setting them up.

Tests derive from TestBase, which derives from Base.  In the
test setUp() methods, we always call TestBase.setUp() first and
then call implementation-specific setup.  Tear down needs to do
the reverse.

This was causing over 20 failures on Windows, and was the culprit
behind about 80% of the files not being cleaned up after test run.
TestBase.tearDown() is responsible for deleting all targets created
during the test run and without this step, on Windows files will
be locked and cannot be deleted.  But TestBase.tearDown() was
calling Base.tearDown() before its own cleanup (i.e. deleting the
targets) and in some cases one of the teardown hooks would be to
call make clean.  So make clean would be run before the targets
had been deleted, and fail to remove the files, and subsequently
result in a failed test as well.

Modified:
    lldb/trunk/test/lldbtest.py
    lldb/trunk/test/make/Makefile.rules
    lldb/trunk/test/types/AbstractBase.py
    lldb/trunk/test/types/HideTestFailures.py

Modified: lldb/trunk/test/lldbtest.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/lldbtest.py?rev=233284&r1=233283&r2=233284&view=diff
==============================================================================
--- lldb/trunk/test/lldbtest.py (original)
+++ lldb/trunk/test/lldbtest.py Thu Mar 26 11:43:25 2015
@@ -339,41 +339,41 @@ def system(commands, **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.')
-    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()
-
-    # Enable trace on failure return while tracking down FreeBSD buildbot issues
-    trace = traceAlways
-    if not trace and retcode and sys.platform.startswith("freebsd"):
-        trace = True
-
-    with recording(test, trace) as sbuf:
-        print >> sbuf
-        print >> sbuf, "os command:", shellCommand
-        print >> sbuf, "with pid:", pid
-        print >> sbuf, "stdout:", output
-        print >> sbuf, "stderr:", error
-        print >> sbuf, "retcode:", retcode
-        print >> sbuf
-
-    if retcode:
-        cmd = kwargs.get("args")
-        if cmd is None:
-            cmd = shellCommand
-        raise CalledProcessError(retcode, cmd)
+    output = ""
+    error = ""
+    for shellCommand in commandList:
+        if 'stdout' in kwargs:
+            raise ValueError('stdout argument not allowed, it will be overridden.')
+        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
+        this_output, this_error = process.communicate()
+        retcode = process.poll()
+
+        # Enable trace on failure return while tracking down FreeBSD buildbot issues
+        trace = traceAlways
+        if not trace and retcode and sys.platform.startswith("freebsd"):
+            trace = True
+
+        with recording(test, trace) as sbuf:
+            print >> sbuf
+            print >> sbuf, "os command:", shellCommand
+            print >> sbuf, "with pid:", pid
+            print >> sbuf, "stdout:", output
+            print >> sbuf, "stderr:", error
+            print >> sbuf, "retcode:", retcode
+            print >> sbuf
+
+        if retcode:
+            cmd = kwargs.get("args")
+            if cmd is None:
+                cmd = shellCommand
+            raise CalledProcessError(retcode, cmd)
+        output = output + this_output
+        error = error + this_error
     return (output, error)
 
 def getsource_if_available(obj):
@@ -1899,8 +1899,6 @@ class TestBase(Base):
         #import traceback
         #traceback.print_stack()
 
-        Base.tearDown(self)
-
         # Delete the target(s) from the debugger as a general cleanup step.
         # This includes terminating the process for each target, if any.
         # We'd like to reuse the debugger for our next test without incurring
@@ -1922,6 +1920,9 @@ class TestBase(Base):
 
         del self.dbg
 
+        # Do this last, to make sure it's in reverse order from how we setup.
+        Base.tearDown(self)
+
     def switch_to_thread_with_stop_reason(self, stop_reason):
         """
         Run the 'thread list' command, and select the thread with stop reason as

Modified: lldb/trunk/test/make/Makefile.rules
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/make/Makefile.rules?rev=233284&r1=233283&r2=233284&view=diff
==============================================================================
--- lldb/trunk/test/make/Makefile.rules (original)
+++ lldb/trunk/test/make/Makefile.rules Thu Mar 26 11:43:25 2015
@@ -472,10 +472,9 @@ ifneq "$(DSYM)" ""
 	$(RM) -r "$(DSYM)"
 endif
 ifeq "$(OS)" "Windows_NT"
-	$(RM) "$(EXE).manifest" $(wildcard *.pdb *.ilk)
+	$(RM) $(wildcard *.manifest *.pdb *.ilk)
   ifneq "$(DYLIB_NAME)" ""
-	  $(RM) $(DYLIB_FILENAME).manifest
-	  $(RM) $(DYLIB_NAME).lib $(DYLIB_NAME).exp
+	$(RM) $(DYLIB_NAME).lib $(DYLIB_NAME).exp
   endif
 endif
 

Modified: lldb/trunk/test/types/AbstractBase.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/types/AbstractBase.py?rev=233284&r1=233283&r2=233284&view=diff
==============================================================================
--- lldb/trunk/test/types/AbstractBase.py (original)
+++ lldb/trunk/test/types/AbstractBase.py Thu Mar 26 11:43:25 2015
@@ -33,10 +33,10 @@ class GenericTester(TestBase):
 
     def tearDown(self):
         """Cleanup the test byproducts."""
-        TestBase.tearDown(self)
         #print "Removing golden-output.txt..."
         if os.path.exists(self.golden_filename):
             os.remove(self.golden_filename)
+        TestBase.tearDown(self)
 
     #==========================================================================#
     # Functions build_and_run() and build_and_run_expr() are generic functions #
@@ -176,6 +176,7 @@ class GenericTester(TestBase):
             nv = ("%s = '%s'" if quotedDisplay else "%s = %s") % (var, val)
             self.expect(output, Msg(var, val, True), exe=False,
                 substrs = [nv])
+        pass
 
     def generic_type_expr_tester(self, exe_name, atoms, quotedDisplay=False, blockCaptured=False):
         """Test that variable expressions with basic types are evaluated correctly."""
@@ -259,3 +260,4 @@ class GenericTester(TestBase):
             valPart = ("'%s'" if quotedDisplay else "%s") % val
             self.expect(output, Msg(var, val, False), exe=False,
                 substrs = [valPart])
+        pass

Modified: lldb/trunk/test/types/HideTestFailures.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/types/HideTestFailures.py?rev=233284&r1=233283&r2=233284&view=diff
==============================================================================
--- lldb/trunk/test/types/HideTestFailures.py (original)
+++ lldb/trunk/test/types/HideTestFailures.py Thu Mar 26 11:43:25 2015
@@ -31,13 +31,13 @@ class DebugIntegerTypesFailures(TestBase
             pass
 
     def tearDown(self):
-        # Call super's tearDown().
-        TestBase.tearDown(self)
         # If we're lucky, test_long_type_with_dsym fails.
         # Let's turn off logging just for that.
         if "test_long_type_with_dsym" in self.id():
             self.runCmd("log disable lldb")
             self.runCmd("log disable gdb-remote")
+        # Call super's tearDown().
+        TestBase.tearDown(self)
 
     @unittest2.skipUnless(sys.platform.startswith("darwin"), "requires Darwin")
     def test_char_type_with_dsym(self):





More information about the lldb-commits mailing list