Yoav Kleinberger has posted comments on this change. Change subject: tests: fix wrong use of assertions ......................................................................
Patch Set 4: (1 comment) http://gerrit.ovirt.org/#/c/28124/4/tests/testrunner.py File tests/testrunner.py: Line 392: raise AssertionError(msg) Line 393: Line 394: Line 395: def runCommand(*args): Line 396: process = subprocess.Popen(args, stderr=subprocess.PIPE, > All over the code we use the utils.execCmd wrapper (together with its slow it's a bad practice. what is the use of continuing it? bear in mind that this is just the first of many patches that are intended to bring the tests into shape: In general these tests are infected with bad practices (and totally wrong assertions). We should not conform to them - we should fix them, step by step. I suggest to continue in a future patch and remove all the execCmd from the tests. I'm up to it if you are. don't you agree? Line 397: stdout=subprocess.PIPE) Line 398: output, error = process.communicate() Line 399: if process.returncode != 0: Line 400: raise TestRunnerError("subprocess failed: returncode=%s: stderr=%r" % -- To view, visit http://gerrit.ovirt.org/28124 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I693923e6dcaa05bc1479db814c0b7696b3536c9c Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yoav Kleinberger <yklei...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Yoav Kleinberger <yklei...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
