Avoid misleading error message from qemu-img #1372
Conversation
perlpunk
force-pushed
the
qemu-img-fail2
branch
from
bc75fe4
to
81d9fc3
Compare
perlpunk
force-pushed
the
qemu-img-fail2
branch
from
81d9fc3
to
ad2f058
Compare
Codecov Report
@@ Coverage Diff @@
## master #1372 +/- ##
==========================================
+ Coverage 62.38% 63.58% +1.19%
==========================================
Files 52 52
Lines 6187 6189 +2
==========================================
+ Hits 3860 3935 +75
+ Misses 2327 2254 -73
Continue to review full report at Codecov.
|
|
Let's try again :) |
| # 1 even for a successful command on ppc | ||
| # Did we get JSON or an error message? | ||
| my $map = eval { decode_json($json) }; | ||
| if (my $err = $@) { |
There was a problem hiding this comment.
The variable $err is unused.
| my $map = decode_json($json); | ||
| # We can't check the exit code of qemu-img, because it sometimes returns | ||
| # 1 even for a successful command on ppc | ||
| # Did we get JSON or an error message? |
There was a problem hiding this comment.
I wouldn't ask questions in comments. What about:
# determine whether we've got JSON or an error message
# note: We can't check the exit code of qemu-img because it sometimes returns 1 even
# on success under PowerPC.
…dler os-autoinst#1370 was the original approach which we reverted after encountering problems relying on the exit code of qemu-img, especially on ppc64le. Instead we went with os-autoinst#1372 which ended up not really addressing the original problem of preventing the confusing JSON parsing error when we erroneously try to parse an error string as JSON. This commit addresses all the following: * Replace a question in the comment by a statement as we should not ask the reader a question from source code * Delete unused variable "$err" * Prevent any error output from "decode_json" * Ensure to output the error message from qemu-img once and only after "Backend process died", this addresses os-autoinst#1370 (comment) Also verified manually with ``` isotovideo -d iso=/dev/shm/ flavor=DVD casedir=/var/lib/openqa/tests/opensuse ``` Old output: ``` [2020-03-30T16:40:31.535 CEST] [debug] running /usr/bin/qemu-img info --output=json /dev/shm/ [2020-03-30T16:40:31.609 CEST] [debug] qemu-img: Could not open '/dev/shm/': A regular file was expected by the 'file' driver, but something else was given [2020-03-30T16:40:31.609 CEST] [debug] Backend process died, backend errors are reported below in the following lines: malformed JSON string, neither tag, array, object, number, string or atom, at character offset 0 (before "qemu-img: Could not ...") at /usr/lib/perl5/vendor_perl/5.26.1/Mojo/JSON.pm line 39. ``` Example output: ``` [2020-03-30T16:39:20.498 CEST] [debug] running /usr/bin/qemu-img info --output=json /dev/shm/ [2020-03-30T16:39:20.508 CEST] [debug] Backend process died, backend errors are reported below in the following lines: qemu-img: Could not open '/dev/shm/': A regular file was expected by the 'file' driver, but something else was given ``` Related progress issue: https://progress.opensuse.org/issues/64854
Then the original problem was not explained enough in the issue ;-) |
|
sorry if https://progress.opensuse.org/issues/64854 wasn't clear enough. The idea was simply that there is no error about malformed JSON. This is what the acceptance criterion describes and actually you nailed it in #1370 and I have not realized you "brought back" that error line in this PR including a test covering this. I hope you like #1376 |
|
I didn't bring back the error message. It will not be in the log, it's hidden by |
|
And yet it is still visible in openQA tests, e.g. see https://openqa.opensuse.org/tests/1215921 as written in https://progress.opensuse.org/issues/64854#note-10 |
Issue: https://progress.opensuse.org/issues/64854