Change in vdsm[master]: testValidation: Add LeakedProcessesPlugin

[Code Review]

From Dan Kenigsberg <danken(a)redhat.com&gt;:

Dan Kenigsberg has posted comments on this change.

Change subject: testValidation: Add LeakedProcessesPlugin
......................................................................


Patch Set 1:

(3 comments)

https://gerrit.ovirt.org/#/c/69324/1/tests/testValidation.py
File tests/testValidation.py:

Line 18: # Refer to the README and COPYING files for full details of the license
Line 19: #
Line 20: import errno
Line 21: import os
Line 22: import json
time to tidy up import order?
Line 23: from nose.plugins.skip import SkipTest
Line 24: from functools import wraps
Line 25: from nose.plugins import Plugin
Line 26: import subprocess


Line 113:             raise AssertionError("Test leaked child processes:\n" +
Line 114:                                  json.dumps(info, indent=4))
Line 115: 
Line 116:     def _child_processes(self):
Line 117:         cmd = ["pgrep", "-P", "%s" % os.getpid()]
stating a subprocess for each tests feels a bit wasteful to me.

Consider doing

subporcesses = set()
for pid in utils.iteratePids():
    try:
        if utils.pidStat(pid).ppid == self.PID:
            subporcesses.add(utils.getCmdArgs(pid))
    except EnvironmentError as e:
        if e.errno != errno.ENOENT:
            raise
        subporcesses.add('terminated')
Line 118:         proc = subprocess.Popen(cmd,
Line 119:                                 stdin=None,
Line 120:                                 stdout=subprocess.PIPE,
Line 121:                                 stderr=subprocess.PIPE)


Line 128:                                % (proc.returncode, err))
Line 129:         return frozenset(out.splitlines())
Line 130: 
Line 131:     def _process_info(self, pid):
Line 132:         path = "/proc/%s/cmdline" % pid
we have vdsm.utils.getCmdArgs() for that.
Line 133:         try:
Line 134:             with open(path) as f:
Line 135:                 line = f.readline()
Line 136:         except EnvironmentError as e:


-- 
To view, visit https://gerrit.ovirt.org/69324
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I451490d56f70ea8a6a685569d984495798e8b297
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes