From f7713c47e8b4c7f5bca5bc7dd8d95606834db217 Mon Sep 17 00:00:00 2001 From: Marc Abramowitz Date: Tue, 1 Apr 2014 10:15:27 -0700 Subject: [PATCH 1/4] testing/conftest.py: Refactor lsof fd leak checking Isolate the logic into one class to make easier to understand, more maintainable. This may aid in later plugging in an alternative implementation, such as one that uses psutil (https://bitbucket.org/hpk42/pytest/pull-request/137/use-psutil-to-detect-open-files-in-tests/diff) --HG-- branch : refactor_LsofFdLeakChecker --- testing/conftest.py | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/testing/conftest.py b/testing/conftest.py index 204c4490e..fd9f83324 100644 --- a/testing/conftest.py +++ b/testing/conftest.py @@ -4,7 +4,23 @@ import sys pytest_plugins = "pytester", import os, py -pid = os.getpid() + +class LsofFdLeakChecker(object): + def get_open_files(self): + out = self._exec_lsof() + open_files = self._parse_lsof_output(out) + return open_files + + def _exec_lsof(self): + pid = os.getpid() + return py.process.cmdexec("lsof -p %d" % pid) + + def _parse_lsof_output(self, out): + def isopen(line): + return ("REG" in line or "CHR" in line) and ( + "deleted" not in line and 'mem' not in line and "txt" not in line) + return [x for x in out.split("\n") if isopen(x)] + def pytest_addoption(parser): parser.addoption('--lsof', @@ -15,11 +31,12 @@ def pytest_configure(config): config._basedir = py.path.local() if config.getvalue("lsof"): try: - out = py.process.cmdexec("lsof -p %d" % pid) + config._fd_leak_checker = LsofFdLeakChecker() + config._openfiles = config._fd_leak_checker.get_open_files() except py.process.cmdexec.Error: pass else: - config._numfiles = len(getopenfiles(out)) + config._numfiles = len(config._openfiles) #def pytest_report_header(): # return "pid: %s" % os.getpid() @@ -31,8 +48,7 @@ def getopenfiles(out): return [x for x in out.split("\n") if isopen(x)] def check_open_files(config): - out2 = py.process.cmdexec("lsof -p %d" % pid) - lines2 = getopenfiles(out2) + lines2 = config._fd_leak_checker.get_open_files() if len(lines2) > config._numfiles + 3: error = [] error.append("***** %s FD leackage detected" % From 064e79761ce555378cd65a2fa00a97ba71cb1d3c Mon Sep 17 00:00:00 2001 From: Marc Abramowitz Date: Tue, 1 Apr 2014 13:41:35 -0700 Subject: [PATCH 2/4] Improve LsofFdLeakChecker; more reliable and useful leak checking * Make it invoke lsof with options for machine-readable output * Parse out file descriptor and filename from lsof output * Draw attention to file descriptors now open that weren't open before --HG-- branch : refactor_LsofFdLeakChecker --- testing/conftest.py | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/testing/conftest.py b/testing/conftest.py index fd9f83324..c2d49a23c 100644 --- a/testing/conftest.py +++ b/testing/conftest.py @@ -13,13 +13,24 @@ class LsofFdLeakChecker(object): def _exec_lsof(self): pid = os.getpid() - return py.process.cmdexec("lsof -p %d" % pid) + return py.process.cmdexec("lsof -Ffn0 -p %d" % pid) def _parse_lsof_output(self, out): def isopen(line): - return ("REG" in line or "CHR" in line) and ( - "deleted" not in line and 'mem' not in line and "txt" not in line) - return [x for x in out.split("\n") if isopen(x)] + return line.startswith('f') and ( + "deleted" not in line and 'mem' not in line and "txt" not in line and 'cwd' not in line) + + open_files = [] + + for line in out.split("\n"): + if isopen(line): + fields = line.split('\0') + fd = int(fields[0][1:]) + filename = fields[1][1:] + if filename.startswith('/'): + open_files.append((fd, filename)) + + return open_files def pytest_addoption(parser): @@ -49,11 +60,16 @@ def getopenfiles(out): def check_open_files(config): lines2 = config._fd_leak_checker.get_open_files() - if len(lines2) > config._numfiles + 3: + new_fds = sorted(set([t[0] for t in lines2]) - set([t[0] for t in config._openfiles])) + open_files = [t for t in lines2 if t[0] in new_fds] + if open_files: error = [] - error.append("***** %s FD leackage detected" % - (len(lines2)-config._numfiles)) - error.extend(lines2) + error.append("***** %s FD leackage detected" % len(open_files)) + error.extend([str(f) for f in open_files]) + error.append("*** Before:") + error.extend([str(f) for f in config._openfiles]) + error.append("*** After:") + error.extend([str(f) for f in lines2]) error.append(error[0]) # update numfile so that the overall test run continuess config._numfiles = len(lines2) From e45a33f0299cd300e68abb47dd9f9de5e86648a4 Mon Sep 17 00:00:00 2001 From: Marc Abramowitz Date: Tue, 1 Apr 2014 14:13:11 -0700 Subject: [PATCH 3/4] testing/conftest.py: Reintialize config._openfiles for each test And no longer need getopenfiles or config._numfiles --HG-- branch : refactor_LsofFdLeakChecker --- testing/conftest.py | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/testing/conftest.py b/testing/conftest.py index c2d49a23c..ebd5b1b6f 100644 --- a/testing/conftest.py +++ b/testing/conftest.py @@ -38,7 +38,8 @@ def pytest_addoption(parser): action="store_true", dest="lsof", default=False, help=("run FD checks if lsof is available")) -def pytest_configure(config): +def pytest_runtest_setup(item): + config = item.config config._basedir = py.path.local() if config.getvalue("lsof"): try: @@ -46,18 +47,10 @@ def pytest_configure(config): config._openfiles = config._fd_leak_checker.get_open_files() except py.process.cmdexec.Error: pass - else: - config._numfiles = len(config._openfiles) #def pytest_report_header(): # return "pid: %s" % os.getpid() -def getopenfiles(out): - def isopen(line): - return ("REG" in line or "CHR" in line) and ( - "deleted" not in line and 'mem' not in line and "txt" not in line) - return [x for x in out.split("\n") if isopen(x)] - def check_open_files(config): lines2 = config._fd_leak_checker.get_open_files() new_fds = sorted(set([t[0] for t in lines2]) - set([t[0] for t in config._openfiles])) @@ -71,13 +64,11 @@ def check_open_files(config): error.append("*** After:") error.extend([str(f) for f in lines2]) error.append(error[0]) - # update numfile so that the overall test run continuess - config._numfiles = len(lines2) raise AssertionError("\n".join(error)) def pytest_runtest_teardown(item, __multicall__): item.config._basedir.chdir() - if hasattr(item.config, '_numfiles'): + if hasattr(item.config, '_openfiles'): x = __multicall__.execute() check_open_files(item.config) return x From f824a731433fae6ae8c9c5538324a660cb11cbfe Mon Sep 17 00:00:00 2001 From: Marc Abramowitz Date: Tue, 1 Apr 2014 15:36:54 -0700 Subject: [PATCH 4/4] Remove cast of fd to int and sorting Casting of fd can break for non-numeric fd (e.g.: "rtd" on Linux) and isn't necessary since we don't need to sort. --HG-- branch : refactor_LsofFdLeakChecker --- testing/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testing/conftest.py b/testing/conftest.py index ebd5b1b6f..e91a64e89 100644 --- a/testing/conftest.py +++ b/testing/conftest.py @@ -25,7 +25,7 @@ class LsofFdLeakChecker(object): for line in out.split("\n"): if isopen(line): fields = line.split('\0') - fd = int(fields[0][1:]) + fd = fields[0][1:] filename = fields[1][1:] if filename.startswith('/'): open_files.append((fd, filename)) @@ -53,7 +53,7 @@ def pytest_runtest_setup(item): def check_open_files(config): lines2 = config._fd_leak_checker.get_open_files() - new_fds = sorted(set([t[0] for t in lines2]) - set([t[0] for t in config._openfiles])) + new_fds = set([t[0] for t in lines2]) - set([t[0] for t in config._openfiles]) open_files = [t for t in lines2 if t[0] in new_fds] if open_files: error = []