Dan Kenigsberg has posted comments on this change.

Change subject: minimalistic (incomplete) test for mkimage.py with fail for non-64base data ......................................................................

Patch Set 1: I would prefer that you didn't submit this

(4 inline comments)

Josef, thanks for your contribution!

Would you make sure that your test works? (on my machine, it does not)

.................................................... File tests/mkimageTests.py Line 1: #author: Josef Pacula, josef.pacula@gmail.com, 2012 We would need the GPL boiler plate in order to accept code in Vdsm. sorry. lawyers. you know.

Line 12: testFiles = {'fileA': 'content of the file A', I suggest that you first try writing a test with a base64 encoding of the content, e.g. 'Y29udGVudCBvZiB0aGUgZmlsZSBB'. Your testFiles causes an error condition, which should happen only in an advanced test.

Line 17: self.assertTrue(mount.isMounted()) did this ever worked? how? you never call mount() here. If you do mount, you should limit this tests to root user only.

.................................................... File vdsm/mkimage.py Line 94: isopath = _getFileName(vmId, files) there's a nasty bug in this function, and you are not completely fixing it.

Since isopath and dirname are mentioned in the "finally" block of this function, they must be assigned before the "try". For example, when your code is run as non-root, I still get

UnboundLocalError: local variable 'dirname' referenced before assignment

in any case, please split this bug-fixing from unittest - it deserves a commit message of its own.

-- To view, visit http://gerrit.ovirt.org/6275 To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment Gerrit-Change-Id: I1a7656359beb5d116ad1919e237fe084a1208d71 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Josef Pacula josef.pacula@gmail.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com

oVirt Jenkins CI Server has posted comments on this change.

Change subject: minimalistic (incomplete) test for mkimage.py with fail for non-64base data ......................................................................

Patch Set 1: No score

Build Started http://jenkins.ovirt.info/job/vdsm_unit_tests_by_patch/166/

-- To view, visit http://gerrit.ovirt.org/6275 To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment Gerrit-Change-Id: I1a7656359beb5d116ad1919e237fe084a1208d71 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Josef Pacula josef.pacula@gmail.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server

oVirt Jenkins CI Server has posted comments on this change.

Change subject: minimalistic (incomplete) test for mkimage.py with fail for non-64base data ......................................................................

Patch Set 1:

Build Started http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/380/

-- To view, visit http://gerrit.ovirt.org/6275 To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment Gerrit-Change-Id: I1a7656359beb5d116ad1919e237fe084a1208d71 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Josef Pacula josef.pacula@gmail.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server

oVirt Jenkins CI Server has posted comments on this change.

Change subject: minimalistic (incomplete) test for mkimage.py with fail for non-64base data ......................................................................

Patch Set 1:

Build Started http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/391/

-- To view, visit http://gerrit.ovirt.org/6275 To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment Gerrit-Change-Id: I1a7656359beb5d116ad1919e237fe084a1208d71 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Josef Pacula josef.pacula@gmail.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skrivanek@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server

Itamar Heim has posted comments on this change.

Change subject: minimalistic (incomplete) test for mkimage.py with fail for non-64base data ......................................................................

Patch Set 1:

still relevant or should be abandoned?