Yeela Kaplan has uploaded a new change for review.
Change subject: Add hooksTests. ......................................................................
Add hooksTests.
Change-Id: Ic4e9c5acc4ae8a1fa352c7cec4724a930c837257 Signed-off-by: Yeela Kaplan ykaplan@redhat.com --- M tests/Makefile.am A tests/hooksTests.py M vdsm/hooks.py 3 files changed, 69 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/89/3589/1 -- To view, visit http://gerrit.ovirt.org/3589 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: Ic4e9c5acc4ae8a1fa352c7cec4724a930c837257 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykaplan@redhat.com
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@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com
Dan Kenigsberg has posted comments on this change.
Change subject: Add hooksTests. ......................................................................
Patch Set 2: I would prefer that you didn't submit this
(3 inline comments)
really minor stuff left.
note that there are several other things in hooks.py that should be tested: _getHookInfo, _getScriptInfo and even the ability to execute hooks.py as a script. (can wait for another patch).
.................................................... File tests/hooksTests.py Line 40: echo -n %s "$1" >> "$_hook_domxml" why you need "$1" is a mystery to me.
.................................................... File vdsm/hooks.py Line 33: # dir path starts with '/' for test purposes trailing redspace is evil
Line 34: # otherwise starts with P_VDSM_HOOKS starts with -> "path is relative to"
-- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com
Dan Kenigsberg has posted comments on this change.
Change subject: Add hooksTests. ......................................................................
Patch Set 3: Verified; Looks good to me, approved
-- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com
Dan Kenigsberg has posted comments on this change.
Change subject: Add hooksTests. ......................................................................
Patch Set 4: Verified; Looks good to me, approved
-- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Add hooksTests. ......................................................................
Add hooksTests.
Change-Id: Ic4e9c5acc4ae8a1fa352c7cec4724a930c837257 Signed-off-by: Yeela Kaplan ykaplan@redhat.com --- M tests/Makefile.am A tests/hooksTests.py M vdsm/hooks.py 3 files changed, 82 insertions(+), 2 deletions(-)
Approvals: Dan Kenigsberg: Verified; Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/3589 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged Gerrit-Change-Id: Ic4e9c5acc4ae8a1fa352c7cec4724a930c837257 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com
vdsm-patches@lists.fedorahosted.org