Re: [Lldb-commits] [lldb] r324743 - [Testsuite] Remove leak tests, it's not useful anymore.
Apologies, I clearly misunderstood. I'll add this again. On Mon, Feb 12, 2018 at 2:52 AM, Pavel Labath via lldb-commits < lldb-commits@lists.llvm.org> wrote: > I think you misunderstood the purpose the purpose of this test. The > test was there to make sure that *lldb* does not leak file descriptors > into the inferior (when I wrote it a couple of years ago, we were > leaking about half a dozen of them). The python version check was > added there just because we found there are some leaks coming from > python itself that we could not control (so we skipped the test with > those versions). > > With this in mind, I think the test is still very much useful. I think > a better cleanup would be to rewrite this test to not depend on python > so much, e.g. by driving the test from a command line lldb client a'la > the tests in `lldb/lit/Expr` (unfortunately I can't think of a way to > check that an FD is not leaked in a different way than with a > full-scale integration test). > > On 9 February 2018 at 22:48, Vedant Kumar via lldb-commits > wrote: > > Nice! > > > > vedant > > > >> On Feb 9, 2018, at 8:06 AM, Davide Italiano via lldb-commits < > lldb-commits@lists.llvm.org> wrote: > >> > >> Author: davide > >> Date: Fri Feb 9 08:06:39 2018 > >> New Revision: 324743 > >> > >> URL: http://llvm.org/viewvc/llvm-project?rev=324743&view=rev > >> Log: > >> [Testsuite] Remove leak tests, it's not useful anymore. > >> > >> This only worked on MacOS, which now ships a newer version of > >> python without this bug. As such, we don't leak the fd, and > >> this test is not needed anymore (as it also hardcoded the python > >> version in the check). > >> > >> Removed: > >>lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd- > leak/Makefile > >>lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd- > leak/TestFdLeak.py > >>lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd- > leak/main.c > >> > >> Removed: lldb/trunk/packages/Python/lldbsuite/test/ > functionalities/avoids-fd-leak/Makefile > >> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/ > Python/lldbsuite/test/functionalities/avoids-fd- > leak/Makefile?rev=324742&view=auto > >> > == > >> --- lldb/trunk/packages/Python/lldbsuite/test/ > functionalities/avoids-fd-leak/Makefile (original) > >> +++ lldb/trunk/packages/Python/lldbsuite/test/ > functionalities/avoids-fd-leak/Makefile (removed) > >> @@ -1,5 +0,0 @@ > >> -LEVEL = ../../make > >> - > >> -C_SOURCES := main.c > >> - > >> -include $(LEVEL)/Makefile.rules > >> > >> Removed: lldb/trunk/packages/Python/lldbsuite/test/ > functionalities/avoids-fd-leak/TestFdLeak.py > >> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/ > Python/lldbsuite/test/functionalities/avoids-fd- > leak/TestFdLeak.py?rev=324742&view=auto > >> > == > >> --- lldb/trunk/packages/Python/lldbsuite/test/ > functionalities/avoids-fd-leak/TestFdLeak.py (original) > >> +++ lldb/trunk/packages/Python/lldbsuite/test/ > functionalities/avoids-fd-leak/TestFdLeak.py (removed) > >> @@ -1,108 +0,0 @@ > >> -""" > >> -Test whether a process started by lldb has no extra file descriptors > open. > >> -""" > >> - > >> -from __future__ import print_function > >> - > >> - > >> -import os > >> -import lldb > >> -from lldbsuite.test import lldbutil > >> -from lldbsuite.test.lldbtest import * > >> -from lldbsuite.test.decorators import * > >> - > >> - > >> -def python_leaky_fd_version(test): > >> -import sys > >> -# Python random module leaks file descriptors on some versions. > >> -if sys.version_info >= (2, 7, 8) and sys.version_info < (2, 7, 10): > >> -return "Python random module leaks file descriptors in this > python version" > >> -return None > >> - > >> - > >> -class AvoidsFdLeakTestCase(TestBase): > >> - > >> -NO_DEBUG_INFO_TESTCASE = True > >> - > >> -mydir = TestBase.compute_mydir(__file__) > >> - > >> -@expectedFailure(python_leaky_fd_version, "bugs.freebsd.org/197376 > ") > >> -@expectedFailureAll( > >> -oslist=['freebsd'], > >> -bugnumber="llvm.org/pr25624 still failing with Python 2.7.10") > >> -# The check for descriptor leakage needs to be implemented > differently > >> -# here. > >> -@skipIfWindows > >> -@skipIfTargetAndroid() # Android have some other file descriptors > open by the shell > >> -@skipIfDarwinEmbedded # # debugserver > on ios has an extra fd open on launch > >> -def test_fd_leak_basic(self): > >> -self.do_test([]) > >> - > >> -@expectedFailure(python_leaky_fd_version, "bugs.freebsd.org/197376 > ") > >> -@expectedFailureAll( > >> -oslist=['freebsd'], > >> -bugnumber="llvm.org/pr25624 still failing with Python 2.7.10") > >> -# The check for descriptor leakage needs to be implemented > differ
Re: [Lldb-commits] [lldb] r324743 - [Testsuite] Remove leak tests, it's not useful anymore.
I think you misunderstood the purpose the purpose of this test. The test was there to make sure that *lldb* does not leak file descriptors into the inferior (when I wrote it a couple of years ago, we were leaking about half a dozen of them). The python version check was added there just because we found there are some leaks coming from python itself that we could not control (so we skipped the test with those versions). With this in mind, I think the test is still very much useful. I think a better cleanup would be to rewrite this test to not depend on python so much, e.g. by driving the test from a command line lldb client a'la the tests in `lldb/lit/Expr` (unfortunately I can't think of a way to check that an FD is not leaked in a different way than with a full-scale integration test). On 9 February 2018 at 22:48, Vedant Kumar via lldb-commits wrote: > Nice! > > vedant > >> On Feb 9, 2018, at 8:06 AM, Davide Italiano via lldb-commits >> wrote: >> >> Author: davide >> Date: Fri Feb 9 08:06:39 2018 >> New Revision: 324743 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=324743&view=rev >> Log: >> [Testsuite] Remove leak tests, it's not useful anymore. >> >> This only worked on MacOS, which now ships a newer version of >> python without this bug. As such, we don't leak the fd, and >> this test is not needed anymore (as it also hardcoded the python >> version in the check). >> >> Removed: >> >> lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile >> >> lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py >> >> lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/main.c >> >> Removed: >> lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile >> URL: >> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile?rev=324742&view=auto >> == >> --- >> lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile >> (original) >> +++ >> lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile >> (removed) >> @@ -1,5 +0,0 @@ >> -LEVEL = ../../make >> - >> -C_SOURCES := main.c >> - >> -include $(LEVEL)/Makefile.rules >> >> Removed: >> lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py >> URL: >> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py?rev=324742&view=auto >> == >> --- >> lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py >> (original) >> +++ >> lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py >> (removed) >> @@ -1,108 +0,0 @@ >> -""" >> -Test whether a process started by lldb has no extra file descriptors open. >> -""" >> - >> -from __future__ import print_function >> - >> - >> -import os >> -import lldb >> -from lldbsuite.test import lldbutil >> -from lldbsuite.test.lldbtest import * >> -from lldbsuite.test.decorators import * >> - >> - >> -def python_leaky_fd_version(test): >> -import sys >> -# Python random module leaks file descriptors on some versions. >> -if sys.version_info >= (2, 7, 8) and sys.version_info < (2, 7, 10): >> -return "Python random module leaks file descriptors in this python >> version" >> -return None >> - >> - >> -class AvoidsFdLeakTestCase(TestBase): >> - >> -NO_DEBUG_INFO_TESTCASE = True >> - >> -mydir = TestBase.compute_mydir(__file__) >> - >> -@expectedFailure(python_leaky_fd_version, "bugs.freebsd.org/197376") >> -@expectedFailureAll( >> -oslist=['freebsd'], >> -bugnumber="llvm.org/pr25624 still failing with Python 2.7.10") >> -# The check for descriptor leakage needs to be implemented differently >> -# here. >> -@skipIfWindows >> -@skipIfTargetAndroid() # Android have some other file descriptors open >> by the shell >> -@skipIfDarwinEmbedded # # debugserver on ios >> has an extra fd open on launch >> -def test_fd_leak_basic(self): >> -self.do_test([]) >> - >> -@expectedFailure(python_leaky_fd_version, "bugs.freebsd.org/197376") >> -@expectedFailureAll( >> -oslist=['freebsd'], >> -bugnumber="llvm.org/pr25624 still failing with Python 2.7.10") >> -# The check for descriptor leakage needs to be implemented differently >> -# here. >> -@skipIfWindows >> -@skipIfTargetAndroid() # Android have some other file descriptors open >> by the shell >> -@skipIfDarwinEmbedded # # debugserver on ios >> has an extra fd open on launch >> -def test_fd_leak_log(self): >> -self.do_test(["log enable -f '/dev/null' lldb commands"]) >> - >> -def do_test(self, commands): >> -
Re: [Lldb-commits] [lldb] r324743 - [Testsuite] Remove leak tests, it's not useful anymore.
Nice! vedant > On Feb 9, 2018, at 8:06 AM, Davide Italiano via lldb-commits > wrote: > > Author: davide > Date: Fri Feb 9 08:06:39 2018 > New Revision: 324743 > > URL: http://llvm.org/viewvc/llvm-project?rev=324743&view=rev > Log: > [Testsuite] Remove leak tests, it's not useful anymore. > > This only worked on MacOS, which now ships a newer version of > python without this bug. As such, we don't leak the fd, and > this test is not needed anymore (as it also hardcoded the python > version in the check). > > Removed: > > lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile > > lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py > > lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/main.c > > Removed: > lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile?rev=324742&view=auto > == > --- > lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile > (original) > +++ > lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile > (removed) > @@ -1,5 +0,0 @@ > -LEVEL = ../../make > - > -C_SOURCES := main.c > - > -include $(LEVEL)/Makefile.rules > > Removed: > lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py?rev=324742&view=auto > == > --- > lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py > (original) > +++ > lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py > (removed) > @@ -1,108 +0,0 @@ > -""" > -Test whether a process started by lldb has no extra file descriptors open. > -""" > - > -from __future__ import print_function > - > - > -import os > -import lldb > -from lldbsuite.test import lldbutil > -from lldbsuite.test.lldbtest import * > -from lldbsuite.test.decorators import * > - > - > -def python_leaky_fd_version(test): > -import sys > -# Python random module leaks file descriptors on some versions. > -if sys.version_info >= (2, 7, 8) and sys.version_info < (2, 7, 10): > -return "Python random module leaks file descriptors in this python > version" > -return None > - > - > -class AvoidsFdLeakTestCase(TestBase): > - > -NO_DEBUG_INFO_TESTCASE = True > - > -mydir = TestBase.compute_mydir(__file__) > - > -@expectedFailure(python_leaky_fd_version, "bugs.freebsd.org/197376") > -@expectedFailureAll( > -oslist=['freebsd'], > -bugnumber="llvm.org/pr25624 still failing with Python 2.7.10") > -# The check for descriptor leakage needs to be implemented differently > -# here. > -@skipIfWindows > -@skipIfTargetAndroid() # Android have some other file descriptors open > by the shell > -@skipIfDarwinEmbedded # # debugserver on ios > has an extra fd open on launch > -def test_fd_leak_basic(self): > -self.do_test([]) > - > -@expectedFailure(python_leaky_fd_version, "bugs.freebsd.org/197376") > -@expectedFailureAll( > -oslist=['freebsd'], > -bugnumber="llvm.org/pr25624 still failing with Python 2.7.10") > -# The check for descriptor leakage needs to be implemented differently > -# here. > -@skipIfWindows > -@skipIfTargetAndroid() # Android have some other file descriptors open > by the shell > -@skipIfDarwinEmbedded # # debugserver on ios > has an extra fd open on launch > -def test_fd_leak_log(self): > -self.do_test(["log enable -f '/dev/null' lldb commands"]) > - > -def do_test(self, commands): > -self.build() > -exe = self.getBuildArtifact("a.out") > - > -for c in commands: > -self.runCmd(c) > - > -target = self.dbg.CreateTarget(exe) > - > -process = target.LaunchSimple( > -None, None, self.get_process_working_directory()) > -self.assertTrue(process, PROCESS_IS_VALID) > - > -self.assertTrue( > -process.GetState() == lldb.eStateExited, > -"Process should have exited.") > -self.assertTrue( > -process.GetExitStatus() == 0, > -"Process returned non-zero status. Were incorrect file > descriptors passed?") > - > -@expectedFailure(python_leaky_fd_version, "bugs.freebsd.org/197376") > -@expectedFailureAll( > -oslist=['freebsd'], > -bugnumber="llvm.org/pr25624 still failing with Python 2.7.10") > -# The check for descriptor leakage needs to be implemented differently > -# here. > -@skipIfWindows > -
[Lldb-commits] [lldb] r324743 - [Testsuite] Remove leak tests, it's not useful anymore.
Author: davide Date: Fri Feb 9 08:06:39 2018 New Revision: 324743 URL: http://llvm.org/viewvc/llvm-project?rev=324743&view=rev Log: [Testsuite] Remove leak tests, it's not useful anymore. This only worked on MacOS, which now ships a newer version of python without this bug. As such, we don't leak the fd, and this test is not needed anymore (as it also hardcoded the python version in the check). Removed: lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/main.c Removed: lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile?rev=324742&view=auto == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile (removed) @@ -1,5 +0,0 @@ -LEVEL = ../../make - -C_SOURCES := main.c - -include $(LEVEL)/Makefile.rules Removed: lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py?rev=324742&view=auto == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py (removed) @@ -1,108 +0,0 @@ -""" -Test whether a process started by lldb has no extra file descriptors open. -""" - -from __future__ import print_function - - -import os -import lldb -from lldbsuite.test import lldbutil -from lldbsuite.test.lldbtest import * -from lldbsuite.test.decorators import * - - -def python_leaky_fd_version(test): -import sys -# Python random module leaks file descriptors on some versions. -if sys.version_info >= (2, 7, 8) and sys.version_info < (2, 7, 10): -return "Python random module leaks file descriptors in this python version" -return None - - -class AvoidsFdLeakTestCase(TestBase): - -NO_DEBUG_INFO_TESTCASE = True - -mydir = TestBase.compute_mydir(__file__) - -@expectedFailure(python_leaky_fd_version, "bugs.freebsd.org/197376") -@expectedFailureAll( -oslist=['freebsd'], -bugnumber="llvm.org/pr25624 still failing with Python 2.7.10") -# The check for descriptor leakage needs to be implemented differently -# here. -@skipIfWindows -@skipIfTargetAndroid() # Android have some other file descriptors open by the shell -@skipIfDarwinEmbedded # # debugserver on ios has an extra fd open on launch -def test_fd_leak_basic(self): -self.do_test([]) - -@expectedFailure(python_leaky_fd_version, "bugs.freebsd.org/197376") -@expectedFailureAll( -oslist=['freebsd'], -bugnumber="llvm.org/pr25624 still failing with Python 2.7.10") -# The check for descriptor leakage needs to be implemented differently -# here. -@skipIfWindows -@skipIfTargetAndroid() # Android have some other file descriptors open by the shell -@skipIfDarwinEmbedded # # debugserver on ios has an extra fd open on launch -def test_fd_leak_log(self): -self.do_test(["log enable -f '/dev/null' lldb commands"]) - -def do_test(self, commands): -self.build() -exe = self.getBuildArtifact("a.out") - -for c in commands: -self.runCmd(c) - -target = self.dbg.CreateTarget(exe) - -process = target.LaunchSimple( -None, None, self.get_process_working_directory()) -self.assertTrue(process, PROCESS_IS_VALID) - -self.assertTrue( -process.GetState() == lldb.eStateExited, -"Process should have exited.") -self.assertTrue( -process.GetExitStatus() == 0, -"Process returned non-zero status. Were incorrect file descriptors passed?") - -@expectedFailure(python_leaky_fd_version, "bugs.freebsd.org/197376") -@expectedFailureAll( -oslist=['freebsd'], -bugnumber="llvm.org/pr25624 still failing with Python 2.7.10") -# The check for descriptor leakage needs to be implemented differently -# here. -@skipIfWindows -@skipIfTargetAndroid() # Android have some other file descriptors open by the shell -@skipIfDarwinEmbedded # # debugserver on ios has an extra fd open on launch -def test_fd_leak_multitarget(self): -self.build() -exe = self.getBuildArtifact("a.out") - -target = self.dbg.CreateTarget(exe) -breakpoint = target.Brea