From 65fe1eb5f83c9ec9c4505287cd0a9b1f1b093c17 Mon Sep 17 00:00:00 2001 From: Zachary Turner Date: Thu, 26 Mar 2015 16:43:25 +0000 Subject: [PATCH] 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. llvm-svn: 233284 --- lldb/test/lldbtest.py | 65 +++++++++++++++-------------- lldb/test/make/Makefile.rules | 5 +-- lldb/test/types/AbstractBase.py | 4 +- lldb/test/types/HideTestFailures.py | 4 +- 4 files changed, 40 insertions(+), 38 deletions(-) diff --git a/lldb/test/lldbtest.py b/lldb/test/lldbtest.py index 8ae51fbe4dea..5c5e28852c2d 100644 --- a/lldb/test/lldbtest.py +++ b/lldb/test/lldbtest.py @@ -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) + 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() - 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 - # 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 - 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) + 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 diff --git a/lldb/test/make/Makefile.rules b/lldb/test/make/Makefile.rules index 496839e61147..063ca8822ad6 100644 --- a/lldb/test/make/Makefile.rules +++ b/lldb/test/make/Makefile.rules @@ -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 diff --git a/lldb/test/types/AbstractBase.py b/lldb/test/types/AbstractBase.py index 62fb1af393b4..3b567f86bfc2 100644 --- a/lldb/test/types/AbstractBase.py +++ b/lldb/test/types/AbstractBase.py @@ -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 diff --git a/lldb/test/types/HideTestFailures.py b/lldb/test/types/HideTestFailures.py index 14c7422ec4e3..f7b0dd59e5cf 100644 --- a/lldb/test/types/HideTestFailures.py +++ b/lldb/test/types/HideTestFailures.py @@ -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):