[KVM-AUTOTEST PATCH 0/5] small kvm_config usability changes

[KVM-AUTOTEST PATCH 0/5] small kvm_config usability changes

[Eduardo Habkost]

From: Eduardo Habkost <ehabkost@raisama.net>

This series introduce some changes on kvm_config.py to make it more usable when
running it directly from the command-line.

Eduardo Habkost (5):
  kvm_config: accept multiple filenames as argument
  kvm_config: print directly to stdout instead of using logging
  kvm_config: store filename on configreader
  kvm_config: add helper to raise exception informing line number
  kvm_config: inform filename and line number on error message

 client/tests/kvm/kvm_config.py |   40 +++++++++++++++++++++++++++++-----------
 1 files changed, 29 insertions(+), 11 deletions(-)

-- 
1.7.3.2

[KVM-AUTOTEST PATCH 1/5] kvm_config: accept multiple filenames as argument

[Eduardo Habkost]

From: Eduardo Habkost <ehabkost@raisama.net>

Useful to test and debug cases where config settings are concatenated together,
without the need to change the base .cfg file.

Signed-off-by: Eduardo Habkost <ehabkost@raisama.net>
---
 client/tests/kvm/kvm_config.py |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/client/tests/kvm/kvm_config.py b/client/tests/kvm/kvm_config.py
index 4fc1029..45d8fe6 100755
--- a/client/tests/kvm/kvm_config.py
+++ b/client/tests/kvm/kvm_config.py
@@ -682,15 +682,18 @@ if __name__ == "__main__":
     options, args = parser.parse_args()
     debug = options.debug
     if args:
-        filename = args[0]
+        filenames = args
     else:
-        filename = os.path.join(os.path.dirname(sys.argv[0]), "tests.cfg")
+        filenames = [os.path.join(os.path.dirname(sys.argv[0]), "tests.cfg")]
 
     # Here we configure the stand alone program to use the autotest
     # logging system.
     logging_manager.configure_logging(kvm_utils.KvmLoggingConfig(),
                                       verbose=debug)
-    dicts = config(filename, debug=debug).get_generator()
+    cfg = config(debug=debug)
+    for fn in filenames:
+        cfg.parse_file(fn)
+    dicts = cfg.get_generator()
     for i, dict in enumerate(dicts):
         logging.info("Dictionary #%d:", i)
         keys = dict.keys()
-- 
1.7.3.2

[KVM-AUTOTEST PATCH 2/5] kvm_config: print directly to stdout instead of using logging

[Eduardo Habkost]

From: Eduardo Habkost <ehabkost@raisama.net>

If the whole purpose of running kvm_config.py directly is to print the
dictionary contents, it is better to simply dump the information to
stdout instead of adding the logginging info and timestamp clutter to
every single line.

Signed-off-by: Eduardo Habkost <ehabkost@raisama.net>
---
 client/tests/kvm/kvm_config.py |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/client/tests/kvm/kvm_config.py b/client/tests/kvm/kvm_config.py
index 45d8fe6..5be2e66 100755
--- a/client/tests/kvm/kvm_config.py
+++ b/client/tests/kvm/kvm_config.py
@@ -695,8 +695,8 @@ if __name__ == "__main__":
         cfg.parse_file(fn)
     dicts = cfg.get_generator()
     for i, dict in enumerate(dicts):
-        logging.info("Dictionary #%d:", i)
+        print "Dictionary #%d:" % (i)
         keys = dict.keys()
         keys.sort()
         for key in keys:
-            logging.info("    %s = %s", key, dict[key])
+            print "    %s = %s" % (key, dict[key])
-- 
1.7.3.2

[KVM-AUTOTEST PATCH 3/5] kvm_config: store filename on configreader

[Eduardo Habkost]

From: Eduardo Habkost <ehabkost@raisama.net>

It will be useful to generate better error messages.

Signed-off-by: Eduardo Habkost <ehabkost@raisama.net>
---
 client/tests/kvm/kvm_config.py |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/client/tests/kvm/kvm_config.py b/client/tests/kvm/kvm_config.py
index 5be2e66..35e2ab9 100755
--- a/client/tests/kvm/kvm_config.py
+++ b/client/tests/kvm/kvm_config.py
@@ -48,7 +48,7 @@ class config:
             raise IOError("File %s not found" % filename)
         self.filename = filename
         str = open(filename).read()
-        self.list = self.parse(configreader(str), self.list)
+        self.list = self.parse(configreader(filename, str), self.list)
 
 
     def parse_string(self, str):
@@ -57,7 +57,7 @@ class config:
 
         @param str: String to parse.
         """
-        self.list = self.parse(configreader(str), self.list)
+        self.list = self.parse(configreader('<string>', str), self.list)
 
 
     def fork_and_parse(self, filename=None, str=None):
@@ -342,7 +342,7 @@ class config:
                                             words[1])
                     if os.path.exists(filename):
                         str = open(filename).read()
-                        list = self.parse(configreader(str), list, restricted)
+                        list = self.parse(configreader(filename, str), list, restricted)
                         if self.debug and not restricted:
                             _debug_print("", "Leaving file %s" % words[1])
                     else:
@@ -539,12 +539,13 @@ class configreader:
     whose readline() and/or seek() methods seem to be slow.
     """
 
-    def __init__(self, str):
+    def __init__(self, filename, str):
         """
         Initialize the reader.
 
         @param str: The string to parse.
         """
+        self.filename = filename
         self.line_index = 0
         self.lines = []
         for line in str.splitlines():
-- 
1.7.3.2

[KVM-AUTOTEST PATCH 4/5] kvm_config: add helper to raise exception informing line number

[Eduardo Habkost]

From: Eduardo Habkost <ehabkost@raisama.net>

Useful for syntax or other errors on the config file. We want to tell
the user on which file:line the error is located.

Signed-off-by: Eduardo Habkost <ehabkost@raisama.net>
---
 client/tests/kvm/kvm_config.py |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/client/tests/kvm/kvm_config.py b/client/tests/kvm/kvm_config.py
index 35e2ab9..c206743 100755
--- a/client/tests/kvm/kvm_config.py
+++ b/client/tests/kvm/kvm_config.py
@@ -548,7 +548,8 @@ class configreader:
         self.filename = filename
         self.line_index = 0
         self.lines = []
-        for line in str.splitlines():
+        self.real_number = []
+        for num,line in enumerate(str.splitlines(), 1):
             line = line.rstrip().expandtabs()
             stripped_line = line.strip()
             indent = len(line) - len(stripped_line)
@@ -557,6 +558,7 @@ class configreader:
                 or stripped_line.startswith("//")):
                 continue
             self.lines.append((line, stripped_line, indent))
+            self.real_number.append(num)
 
 
     def get_next_line(self):
@@ -589,6 +591,18 @@ class configreader:
         """
         self.line_index = index
 
+    def raise_error(self, msg):
+        """Raise an error related to the last line returned by get_next_line()
+        """
+        if self.line_index == 0: # nothing was read. shouldn't happen, but...
+            line_id = 'BEGIN'
+        elif self.line_index >= len(self.lines): # past EOF
+            line_id = 'EOF'
+        else:
+            # line_index is the _next_ line. get the previous one
+            line_id = str(self.real_number[self.line_index-1])
+        raise error.AutotestError("%s:%s: %s" % (self.filename, line_id, msg))
+
 
 # Array structure:
 # ----------------
-- 
1.7.3.2

Re: [KVM-AUTOTEST PATCH 4/5] kvm_config: add helper to raise exception informing line number

[Lucas Meneghel Rodrigues]

On Thu, 2011-01-06 at 14:12 -0200, Eduardo Habkost wrote:
> From: Eduardo Habkost <ehabkost@raisama.net>
> 
> Useful for syntax or other errors on the config file. We want to tell
> the user on which file:line the error is located.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@raisama.net>
> ---
>  client/tests/kvm/kvm_config.py |   16 +++++++++++++++-
>  1 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/client/tests/kvm/kvm_config.py b/client/tests/kvm/kvm_config.py
> index 35e2ab9..c206743 100755
> --- a/client/tests/kvm/kvm_config.py
> +++ b/client/tests/kvm/kvm_config.py
> @@ -548,7 +548,8 @@ class configreader:
>          self.filename = filename
>          self.line_index = 0
>          self.lines = []
> -        for line in str.splitlines():
> +        self.real_number = []
> +        for num,line in enumerate(str.splitlines(), 1):

^ enumerate in py 2.4 takes exactly 1 argument, so it's not possible to
provide the enumerate start index like this, 

http://docs.python.org/library/functions.html#enumerate

so we have to do something like:

+        sequence = str.splitlines()[1:]
+        for num,line in enumerate(sequence):

To make it py2.4 compliant.

I have read your patchset, looks good to me. Before applying it, I am
going to give Michael time so he can read the patches as well.

>              line = line.rstrip().expandtabs()
>              stripped_line = line.strip()
>              indent = len(line) - len(stripped_line)
> @@ -557,6 +558,7 @@ class configreader:
>                  or stripped_line.startswith("//")):
>                  continue
>              self.lines.append((line, stripped_line, indent))
> +            self.real_number.append(num)
>  
> 
>      def get_next_line(self):
> @@ -589,6 +591,18 @@ class configreader:
>          """
>          self.line_index = index
>  
> +    def raise_error(self, msg):
> +        """Raise an error related to the last line returned by get_next_line()
> +        """
> +        if self.line_index == 0: # nothing was read. shouldn't happen, but...
> +            line_id = 'BEGIN'
> +        elif self.line_index >= len(self.lines): # past EOF
> +            line_id = 'EOF'
> +        else:
> +            # line_index is the _next_ line. get the previous one
> +            line_id = str(self.real_number[self.line_index-1])
> +        raise error.AutotestError("%s:%s: %s" % (self.filename, line_id, msg))
> +
>  
>  # Array structure:
>  # ----------------

Re: [KVM-AUTOTEST PATCH 4/5] kvm_config: add helper to raise exception informing line number

[Eduardo Habkost]

On Tue, Jan 11, 2011 at 01:48:22AM -0200, Lucas Meneghel Rodrigues wrote:
> On Thu, 2011-01-06 at 14:12 -0200, Eduardo Habkost wrote:
> > +        for num,line in enumerate(str.splitlines(), 1):
> 
> ^ enumerate in py 2.4 takes exactly 1 argument, so it's not possible to
> provide the enumerate start index like this, 
> 
> http://docs.python.org/library/functions.html#enumerate

Oops! Right.


> 
> so we have to do something like:
> 
> +        sequence = str.splitlines()[1:]
> +        for num,line in enumerate(sequence):
> 
> To make it py2.4 compliant.

No, that's not what the 'start' parameter does. You don't want to skip the
first line from the input.

However, we can simply do the following:

    diff --git a/client/tests/kvm/kvm_config.py b/client/tests/kvm/kvm_config.py
    index c206743..5acf247 100755
    --- a/client/tests/kvm/kvm_config.py
    +++ b/client/tests/kvm/kvm_config.py
    @@ -549,7 +549,7 @@ class configreader:
             self.line_index = 0
             self.lines = []
             self.real_number = []
    -        for num,line in enumerate(str.splitlines(), 1):
    +        for num,line in enumerate(str.splitlines()):
                 line = line.rstrip().expandtabs()
                 stripped_line = line.strip()
                 indent = len(line) - len(stripped_line)
    @@ -558,7 +558,7 @@ class configreader:
                     or stripped_line.startswith("//")):
                     continue
                 self.lines.append((line, stripped_line, indent))
    -            self.real_number.append(num)
    +            self.real_number.append(num+1)
     
     
         def get_next_line(self):

Updated patch below.

--------------------
>From 09a1c35fa79127767de3c54dd02890658498e204 Mon Sep 17 00:00:00 2001
From: Eduardo Habkost <ehabkost@raisama.net>
Date: Wed, 5 Jan 2011 17:34:48 -0200
Subject: [PATCH] kvm_config: add helper to raise exception informing line number

Useful for syntax or other errors on the config file. We want to tell
the user on which file:line the error is located.

Signed-off-by: Eduardo Habkost <ehabkost@raisama.net>
---
 client/tests/kvm/kvm_config.py |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/client/tests/kvm/kvm_config.py b/client/tests/kvm/kvm_config.py
index 35e2ab9..5acf247 100755
--- a/client/tests/kvm/kvm_config.py
+++ b/client/tests/kvm/kvm_config.py
@@ -548,7 +548,8 @@ class configreader:
         self.filename = filename
         self.line_index = 0
         self.lines = []
-        for line in str.splitlines():
+        self.real_number = []
+        for num,line in enumerate(str.splitlines()):
             line = line.rstrip().expandtabs()
             stripped_line = line.strip()
             indent = len(line) - len(stripped_line)
@@ -557,6 +558,7 @@ class configreader:
                 or stripped_line.startswith("//")):
                 continue
             self.lines.append((line, stripped_line, indent))
+            self.real_number.append(num+1)
 
 
     def get_next_line(self):
@@ -589,6 +591,18 @@ class configreader:
         """
         self.line_index = index
 
+    def raise_error(self, msg):
+        """Raise an error related to the last line returned by get_next_line()
+        """
+        if self.line_index == 0: # nothing was read. shouldn't happen, but...
+            line_id = 'BEGIN'
+        elif self.line_index >= len(self.lines): # past EOF
+            line_id = 'EOF'
+        else:
+            # line_index is the _next_ line. get the previous one
+            line_id = str(self.real_number[self.line_index-1])
+        raise error.AutotestError("%s:%s: %s" % (self.filename, line_id, msg))
+
 
 # Array structure:
 # ----------------
-- 
1.7.3.2

-- 
Eduardo

[KVM-AUTOTEST PATCH 5/5] kvm_config: inform filename and line number on error message

[Eduardo Habkost]

From: Eduardo Habkost <ehabkost@raisama.net>

Include the filename and line number on the "Using variants in this
context is not allowed" exception error message.

Signed-off-by: Eduardo Habkost <ehabkost@raisama.net>
---
 client/tests/kvm/kvm_config.py |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/client/tests/kvm/kvm_config.py b/client/tests/kvm/kvm_config.py
index c206743..c4d9a01 100755
--- a/client/tests/kvm/kvm_config.py
+++ b/client/tests/kvm/kvm_config.py
@@ -297,7 +297,7 @@ class config:
                 # (inside an exception or inside subvariants)
                 if restricted:
                     e_msg = "Using variants in this context is not allowed"
-                    raise error.AutotestError(e_msg)
+                    cr.raise_error(e_msg)
                 if self.debug and not restricted:
                     _debug_print(indented_line,
                                  "Entering variants block (%d dicts in "
-- 
1.7.3.2