From Dan Kenigsberg <[email protected]>: Dan Kenigsberg has posted comments on this change.
Change subject: testValidation: Add ProcesLeakChecker plugin ...................................................................... Patch Set 2: (4 comments) https://gerrit.ovirt.org/#/c/69324/1/tests/testValidation.py File tests/testValidation.py: Line 113: self._start_processes = self._child_processes() Line 114: Line 115: def stopTest(self, test): Line 116: leaked_processes = self._child_processes() - self._start_processes Line 117: if leaked_processes: > This will be less efficient, see https://gerrit.ovirt.org/69331 this surprises me, even though I remember the old Unix mantra of "do one thing and do it well". are you sure you counted cpu eaten by subprocesses? Line 118: info = [dict(pid=pid, cmdline=utils.getCmdArgs(pid)) Line 119: for pid in leaked_processes] Line 120: raise AssertionError("Test leaked child processes:\n" + Line 121: json.dumps(info, indent=4)) https://gerrit.ovirt.org/#/c/69324/2/tests/testValidation.py File tests/testValidation.py: Line 27: Line 28: from nose.plugins.skip import SkipTest Line 29: from nose.plugins import Plugin Line 30: Line 31: from vdsm import utils argh, sorry. I did this myself in another patch. would you rebase on top of my --with-leaked-thread-check patch? Line 32: from vdsm.compat import CPopen Line 33: Line 34: Line 35: class SlowTestsPlugin(Plugin): Line 120: raise AssertionError("Test leaked child processes:\n" + Line 121: json.dumps(info, indent=4)) Line 122: Line 123: def _child_processes(self): Line 124: cmd = ["pgrep", "-P", "%s" % os.getpid()] you asked me to define a not-so-different constant at the class level, to avoid reevaluation of getpid()! Line 125: proc = CPopen(cmd, stdin=None, stdout=subprocess.PIPE, Line 126: stderr=subprocess.PIPE) Line 127: out, err = proc.communicate() Line 128: # EXIT STATUS Line 123: def _child_processes(self): Line 124: cmd = ["pgrep", "-P", "%s" % os.getpid()] Line 125: proc = CPopen(cmd, stdin=None, stdout=subprocess.PIPE, Line 126: stderr=subprocess.PIPE) Line 127: out, err = proc.communicate() do you have a grudge against execCmd()? /me really wonder why you opted against the simple path. Line 128: # EXIT STATUS Line 129: # 0 One or more processes matched the criteria. Line 130: # 1 No processes matched. Line 131: if proc.returncode not in (0, 1): -- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list -- [email protected] To unsubscribe send an email to [email protected]
