[KVM-AUTOTEST PATCH 0/4] Step file tests: introducing a new feature and some small changes

[KVM-AUTOTEST PATCH 0/4] Step file tests: introducing a new feature and some small changes

[Michael Goldish]

The following patches make some changes to step file tests:
* Rename parameter output_dir in barrier_2() to debug_dir.
* Move some code from run_steps() into barrier_2().
* Add a new feature (keep_screendump_history) which allows the user to see
  exactly what went wrong with a step file test. Currently only the last
  screendump is kept for each failed step file test; with this feature enabled,
  all screendumps are kept in compact JPG format.
* Use the new feature by default in kvm_tests.cfg.sample.

[KVM-AUTOTEST PATCH 1/4] kvm_guest_wizard: rename output_dir to debug_dir in barrier_2()

[Michael Goldish]

The name 'debug_dir' makes it clearer that it corresponds to test.debugdir.

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_guest_wizard.py |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/client/tests/kvm/kvm_guest_wizard.py b/client/tests/kvm/kvm_guest_wizard.py
index 2dd9be5..143e61e 100644
--- a/client/tests/kvm/kvm_guest_wizard.py
+++ b/client/tests/kvm/kvm_guest_wizard.py
@@ -18,7 +18,7 @@ def handle_var(vm, params, varname):
 
 
 def barrier_2(vm, words, fail_if_stuck_for, stuck_detection_history,
-              output_dir, data_scrdump_filename, current_step_num):
+              debug_dir, data_scrdump_filename, current_step_num):
     if len(words) < 7:
         logging.error("Bad barrier_2 command line")
         return False
@@ -34,12 +34,12 @@ def barrier_2(vm, words, fail_if_stuck_for, stuck_detection_history,
     if sleep_duration < 1.0: sleep_duration = 1.0
     if sleep_duration > 10.0: sleep_duration = 10.0
 
-    scrdump_filename = os.path.join(output_dir, "scrdump.ppm")
-    cropped_scrdump_filename = os.path.join(output_dir, "cropped_scrdump.ppm")
-    expected_scrdump_filename = os.path.join(output_dir, "scrdump_expected.ppm")
-    expected_cropped_scrdump_filename = os.path.join(output_dir,
+    scrdump_filename = os.path.join(debug_dir, "scrdump.ppm")
+    cropped_scrdump_filename = os.path.join(debug_dir, "cropped_scrdump.ppm")
+    expected_scrdump_filename = os.path.join(debug_dir, "scrdump_expected.ppm")
+    expected_cropped_scrdump_filename = os.path.join(debug_dir,
                                                  "cropped_scrdump_expected.ppm")
-    comparison_filename = os.path.join(output_dir, "comparison.ppm")
+    comparison_filename = os.path.join(debug_dir, "comparison.ppm")
 
     end_time = time.time() + timeout
     end_time_stuck = time.time() + fail_if_stuck_for
@@ -99,7 +99,7 @@ def barrier_2(vm, words, fail_if_stuck_for, stuck_detection_history,
         prev_whole_image_md5sums.insert(0, whole_image_md5sum)
         # Limit queue length to stuck_detection_history
         prev_whole_image_md5sums = \
-        prev_whole_image_md5sums[:stuck_detection_history]
+                prev_whole_image_md5sums[:stuck_detection_history]
 
         # Sleep for a while
         time.sleep(sleep_duration)
@@ -113,12 +113,12 @@ def barrier_2(vm, words, fail_if_stuck_for, stuck_detection_history,
         logging.info(message)
         return False
     else:
-        # Collect information and put it in output_dir
+        # Collect information and put it in debug_dir
         if data_scrdump_filename and os.path.exists(data_scrdump_filename):
             # Read expected screendump image
             (ew, eh, edata) = \
             ppm_utils.image_read_from_ppm_file(data_scrdump_filename)
-            # Write it in output_dir
+            # Write it in debug_dir
             ppm_utils.image_write_to_ppm_file(expected_scrdump_filename,
                                               ew, eh, edata)
             # Write the cropped version as well
@@ -131,7 +131,7 @@ def barrier_2(vm, words, fail_if_stuck_for, stuck_detection_history,
                 ppm_utils.image_write_to_ppm_file(comparison_filename, w, h,
                                                   data)
         # Print error messages and fail the test
-        long_message = message + "\n(see analysis at %s)" % output_dir
+        long_message = message + "\n(see analysis at %s)" % debug_dir
         logging.error(long_message)
         raise error.TestFail, message
 
-- 
1.5.4.1

[KVM-AUTOTEST PATCH 2/4] kvm_guest_wizard: pass 'params' directly to barrier_2()

[Michael Goldish]

Currently parameters for barrier_2() are extracted from 'params' in the main
run_steps() test routine, and then passed to barrier_2().
Instead, let barrier_2() extract parameters from 'params' as it sees fit.
This will make adding new parameters slightly easier and cleaner.

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_guest_wizard.py |   37 ++++++++++++++++-----------------
 1 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/client/tests/kvm/kvm_guest_wizard.py b/client/tests/kvm/kvm_guest_wizard.py
index 143e61e..eb0e2d5 100644
--- a/client/tests/kvm/kvm_guest_wizard.py
+++ b/client/tests/kvm/kvm_guest_wizard.py
@@ -17,8 +17,8 @@ def handle_var(vm, params, varname):
     return True
 
 
-def barrier_2(vm, words, fail_if_stuck_for, stuck_detection_history,
-              debug_dir, data_scrdump_filename, current_step_num):
+def barrier_2(vm, words, params, debug_dir, data_scrdump_filename,
+              current_step_num):
     if len(words) < 7:
         logging.error("Bad barrier_2 command line")
         return False
@@ -41,6 +41,18 @@ def barrier_2(vm, words, fail_if_stuck_for, stuck_detection_history,
                                                  "cropped_scrdump_expected.ppm")
     comparison_filename = os.path.join(debug_dir, "comparison.ppm")
 
+    fail_if_stuck_for = params.get("fail_if_stuck_for")
+    if fail_if_stuck_for:
+        fail_if_stuck_for = float(fail_if_stuck_for)
+    else:
+        fail_if_stuck_for = 1e308
+
+    stuck_detection_history = params.get("stuck_detection_history")
+    if stuck_detection_history:
+        stuck_detection_history = int(stuck_detection_history)
+    else:
+        stuck_detection_history = 2
+
     end_time = time.time() + timeout
     end_time_stuck = time.time() + fail_if_stuck_for
     start_time = time.time()
@@ -151,18 +163,6 @@ def run_steps(test, params, env):
     if not os.path.exists(steps_filename):
         raise error.TestError("Steps file not found: %s" % steps_filename)
 
-    fail_if_stuck_for = params.get("fail_if_stuck_for")
-    if fail_if_stuck_for:
-        fail_if_stuck_for = float(fail_if_stuck_for)
-    else:
-        fail_if_stuck_for = 1e308
-
-    stuck_detection_history = params.get("stuck_detection_history")
-    if stuck_detection_history:
-        stuck_detection_history = int(stuck_detection_history)
-    else:
-        stuck_detection_history = 2
-
     sf = open(steps_filename, "r")
     lines = sf.readlines()
     sf.close()
@@ -201,13 +201,12 @@ def run_steps(test, params, env):
                 logging.error("Variable not defined: %s" % words[1])
         elif words[0] == "barrier_2":
             if current_screendump:
-                scrdump_filename = (
-                os.path.join(ppm_utils.get_data_dir(steps_filename),
-                             current_screendump))
+                scrdump_filename = os.path.join(
+                    ppm_utils.get_data_dir(steps_filename),
+                    current_screendump)
             else:
                 scrdump_filename = None
-            if not barrier_2(vm, words, fail_if_stuck_for,
-                             stuck_detection_history, test.debugdir,
+            if not barrier_2(vm, words, params, test.debugdir,
                              scrdump_filename, current_step_num):
                 skip_current_step = True
         else:
-- 
1.5.4.1

Re: [KVM-AUTOTEST PATCH 2/4] kvm_guest_wizard: pass 'params' directly to barrier_2()

[Uri Lublin]

On 06/15/2009 10:45 PM, Michael Goldish wrote:
> Currently parameters for barrier_2() are extracted from 'params' in the main
> run_steps() test routine, and then passed to barrier_2().
> Instead, let barrier_2() extract parameters from 'params' as it sees fit.
> This will make adding new parameters slightly easier and cleaner.
>
> Signed-off-by: Michael Goldish<mgoldish@redhat.com>
> ---
>   client/tests/kvm/kvm_guest_wizard.py |   37 ++++++++++++++++-----------------
>   1 files changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/client/tests/kvm/kvm_guest_wizard.py b/client/tests/kvm/kvm_guest_wizard.py
> index 143e61e..eb0e2d5 100644
> --- a/client/tests/kvm/kvm_guest_wizard.py
> +++ b/client/tests/kvm/kvm_guest_wizard.py


> @@ -41,6 +41,18 @@ def barrier_2(vm, words, fail_if_stuck_for, stuck_detection_history,
>                                                    "cropped_scrdump_expected.ppm")
>       comparison_filename = os.path.join(debug_dir, "comparison.ppm")
>
> +    fail_if_stuck_for = params.get("fail_if_stuck_for")
> +    if fail_if_stuck_for:
> +        fail_if_stuck_for = float(fail_if_stuck_for)
> +    else:
> +        fail_if_stuck_for = 1e308
> +
> +    stuck_detection_history = params.get("stuck_detection_history")
> +    if stuck_detection_history:
> +        stuck_detection_history = int(stuck_detection_history)
> +    else:
> +        stuck_detection_history = 2
> +

Could be simplified by (preferably in a separate patch):
        fail_if_stuck_for = params.get("fail_if_stuck_for", 1e308)
        fail_if_stuck_for = float(fail_if_stuck_for)

        stuck_detection_history = params.get("stuck_detection_history", 2)
        stuck_detection_history = int(stuck_detection_history)

or even with a single line for each param:
        var = cast(params.get(var_name, default_value))


Regards,
     Uri

[KVM-AUTOTEST PATCH 3/4] kvm_guest_wizard: allow keeping screendump history for debugging purposes

[Michael Goldish]

Add two new step file test parameters:
- keep_screendump_history: if equals 'yes', screendump history is saved in
  test.debugdir/barrier_history in JPG format.  Each screendump taken by the
  test is saved if it differs from the previous screendump.  By default, when
  a barrier succeeds all history is removed, so eventually the remaining files
  are only those of the (last) failed barrier, if any.
- keep_all_history: if equals 'yes', screendump history is not removed upon
  barrier success.  The test leaves behind all the collected screendump history.

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_guest_wizard.py |   38 ++++++++++++++++++++++++++++-----
 1 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/client/tests/kvm/kvm_guest_wizard.py b/client/tests/kvm/kvm_guest_wizard.py
index eb0e2d5..73b830e 100644
--- a/client/tests/kvm/kvm_guest_wizard.py
+++ b/client/tests/kvm/kvm_guest_wizard.py
@@ -1,6 +1,6 @@
 import os, time, md5, re, shutil, logging
 from autotest_lib.client.common_lib import utils, error
-import kvm_utils, ppm_utils
+import kvm_utils, ppm_utils, kvm_subprocess
 
 """
 Utilities to perform automatic guest installation using step files.
@@ -53,6 +53,11 @@ def barrier_2(vm, words, params, debug_dir, data_scrdump_filename,
     else:
         stuck_detection_history = 2
 
+    keep_screendump_history = params.get("keep_screendump_history") == "yes"
+    if keep_screendump_history:
+        keep_all_history = params.get("keep_all_history") == "yes"
+        history_dir = os.path.join(debug_dir, "barrier_history")
+
     end_time = time.time() + timeout
     end_time_stuck = time.time() + fail_if_stuck_for
     start_time = time.time()
@@ -91,21 +96,42 @@ def barrier_2(vm, words, params, debug_dir, data_scrdump_filename,
         # Read image file
         (w, h, data) = ppm_utils.image_read_from_ppm_file(scrdump_filename)
 
+        # Compute md5sum of whole image
+        whole_image_md5sum = ppm_utils.image_md5sum(w, h, data)
+
+        # Write screendump to history_dir (as JPG) if requested
+        # and if the screendump differs from the previous one
+        if (keep_screendump_history and
+            whole_image_md5sum not in prev_whole_image_md5sums[:1]):
+            try:
+                os.makedirs(history_dir)
+            except:
+                pass
+            history_scrdump_filename = os.path.join(history_dir,
+                    "scrdump-step_%s-%s.jpg" % (current_step_num,
+                                                time.strftime("%Y%m%d-%H%M%S")))
+            kvm_subprocess.run_fg("convert -quality 30 %s %s" %
+                                  (scrdump_filename, history_scrdump_filename),
+                                  logging.debug, "(convert) ", timeout=30)
+
         # Compare md5sum of barrier region with the expected md5sum
         calced_md5sum = ppm_utils.get_region_md5sum(w, h, data, x1, y1, dx, dy,
                                                     cropped_scrdump_filename)
         if calced_md5sum == md5sum:
+            # Success -- remove screendump history unless requested not to
+            if keep_screendump_history and not keep_all_history:
+                kvm_subprocess.run_fg("rm -rvf %s" % history_dir,
+                                      logging.debug, "(rm) ", timeout=30)
+            # Report success
             return True
 
-        # Compute md5sum of whole image in order to compare it with
-        # previous ones
-        whole_image_md5sum = ppm_utils.image_md5sum(w, h, data)
+        # Insert image md5sum into queue of last seen images:
         # If md5sum is already in queue...
         if whole_image_md5sum in prev_whole_image_md5sums:
             # Remove md5sum from queue
             prev_whole_image_md5sums.remove(whole_image_md5sum)
         else:
-            # Extend 'stuck' timeout
+            # Otherwise extend 'stuck' timeout
             end_time_stuck = time.time() + fail_if_stuck_for
         # Insert md5sum at beginning of queue
         prev_whole_image_md5sums.insert(0, whole_image_md5sum)
@@ -129,7 +155,7 @@ def barrier_2(vm, words, params, debug_dir, data_scrdump_filename,
         if data_scrdump_filename and os.path.exists(data_scrdump_filename):
             # Read expected screendump image
             (ew, eh, edata) = \
-            ppm_utils.image_read_from_ppm_file(data_scrdump_filename)
+                    ppm_utils.image_read_from_ppm_file(data_scrdump_filename)
             # Write it in debug_dir
             ppm_utils.image_write_to_ppm_file(expected_scrdump_filename,
                                               ew, eh, edata)
-- 
1.5.4.1

Re: [Autotest] [KVM-AUTOTEST PATCH 3/4] kvm_guest_wizard: allow keeping screendump history for debugging purposes

[Lucas Meneghel Rodrigues]

On Mon, 2009-06-15 at 22:45 +0300, Michael Goldish wrote:
> Add two new step file test parameters:
> - keep_screendump_history: if equals 'yes', screendump history is saved in
>   test.debugdir/barrier_history in JPG format.  Each screendump taken by the
>   test is saved if it differs from the previous screendump.  By default, when
>   a barrier succeeds all history is removed, so eventually the remaining files
>   are only those of the (last) failed barrier, if any.
> - keep_all_history: if equals 'yes', screendump history is not removed upon
>   barrier success.  The test leaves behind all the collected screendump history.
> 
> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
>  client/tests/kvm/kvm_guest_wizard.py |   38 ++++++++++++++++++++++++++++-----
>  1 files changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/client/tests/kvm/kvm_guest_wizard.py b/client/tests/kvm/kvm_guest_wizard.py
> index eb0e2d5..73b830e 100644
> --- a/client/tests/kvm/kvm_guest_wizard.py
> +++ b/client/tests/kvm/kvm_guest_wizard.py
> @@ -1,6 +1,6 @@
>  import os, time, md5, re, shutil, logging
>  from autotest_lib.client.common_lib import utils, error
> -import kvm_utils, ppm_utils
> +import kvm_utils, ppm_utils, kvm_subprocess

I see, it depends on kvm subprocess, so I will apply this patch after
the kvm subprocess patch is applied.

>  """
>  Utilities to perform automatic guest installation using step files.
> @@ -53,6 +53,11 @@ def barrier_2(vm, words, params, debug_dir, data_scrdump_filename,
>      else:
>          stuck_detection_history = 2
>  
> +    keep_screendump_history = params.get("keep_screendump_history") == "yes"
> +    if keep_screendump_history:
> +        keep_all_history = params.get("keep_all_history") == "yes"
> +        history_dir = os.path.join(debug_dir, "barrier_history")
> +

Here I also think that we should check for the presence of ImageMagick
to avoid failing the test out of necessity.

>      end_time = time.time() + timeout
>      end_time_stuck = time.time() + fail_if_stuck_for
>      start_time = time.time()
> @@ -91,21 +96,42 @@ def barrier_2(vm, words, params, debug_dir, data_scrdump_filename,
>          # Read image file
>          (w, h, data) = ppm_utils.image_read_from_ppm_file(scrdump_filename)
>  
> +        # Compute md5sum of whole image
> +        whole_image_md5sum = ppm_utils.image_md5sum(w, h, data)
> +
> +        # Write screendump to history_dir (as JPG) if requested
> +        # and if the screendump differs from the previous one
> +        if (keep_screendump_history and
> +            whole_image_md5sum not in prev_whole_image_md5sums[:1]):
> +            try:
> +                os.makedirs(history_dir)
> +            except:
> +                pass
> +            history_scrdump_filename = os.path.join(history_dir,
> +                    "scrdump-step_%s-%s.jpg" % (current_step_num,
> +                                                time.strftime("%Y%m%d-%H%M%S")))
> +            kvm_subprocess.run_fg("convert -quality 30 %s %s" %
> +                                  (scrdump_filename, history_scrdump_filename),
> +                                  logging.debug, "(convert) ", timeout=30)
> +
>          # Compare md5sum of barrier region with the expected md5sum
>          calced_md5sum = ppm_utils.get_region_md5sum(w, h, data, x1, y1, dx, dy,
>                                                      cropped_scrdump_filename)
>          if calced_md5sum == md5sum:
> +            # Success -- remove screendump history unless requested not to
> +            if keep_screendump_history and not keep_all_history:
> +                kvm_subprocess.run_fg("rm -rvf %s" % history_dir,
> +                                      logging.debug, "(rm) ", timeout=30)
> +            # Report success
>              return True
>  
> -        # Compute md5sum of whole image in order to compare it with
> -        # previous ones
> -        whole_image_md5sum = ppm_utils.image_md5sum(w, h, data)
> +        # Insert image md5sum into queue of last seen images:
>          # If md5sum is already in queue...
>          if whole_image_md5sum in prev_whole_image_md5sums:
>              # Remove md5sum from queue
>              prev_whole_image_md5sums.remove(whole_image_md5sum)
>          else:
> -            # Extend 'stuck' timeout
> +            # Otherwise extend 'stuck' timeout
>              end_time_stuck = time.time() + fail_if_stuck_for
>          # Insert md5sum at beginning of queue
>          prev_whole_image_md5sums.insert(0, whole_image_md5sum)
> @@ -129,7 +155,7 @@ def barrier_2(vm, words, params, debug_dir, data_scrdump_filename,
>          if data_scrdump_filename and os.path.exists(data_scrdump_filename):
>              # Read expected screendump image
>              (ew, eh, edata) = \
> -            ppm_utils.image_read_from_ppm_file(data_scrdump_filename)
> +                    ppm_utils.image_read_from_ppm_file(data_scrdump_filename)
>              # Write it in debug_dir
>              ppm_utils.image_write_to_ppm_file(expected_scrdump_filename,
>                                                ew, eh, edata)
-- 
Lucas Meneghel Rodrigues
Software Engineer (QE)
Red Hat - Emerging Technologies

Re: [Autotest] [KVM-AUTOTEST PATCH 3/4] kvm_guest_wizard: allow keeping screendump history for debugging purposes

[Michael Goldish]

----- "Lucas Meneghel Rodrigues" <lmr@redhat.com> wrote:

> On Mon, 2009-06-15 at 22:45 +0300, Michael Goldish wrote:
> > Add two new step file test parameters:
> > - keep_screendump_history: if equals 'yes', screendump history is
> saved in
> >   test.debugdir/barrier_history in JPG format.  Each screendump
> taken by the
> >   test is saved if it differs from the previous screendump.  By
> default, when
> >   a barrier succeeds all history is removed, so eventually the
> remaining files
> >   are only those of the (last) failed barrier, if any.
> > - keep_all_history: if equals 'yes', screendump history is not
> removed upon
> >   barrier success.  The test leaves behind all the collected
> screendump history.
> > 
> > Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> > ---
> >  client/tests/kvm/kvm_guest_wizard.py |   38
> ++++++++++++++++++++++++++++-----
> >  1 files changed, 32 insertions(+), 6 deletions(-)
> > 
> > diff --git a/client/tests/kvm/kvm_guest_wizard.py
> b/client/tests/kvm/kvm_guest_wizard.py
> > index eb0e2d5..73b830e 100644
> > --- a/client/tests/kvm/kvm_guest_wizard.py
> > +++ b/client/tests/kvm/kvm_guest_wizard.py
> > @@ -1,6 +1,6 @@
> >  import os, time, md5, re, shutil, logging
> >  from autotest_lib.client.common_lib import utils, error
> > -import kvm_utils, ppm_utils
> > +import kvm_utils, ppm_utils, kvm_subprocess
> 
> I see, it depends on kvm subprocess, so I will apply this patch after
> the kvm subprocess patch is applied.
> 
> >  """
> >  Utilities to perform automatic guest installation using step
> files.
> > @@ -53,6 +53,11 @@ def barrier_2(vm, words, params, debug_dir,
> data_scrdump_filename,
> >      else:
> >          stuck_detection_history = 2
> >  
> > +    keep_screendump_history = params.get("keep_screendump_history")
> == "yes"
> > +    if keep_screendump_history:
> > +        keep_all_history = params.get("keep_all_history") == "yes"
> > +        history_dir = os.path.join(debug_dir, "barrier_history")
> > +
> 
> Here I also think that we should check for the presence of
> ImageMagick to avoid failing the test out of necessity.

Again, the test won't fail as long as kvm_subprocess doesn't raise exceptions.
It'll just display bash's "command not found" response in debug messages with
a "(convert) " prefix.

> >      end_time = time.time() + timeout
> >      end_time_stuck = time.time() + fail_if_stuck_for
> >      start_time = time.time()
> > @@ -91,21 +96,42 @@ def barrier_2(vm, words, params, debug_dir,
> data_scrdump_filename,
> >          # Read image file
> >          (w, h, data) =
> ppm_utils.image_read_from_ppm_file(scrdump_filename)
> >  
> > +        # Compute md5sum of whole image
> > +        whole_image_md5sum = ppm_utils.image_md5sum(w, h, data)
> > +
> > +        # Write screendump to history_dir (as JPG) if requested
> > +        # and if the screendump differs from the previous one
> > +        if (keep_screendump_history and
> > +            whole_image_md5sum not in
> prev_whole_image_md5sums[:1]):
> > +            try:
> > +                os.makedirs(history_dir)
> > +            except:
> > +                pass
> > +            history_scrdump_filename = os.path.join(history_dir,
> > +                    "scrdump-step_%s-%s.jpg" % (current_step_num,
> > +                                               
> time.strftime("%Y%m%d-%H%M%S")))
> > +            kvm_subprocess.run_fg("convert -quality 30 %s %s" %
> > +                                  (scrdump_filename,
> history_scrdump_filename),
> > +                                  logging.debug, "(convert) ",
> timeout=30)
> > +
> >          # Compare md5sum of barrier region with the expected
> md5sum
> >          calced_md5sum = ppm_utils.get_region_md5sum(w, h, data, x1,
> y1, dx, dy,
> >                                                     
> cropped_scrdump_filename)
> >          if calced_md5sum == md5sum:
> > +            # Success -- remove screendump history unless requested
> not to
> > +            if keep_screendump_history and not keep_all_history:
> > +                kvm_subprocess.run_fg("rm -rvf %s" % history_dir,
> > +                                      logging.debug, "(rm) ",
> timeout=30)
> > +            # Report success
> >              return True
> >  
> > -        # Compute md5sum of whole image in order to compare it
> with
> > -        # previous ones
> > -        whole_image_md5sum = ppm_utils.image_md5sum(w, h, data)
> > +        # Insert image md5sum into queue of last seen images:
> >          # If md5sum is already in queue...
> >          if whole_image_md5sum in prev_whole_image_md5sums:
> >              # Remove md5sum from queue
> >              prev_whole_image_md5sums.remove(whole_image_md5sum)
> >          else:
> > -            # Extend 'stuck' timeout
> > +            # Otherwise extend 'stuck' timeout
> >              end_time_stuck = time.time() + fail_if_stuck_for
> >          # Insert md5sum at beginning of queue
> >          prev_whole_image_md5sums.insert(0, whole_image_md5sum)
> > @@ -129,7 +155,7 @@ def barrier_2(vm, words, params, debug_dir,
> data_scrdump_filename,
> >          if data_scrdump_filename and
> os.path.exists(data_scrdump_filename):
> >              # Read expected screendump image
> >              (ew, eh, edata) = \
> > -           
> ppm_utils.image_read_from_ppm_file(data_scrdump_filename)
> > +                   
> ppm_utils.image_read_from_ppm_file(data_scrdump_filename)
> >              # Write it in debug_dir
> >             
> ppm_utils.image_write_to_ppm_file(expected_scrdump_filename,
> >                                                ew, eh, edata)
> -- 
> Lucas Meneghel Rodrigues
> Software Engineer (QE)
> Red Hat - Emerging Technologies

[KVM-AUTOTEST PATCH 4/4] kvm_tests.cfg.sample: add 'keep_screendump_history = yes' to step file tests

[Michael Goldish]

This should be rather harmless because the history does not take up much space,
and is only kept for the failed barriers in failed tests, by default.
It should significantly ease debugging of failed step file tests.

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_tests.cfg.sample |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/client/tests/kvm/kvm_tests.cfg.sample b/client/tests/kvm/kvm_tests.cfg.sample
index c73da7c..54c9975 100644
--- a/client/tests/kvm/kvm_tests.cfg.sample
+++ b/client/tests/kvm/kvm_tests.cfg.sample
@@ -30,6 +30,7 @@ variants:
         type = steps
         fail_if_stuck_for = 300
         stuck_detection_history = 2
+        keep_screendump_history = yes
         force_create_image = yes
         kill_vm = yes
         kill_vm_timeout = 60
@@ -39,6 +40,7 @@ variants:
         type = steps
         fail_if_stuck_for = 300
         stuck_detection_history = 2
+        keep_screendump_history = yes
 
     - boot:         install setup
         type = boot
-- 
1.5.4.1

Re: [Autotest] [KVM-AUTOTEST PATCH 4/4] kvm_tests.cfg.sample: add 'keep_screendump_history = yes' to step file tests

[Lucas Meneghel Rodrigues]

On Mon, 2009-06-15 at 22:45 +0300, Michael Goldish wrote:
> This should be rather harmless because the history does not take up much space,
> and is only kept for the failed barriers in failed tests, by default.
> It should significantly ease debugging of failed step file tests.
> 
> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
>  client/tests/kvm/kvm_tests.cfg.sample |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/client/tests/kvm/kvm_tests.cfg.sample b/client/tests/kvm/kvm_tests.cfg.sample
> index c73da7c..54c9975 100644
> --- a/client/tests/kvm/kvm_tests.cfg.sample
> +++ b/client/tests/kvm/kvm_tests.cfg.sample
> @@ -30,6 +30,7 @@ variants:
>          type = steps
>          fail_if_stuck_for = 300
>          stuck_detection_history = 2
> +        keep_screendump_history = yes
>          force_create_image = yes
>          kill_vm = yes
>          kill_vm_timeout = 60
> @@ -39,6 +40,7 @@ variants:
>          type = steps
>          fail_if_stuck_for = 300
>          stuck_detection_history = 2
> +        keep_screendump_history = yes
>  
>      - boot:         install setup
>          type = boot

Excellent feature indeed.

-- 
Lucas Meneghel Rodrigues
Software Engineer (QE)
Red Hat - Emerging Technologies

Re: [KVM-AUTOTEST PATCH 0/4] Step file tests: introducing a new feature and some small changes

[Lucas Meneghel Rodrigues]

On Mon, 2009-06-15 at 22:45 +0300, Michael Goldish wrote:
> The following patches make some changes to step file tests:
> * Rename parameter output_dir in barrier_2() to debug_dir.
> * Move some code from run_steps() into barrier_2().
> * Add a new feature (keep_screendump_history) which allows the user to see
>   exactly what went wrong with a step file test. Currently only the last
>   screendump is kept for each failed step file test; with this feature enabled,
>   all screendumps are kept in compact JPG format.
> * Use the new feature by default in kvm_tests.cfg.sample.
> 

Other than minor comments, patch set looks good to me. Please let me
know if you agree with my comments.

-- 
Lucas Meneghel Rodrigues
Software Engineer (QE)
Red Hat - Emerging Technologies

Re: [KVM-AUTOTEST PATCH 2/4] kvm_guest_wizard: pass 'params' directly to barrier_2()

[Michael Goldish]

----- "Uri Lublin" <uril@redhat.com> wrote:

> On 06/15/2009 10:45 PM, Michael Goldish wrote:
> > Currently parameters for barrier_2() are extracted from 'params' in
> the main
> > run_steps() test routine, and then passed to barrier_2().
> > Instead, let barrier_2() extract parameters from 'params' as it sees
> fit.
> > This will make adding new parameters slightly easier and cleaner.
> >
> > Signed-off-by: Michael Goldish<mgoldish@redhat.com>
> > ---
> >   client/tests/kvm/kvm_guest_wizard.py |   37
> ++++++++++++++++-----------------
> >   1 files changed, 18 insertions(+), 19 deletions(-)
> >
> > diff --git a/client/tests/kvm/kvm_guest_wizard.py
> b/client/tests/kvm/kvm_guest_wizard.py
> > index 143e61e..eb0e2d5 100644
> > --- a/client/tests/kvm/kvm_guest_wizard.py
> > +++ b/client/tests/kvm/kvm_guest_wizard.py
> 
> 
> > @@ -41,6 +41,18 @@ def barrier_2(vm, words, fail_if_stuck_for,
> stuck_detection_history,
> >                                                   
> "cropped_scrdump_expected.ppm")
> >       comparison_filename = os.path.join(debug_dir,
> "comparison.ppm")
> >
> > +    fail_if_stuck_for = params.get("fail_if_stuck_for")
> > +    if fail_if_stuck_for:
> > +        fail_if_stuck_for = float(fail_if_stuck_for)
> > +    else:
> > +        fail_if_stuck_for = 1e308
> > +
> > +    stuck_detection_history =
> params.get("stuck_detection_history")
> > +    if stuck_detection_history:
> > +        stuck_detection_history = int(stuck_detection_history)
> > +    else:
> > +        stuck_detection_history = 2
> > +
> 
> Could be simplified by (preferably in a separate patch):
>         fail_if_stuck_for = params.get("fail_if_stuck_for", 1e308)
>         fail_if_stuck_for = float(fail_if_stuck_for)
> 
>         stuck_detection_history =
> params.get("stuck_detection_history", 2)
>         stuck_detection_history = int(stuck_detection_history)
> 
> or even with a single line for each param:
>         var = cast(params.get(var_name, default_value))

But what if the user wants to disable the feature or use the default
value by setting fail_if_stuck_for = ''?
This is required, for example, when the param is set to 300 for all
tests, and the user wants to disable it only for Fedora tests.
In that case params.get("fail_if_stuck_for", 1e308) would return ''
and the following cast would raise an exception.