Nir Soffer has posted comments on this change. Change subject: v2v: small test improvement ......................................................................
Patch Set 1: (2 comments) https://gerrit.ovirt.org/#/c/56694/1/tests/v2vTests.py File tests/v2vTests.py: Line 377: self.assertEqual( Line 378: sorted(vm['vmName'] for vm in vms), Line 379: sorted(spec.name for spec in VM_SPECS Line 380: if spec.active == active) Line 381: ) This is much more complicated compared with the previous assertions. Previously we asserted that we have 2 vms in the result, which should match the list of vm specification (2 vms were active). What is this complicated assert buy us? Test should be simple as possible, otherwise we may incorrect tests. Line 382: Line 383: def testOutputParser(self): Line 384: output = ''.join(['[ 0.0] Opening the source -i libvirt ://roo...\n', Line 385: '[ 1.0] Creating an overlay to protect the f...\n', Line 516: def test_list_defined_domains(self): Line 517: vms = self._mock.listDefinedDomains() Line 518: self.assertEqual( Line 519: sorted(vms), Line 520: sorted(spec.name for spec in VM_SPECS if not spec.active)) Same, why is this better then the old simpler test? Line 521: Line 522: def test_list_domains_id(self): Line 523: vms = self._mock.listDomainsID() Line 524: self.assertEqual(len(vms), 2) -- To view, visit https://gerrit.ovirt.org/56694 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I89c46efc9836fe0f0ef680084f1921ef3948055f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky <tgole...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Tomas Golembiovsky <tgole...@redhat.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
