Change in vdsm[master]: v2v: test get_ova_info with via tar input

nsoffer at redhat.com nsoffer at redhat.com
Sun Dec 13 12:37:56 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: v2v: test get_ova_info with via tar input
......................................................................


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/50376/1/tests/v2vTests.py
File tests/v2vTests.py:

Line 366:             # ova as tar file
Line 367:             with tarfile.open(ovapath, 'w') as tar:
Line 368:                 tar.add(ovfpath, arcname='testvm.ovf')
Line 369:             vm = v2v.get_ova_info(ovapath)
Line 370:             self._checkOvaInfoOutput(vm['vmList'])
The reason we asked to split the test was not smaller patches, the original patch was not too big. The reason is having a separate test method per ovf type, so if for example the test for reading ovf from directory fails, the other tests would succeed.

Here is a suggested way to organized these tests:

    class TestGetOVAInfo(VdsmTestCase):

        def test_directory(self):
            ...
            self.check(vm)

        def test_zip(self):
            ...
            self.check(vm)

        def test_tar(self):
            ...
            self.check(vm)

        def check(self, vm):
            # check that vm contains the required fileds

Notes:

- I'm using separate class since these tests have nothing common with other tests, and I want short and clear test names (in the context of testing get_ova_info()).
- test case class should be named TestXXX, for compatibility with other testing frameworks
- I'm using lower_case test methods, this is the only style allowed now for new code.
- I'm not using private names for test helpers, test cases are used only by the testing framework, calling only methods that starts with "test_". Any other name is safe to use, and it does not confuse anyone using this class since nobody is using it but the testing framework.

Please send one patch using these guidelines instead of the 3 patches we have now.
Line 371: 
Line 372:     def _checkOvaInfoOutput(self, vm):
Line 373:         self.assertEquals(vm['vmName'], 'First')
Line 374:         self.assertEquals(vm['memSize'], 2048)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3237198dcba3b668c0954b6e335d4293dc5e5703
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list