----- "Lucas Meneghel Rodrigues" <lmr@xxxxxxxxxx> wrote:
> On Tue, 2009-06-16 at 14:00 +0300, Michael Goldish wrote:
> > This is intended to save disk space. Requires ImageMagick (uses
> mogrify).
> >
> > To enable:
> > convert_ppm_files_to_png = yes
> >
> > To enable only for failed tests:
> > convert_ppm_files_to_png_on_error = yes
> >
> > Reminder: by default PPM files are removed after the test (and after
> the
> > conversion, if requested). To keep them specify:
> > keep_ppm_files = yes
> >
> > To keep them only for failed tests:
> > keep_ppm_files_on_error = yes
> >
> > A reasonable choice would be to keep PNG files for failed tests, and
> never
> > keep PPM files. To do this specify
> > convert_ppm_files_to_png_on_error = yes
> > without specifying 'keep_ppm_files = yes' or
> 'keep_ppm_files_on_error = yes'
> > (or explicitly set to 'no', if it was set to 'yes' on a higher level
> in the
> > config hierarchy).
> >
> > This patch also makes small cosmetic changes to the 'keep_ppm_files'
> feature.
> >
> > Signed-off-by: Michael Goldish <mgoldish@xxxxxxxxxx>
> > ---
> > client/tests/kvm/kvm_preprocessing.py | 16 ++++++++++++----
> > 1 files changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/client/tests/kvm/kvm_preprocessing.py
> b/client/tests/kvm/kvm_preprocessing.py
> > index 7fac46a..26e5210 100644
> > --- a/client/tests/kvm/kvm_preprocessing.py
> > +++ b/client/tests/kvm/kvm_preprocessing.py
> > @@ -223,11 +223,19 @@ def postprocess(test, params, env):
> > """
> > process(test, params, env, postprocess_image, postprocess_vm)
> >
> > - # See if we should get rid of all PPM files
> > - if not params.get("keep_ppm_files") == "yes":
> > - # Remove them all
> > + # Should we convert PPM files to PNG format?
> > + if params.get("convert_ppm_files_to_png") == "yes":
> > + logging.debug("'convert_ppm_files_to_png' specified;
> converting PPM"
> > + " files to PNG format...")
> > + mogrify_cmd = ("mogrify -format png %s" %
> > + os.path.join(test.debugdir, "*.ppm"))
> > + kvm_subprocess.run_fg(mogrify_cmd, logging.debug,
> "(mogrify) ",
> > + timeout=30.0)
>
> As the above call can fail if ImageMagick is not installed, we might
> perform a check before (the autotest lib os_dep does rudimentary
> command
> and library presence checks), and proceed with conversion only if the
> command does exist. It's safer this way.
The call should never fail because we currently don't raise exceptions.
It's actually easier to skip the check, because in case ImageMagick
isn't installed, you'll just get a nice message like:
'convert_ppm_files_to_png' specified; converting PPM files to PNG format...
(mogrify) bash: mogrify: command not found
(mogrify) (Process terminated with status 127)
Personally I am fond of these messages because they indicate to me that
kvm_subprocess is functioning properly, and I also think they're quite
readable because they contain "bash's own words", but since we might
raise exceptions in run_bg()/run_fg() in the future, I see your point.
I suppose I should do something like:
logging.debug("converting ...")
found = True
try:
os_dep.command("mogrify")
found = True
except:
logging.error("'mogrify' not found; ImageMagick is probably not installed")
found = False
if found:
mogrify_cmd = ...
kvm_subprocess.run_fg(...)
Do you see a shorter/more elegant syntax to do this?
(We can put the conversion code inside the 'try' clause to avoid using
that 'found' variable, but it's generally recommended to make the code
inside a 'try' clause as specific as possible.)
In any case, I don't want to fail the test just because mogrify is missing,
so we do need to catch the exception raised by os_dep.command().
> > + # Should we keep the PPM files?
> > + if params.get("keep_ppm_files") != "yes":
> > logging.debug("'keep_ppm_files' not specified; removing all
> PPM files"
> > - " from results dir...")
> > + " from debug dir...")
> > rm_cmd = "rm -vf %s" % os.path.join(test.debugdir,
> "*.ppm")
> > kvm_subprocess.run_fg(rm_cmd, logging.debug, "(rm) ",
> timeout=5.0)
> >
> --
> Lucas Meneghel Rodrigues
> Software Engineer (QE)
> Red Hat - Emerging Technologies
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
