Dan Kenigsberg has posted comments on this change.
Change subject: Add hooksTests.
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(5 inline comments)
....................................................
File tests/hooksTests.py
Line 35: echo -n %s $1 >> "$_hook_domxml"
"$1" should be quoted too. (just for good practice, here it means very little)
hmm, why do you have it here at all?
Line 58: dirName = tempfile.mkdtemp(dir="/tmp")
why do you care that dir="/tmp"? mkdtemp does the Right Thing on its own.
Line 62: os.rmdir(dirName)
you can easily add a test for hooks._scriptsPerDir
....................................................
File vdsm/hooks.py
Line 34: if (dir[0]=='/'):
wrap with spaces, to make pep8 happy
Line 41: def _runHooksDir(domxml, dir, vmconf={}, raiseError=True):
please add a docstring about the funny new sematics of "dir"
(a leading / means absolute path, anything else is relative to P_VDSM_HOOKS)
--
To view, visit
http://gerrit.ovirt.org/3589
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4e9c5acc4ae8a1fa352c7cec4724a930c837257
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>