Re: [libvirt] [PATCH 03/12] tests: cleanup qemuxml2argvtest
On Tue, Mar 15, 2016 at 14:15:59 +0100, Pavel Hrdina wrote:
This removes the testFailed magic and makes the code more readable.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
tests/qemuxml2argvtest.c | 62 ++++++++++++++++++++++--------------------------
1 file changed, 28 insertions(+), 34 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index e15da37..150a50d 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
...
@@ -391,15 +372,28 @@ static int testCompareXMLToArgvFiles(const char
*xml,
if (virtTestCompareToFile(actualargv, cmdline) < 0)
goto out;
+ ret = 0;
+
ok:
- if (!virtTestOOMActive() &&
- (flags & FLAG_EXPECT_ERROR)) {
- /* need to suppress the errors */
+ if (ret == 0 &&
+ ((flags & FLAG_EXPECT_ERROR) |
+ (flags & FLAG_EXPECT_FAILURE))) {
Are you trying to optimize the code here or is it just a typo (| vs ||)?
:-) In any case
if (ret == 0 &&
(flags & FLAG_EXPECT_ERROR ||
flags & FLAG_EXPECT_FAILURE)) {
or (if you prefer bit operations, which is uglier IMHO)
if (ret == 0 &&
flags & (FLAG_EXPECT_ERROR | FLAG_EXPECT_FAILURE)) {
would look a bit better.
+ ret = -1;
+ VIR_TEST_DEBUG("Error expected but there wasn't any.\n");
+ goto out;
+ }
+ if (!virtTestOOMActive()) {
+ if (flags & FLAG_EXPECT_ERROR) {
+ if ((log = virtTestLogContentAndReset()))
+ VIR_TEST_DEBUG("Got expected error: \n%s", log);
+ } else if (flags & FLAG_EXPECT_FAILURE) {
+ VIR_TEST_DEBUG("Got expected failure: %s\n",
+ virGetLastErrorMessage());
Indentation is off a bit.
+ }
virResetLastError();
+ ret = 0;
}
- ret = 0;
-
out:
VIR_FREE(log);
VIR_FREE(actualargv);
ACK once the nits are fixed.
Jirka