BIND 10 trac1290, updated. 043ff1e7ec5f2c8e3d6b7e278418fc03eea2b09f [1290] address review comments

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Nov 3 15:53:26 UTC 2011


The branch, trac1290 has been updated
       via  043ff1e7ec5f2c8e3d6b7e278418fc03eea2b09f (commit)
      from  46adf014f18c6b3f9a685b8f0fdd0775a583a7c5 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 043ff1e7ec5f2c8e3d6b7e278418fc03eea2b09f
Author: Jelte Jansen <jelte at isc.org>
Date:   Thu Nov 3 16:53:15 2011 +0100

    [1290] address review comments

-----------------------------------------------------------------------

Summary of changes:
 src/bin/bindctl/bindcmd.py                         |    2 +-
 tests/lettuce/README                               |    5 +
 tests/lettuce/README.tutorial                      |   28 +++-
 .../lettuce/configurations/example.org.config.orig |   18 ++-
 tests/lettuce/configurations/example2.org.config   |   19 ++-
 tests/lettuce/configurations/no_db_file.config     |   11 ++-
 tests/lettuce/features/example.feature             |    4 +-
 tests/lettuce/features/terrain/bind10_control.py   |   68 +++++++--
 tests/lettuce/features/terrain/querying.py         |  118 +++++++++++++-
 tests/lettuce/features/terrain/steps.py            |   47 ++++++
 tests/lettuce/features/terrain/terrain.py          |  172 ++++++++++++++++++--
 11 files changed, 448 insertions(+), 44 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/bindctl/bindcmd.py b/src/bin/bindctl/bindcmd.py
index 3854f16..5b48d4e 100644
--- a/src/bin/bindctl/bindcmd.py
+++ b/src/bin/bindctl/bindcmd.py
@@ -123,7 +123,7 @@ class BindCmdInterpreter(Cmd):
         '''Parse commands from user and send them to cmdctl. '''
         try:
             if not self.login_to_cmdctl():
-                return
+                return 1
 
             self.cmdloop()
             print('\nExit from bindctl')
diff --git a/tests/lettuce/README b/tests/lettuce/README
index 00ea623..7bee4f2 100644
--- a/tests/lettuce/README
+++ b/tests/lettuce/README
@@ -112,6 +112,11 @@ Some very general steps are defined in terrain/steps.py.
 Initialization code, cleanup code, and helper classes are defined in
 terrain/terrain.py.
 
+To find the right steps, case insensitive matching is used. Parameters taken
+from the steps are case-sensitive though. So a step defined as
+'do foo with value (bar)' will be matched when using
+'Do Foo with value xyz', but xyz will be taken as given.
+
 If you need to add steps that are very particular to one test, create a new 
 file with a name relevant for that test in terrain. We may want to consider 
 creating a specific subdirectory for these, but at this moment it is unclear 
diff --git a/tests/lettuce/README.tutorial b/tests/lettuce/README.tutorial
index eca2cd0..2f138cf 100644
--- a/tests/lettuce/README.tutorial
+++ b/tests/lettuce/README.tutorial
@@ -114,17 +114,27 @@ Feature: showing off BIND 10
 
 So take a look at one of those steps, let's pick the first one.
 
-A step is defined through a python decorator, which in essence is a
-regular expression; each captured group will be passed as an argument
-to the function we define. For bind10, i defined a configuration file,
-a cmdctl port, and a process name. The first two should be
-self-evident, and the process name is an optional name we give it,
-should we want to address it in the rest of the tests. This is most
-useful if we want to start multiple instances. In the next step (the
-wait for auth to start), I added a 'of <instance>'. So if we define
-the bind10 'as my_bind10', we can specify that one here as 'of my
+A step is defined through a python decorator, which in essence is a regular
+expression; lettuce searches through all defined steps to find one that
+matches. These are 'partial' matches (unless specified otherwise in the
+regular expression itself), so if the step is defined with "do foo bar", the
+scenario can add words for readability "When I do foo bar".
+
+Each captured group will be passed as an argument to the function we define.
+For bind10, i defined a configuration file, a cmdctl port, and a process
+name. The first two should be self-evident, and the process name is an
+optional name we give it, should we want to address it in the rest of the
+tests. This is most useful if we want to start multiple instances. In the
+next step (the wait for auth to start), I added a 'of <instance>'. So if we 
+define the bind10 'as my_bind10', we can specify that one here as 'of my
 bind10'.
 
+--
+        When I start bind10 with configuration second.config
+        with cmdctl port 12345 as b10_second_instance
+--
+(line wrapped for readability)
+
 But notice how we needed two steps, which we probably always need (but
 not entirely always)? We can also combine steps; for instance:
 
diff --git a/tests/lettuce/configurations/example.org.config.orig b/tests/lettuce/configurations/example.org.config.orig
index ec60c4b..642f2dd 100644
--- a/tests/lettuce/configurations/example.org.config.orig
+++ b/tests/lettuce/configurations/example.org.config.orig
@@ -1 +1,17 @@
-{"version": 2, "Logging": {"loggers": [{"debuglevel": 99, "severity": "DEBUG", "name": "auth"}]}, "Auth": {"database_file": "data/example.org.sqlite3", "listen_on": [{"port": 47806, "address": "127.0.0.1"}]}}
+{
+    "version": 2,
+    "Logging": {
+        "loggers": [ {
+            "debuglevel": 99,
+            "severity": "DEBUG",
+            "name": "auth"
+        } ]
+    },
+    "Auth": {
+        "database_file": "data/example.org.sqlite3",
+        "listen_on": [ {
+            "port": 47806,
+            "address": "127.0.0.1"
+        } ]
+    }
+}
diff --git a/tests/lettuce/configurations/example2.org.config b/tests/lettuce/configurations/example2.org.config
index 369ca14..1a40d1b 100644
--- a/tests/lettuce/configurations/example2.org.config
+++ b/tests/lettuce/configurations/example2.org.config
@@ -1 +1,18 @@
-{"version": 2, "Logging": {"loggers": [{"severity": "DEBUG", "name": "auth", "debuglevel": 99}]}, "Auth": {"database_file": "data/example.org.sqlite3", "listen_on": [{"port": 47807, "address": "127.0.0.1"}]}}
+{
+    "version": 2,
+    "Logging": {
+        "loggers": [ {
+            "severity": "DEBUG",
+            "name": "auth",
+            "debuglevel": 99
+        }
+        ]
+    },
+    "Auth": {
+        "database_file": "data/example.org.sqlite3",
+        "listen_on": [ {
+            "port": 47807,
+            "address": "127.0.0.1"
+        } ]
+    }
+}
diff --git a/tests/lettuce/configurations/no_db_file.config b/tests/lettuce/configurations/no_db_file.config
index e4fe996..f865354 100644
--- a/tests/lettuce/configurations/no_db_file.config
+++ b/tests/lettuce/configurations/no_db_file.config
@@ -1 +1,10 @@
-{"version": 2, "Auth": {"database_file": "data/test_nonexistent_db.sqlite3", "listen_on": [{"port": 47806, "address": "127.0.0.1"}]}}
+{
+    "version": 2,
+    "Auth": {
+        "database_file": "data/test_nonexistent_db.sqlite3",
+        "listen_on": [ {
+            "port": 47806,
+            "address": "127.0.0.1"
+        } ]
+    }
+}
diff --git a/tests/lettuce/features/example.feature b/tests/lettuce/features/example.feature
index a69ff8e..d1ed6b3 100644
--- a/tests/lettuce/features/example.feature
+++ b/tests/lettuce/features/example.feature
@@ -18,7 +18,7 @@ Feature: Example feature
         # that we are sure this file does not exist, see
         # features/terrain/terrain.py
         
-        # Standard check to test (non-)existance of a file
+        # Standard check to test (non-)existence of a file
         # This file is actually automatically
         The file data/test_nonexistent_db.sqlite3 should not exist
 
@@ -85,6 +85,8 @@ Feature: Example feature
         The last query response should have ancount 0
         The last query response should have nscount 1
         The last query response should have adcount 0
+        # When checking flags, we must pass them exactly as they appear in
+        # the output of dig.
         The last query response should have flags qr aa rd
 
         A query for www.example.org type TXT should have rcode NOERROR
diff --git a/tests/lettuce/features/terrain/bind10_control.py b/tests/lettuce/features/terrain/bind10_control.py
index 000ba69..e104a81 100644
--- a/tests/lettuce/features/terrain/bind10_control.py
+++ b/tests/lettuce/features/terrain/bind10_control.py
@@ -1,15 +1,43 @@
+# Copyright (C) 2011  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
 from lettuce import *
 import subprocess
 import re
 
-def check_lines(output, lines):
-    for line in lines:
-        if output.find(line) != -1:
-            return line
-
 @step('start bind10(?: with configuration (\S+))?' +\
       '(?: with cmdctl port (\d+))?(?: as (\S+))?')
 def start_bind10(step, config_file, cmdctl_port, process_name):
+    """
+    Start BIND 10 with the given optional config file, cmdctl port, and
+    store the running process in world with the given process name.
+    Parameters:
+    config_file ('with configuration <file>', optional): this configuration
+                will be used. The path is relative to the base lettuce
+                directory.
+    cmdctl_port ('with cmdctl port <portnr>', optional): The port on which
+                b10-cmdctl listens for bindctl commands. Defaults to 47805.
+    process_name ('as <name>', optional). This is the name that can be used
+                 in the following steps of the scenario to refer to this
+                 BIND 10 instance. Defaults to 'bind10'.
+    This call will block until BIND10_STARTUP_COMPLETE or BIND10_STARTUP_ERROR
+    is logged. In the case of the latter, or if it times out, the step (and
+    scenario) will fail.
+    It will also fail if there is a running process with the given process_name
+    already.
+    """
     args = [ 'bind10', '-v' ]
     if config_file is not None:
         args.append('-p')
@@ -36,6 +64,12 @@ def start_bind10(step, config_file, cmdctl_port, process_name):
 
 @step('wait for bind10 auth (?:of (\w+) )?to start')
 def wait_for_auth(step, process_name):
+    """Wait for b10-auth to run. This is done by blocking until the message
+       AUTH_SERVER_STARTED is logged.
+       Parameters:
+       process_name ('of <name', optional): The name of the BIND 10 instance
+                    to wait for. Defaults to 'bind10'.
+    """
     if process_name is None:
         process_name = "bind10"
     world.processes.wait_for_stderr_str(process_name, ['AUTH_SERVER_STARTED'],
@@ -43,12 +77,28 @@ def wait_for_auth(step, process_name):
 
 @step('have bind10 running(?: with configuration ([\w.]+))?')
 def have_bind10_running(step, config_file):
+    """
+    Compound convenience step for running bind10, which consists of
+    start_bind10 and wait_for_auth.
+    Currently only supports the 'with configuration' option.
+    """
     step.given('start bind10 with configuration ' + config_file)
     step.given('wait for bind10 auth to start')
 
- at step('set bind10 configuration (\S+) to (.*)')
-def set_config_command(step, name, value):
-    args = ['bindctl', '-p', '47805']
+ at step('set bind10 configuration (\S+) to (.*)(?: with cmdctl port (\d+))?')
+def set_config_command(step, name, value, cmdctl_port):
+    """
+    Run bindctl, set the given configuration to the given value, and commit it.
+    Parameters:
+    name ('configuration <name>'): Identifier of the configuration to set
+    value ('to <value>'): value to set it to.
+    cmdctl_port ('with cmdctl port <portnr>', optional): cmdctl port to send
+                the command to. Defaults to 47805.
+    Fails if cmdctl does not exit with status code 0.
+    """
+    if cmdctl_port is None:
+        cmdctl_port = '47805'
+    args = ['bindctl', '-p', cmdctl_port]
     bindctl = subprocess.Popen(args, 1, None, subprocess.PIPE,
                                subprocess.PIPE, None)
     bindctl.stdin.write("config set " + name + " " + value + "\n")
@@ -56,5 +106,3 @@ def set_config_command(step, name, value):
     bindctl.stdin.write("quit\n")
     result = bindctl.wait()
     assert result == 0, "bindctl exit code: " + str(result)
-
-
diff --git a/tests/lettuce/features/terrain/querying.py b/tests/lettuce/features/terrain/querying.py
index 2818276..ea89b18 100644
--- a/tests/lettuce/features/terrain/querying.py
+++ b/tests/lettuce/features/terrain/querying.py
@@ -1,6 +1,17 @@
-from lettuce import *
-import subprocess
-import re
+# Copyright (C) 2011  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
 # This script provides querying functionality
 # The most important step is
@@ -16,6 +27,10 @@ import re
 #
 # Also see example.feature for some examples
 
+from lettuce import *
+import subprocess
+import re
+
 #
 # define a class to easily access different parts
 # We may consider using our full library for this, but for now
@@ -27,7 +42,8 @@ import re
 # The following attributes are 'parsed' from the response, all as strings,
 # and end up as direct attributes of the QueryResult object:
 # opcode, rcode, id, flags, qdcount, ancount, nscount, adcount
-# (flags is one string with all flags)
+# (flags is one string with all flags, in the order they appear in the
+# response packet.)
 #
 # this will set 'rcode' as the result code, we 'define' one additional
 # rcode, "NO_ANSWER", if the dig process returned an error code itself
@@ -43,7 +59,19 @@ class QueryResult(object):
                           "([0-9]+), AUTHORITY: ([0-9]+), ADDITIONAL: ([0-9]+)")
 
     def __init__(self, name, qtype, qclass, address, port):
-        args = [ 'dig', '+tries=1', '@' + address, '-p', str(port) ]
+        """
+        Constructor. This fires of a query using dig.
+        Parameters:
+        name: The domain name to query
+        qtype: The RR type to query. Defaults to A if it is None.
+        qclass: The RR class to query. Defaults to IN if it is None.
+        address: The IP adress to send the query to.
+        port: The port number to send the query to.
+        All parameters must be either strings or have the correct string
+        representation.
+        Only one query attempt will be made.
+        """
+        args = [ 'dig', '+tries=1', '@' + str(address), '-p', str(port) ]
         if qtype is not None:
             args.append('-t')
             args.append(str(qtype))
@@ -68,8 +96,9 @@ class QueryResult(object):
                 self.line_handler(out)
 
     def _check_next_header(self, line):
-        """Returns true if we found a next header, and sets the internal
-           line handler to the appropriate value.
+        """
+        Returns true if we found a next header, and sets the internal
+        line handler to the appropriate value.
         """
         if line == ";; ANSWER SECTION:\n":
             self.line_handler = self.parse_answer
@@ -84,6 +113,11 @@ class QueryResult(object):
         return True
 
     def parse_header(self, line):
+        """
+        Parse the header lines of the query response.
+        Parameters:
+        line: The current line of the response.
+        """
         if not self._check_next_header(line):
             status_match = self.status_re.search(line)
             flags_match = self.flags_re.search(line)
@@ -98,31 +132,69 @@ class QueryResult(object):
                 self.adcount = flags_match.group(5)
 
     def parse_question(self, line):
+        """
+        Parse the question section lines of the query response.
+        Parameters:
+        line: The current line of the response.
+        """
         if not self._check_next_header(line):
             if line != "\n":
                 self.question_section.append(line.strip())
 
     def parse_answer(self, line):
+        """
+        Parse the answer section lines of the query response.
+        Parameters:
+        line: The current line of the response.
+        """
         if not self._check_next_header(line):
             if line != "\n":
                 self.answer_section.append(line.strip())
 
     def parse_authority(self, line):
+        """
+        Parse the authority section lines of the query response.
+        Parameters:
+        line: The current line of the response.
+        """
         if not self._check_next_header(line):
             if line != "\n":
                 self.authority_section.append(line.strip())
 
-    def parse_authority(self, line):
+    def parse_additional(self, line):
+        """
+        Parse the additional section lines of the query response.
+        Parameters:
+        line: The current line of the response.
+        """
         if not self._check_next_header(line):
             if line != "\n":
                 self.additional_section.append(line.strip())
 
     def parse_footer(self, line):
+        """
+        Parse the footer lines of the query response.
+        Parameters:
+        line: The current line of the response.
+        """
         pass
 
 @step('A query for ([\w.]+) (?:type ([A-Z]+) )?(?:class ([A-Z]+) )?' +
       '(?:to ([^:]+)(?::([0-9]+))? )?should have rcode ([\w.]+)')
 def query(step, query_name, qtype, qclass, addr, port, rcode):
+    """
+    Run a query, check the rcode of the response, and store the query
+    result in world.last_query_result.
+    Parameters:
+    query_name ('query for <name>'): The domain name to query.
+    qtype ('type <type>', optional): The RR type to query. Defaults to A.
+    qclass ('class <class>', optional): The RR class to query. Defaults to IN.
+    addr ('to <address>', optional): The IP address of the nameserver to query.
+                           Defaults to 127.0.0.1.
+    port (':<port>', optional): The port number of the nameserver to query.
+                      Defaults to 47806.
+    rcode ('should have rcode <rcode>'): The expected rcode of the answer.
+    """
     if qtype is None:
         qtype = "A"
     if qclass is None:
@@ -138,6 +210,15 @@ def query(step, query_name, qtype, qclass, addr, port, rcode):
 
 @step('The SOA serial for ([\w.]+) should be ([0-9]+)')
 def query_soa(step, query_name, serial):
+    """
+    Convenience function to check the SOA SERIAL value of the given zone at
+    the nameserver at the default address (127.0.0.1:47806).
+    Parameters:
+    query_name ('for <name>'): The zone to find the SOA record for.
+    serial ('should be <number>'): The expected value of the SOA SERIAL.
+    If the rcode is not NOERROR, or the answer section does not contain the
+    SOA record, this step fails.
+    """
     query_result = QueryResult(query_name, "SOA", "IN", "127.0.0.1", "47806")
     assert "NOERROR" == query_result.rcode,\
         "Got " + query_result.rcode + ", expected NOERROR"
@@ -149,6 +230,16 @@ def query_soa(step, query_name, serial):
 
 @step('last query response should have (\S+) (.+)')
 def check_last_query(step, item, value):
+    """
+    Check a specific value in the reponse from the last successful query sent.
+    Parameters:
+    item: The item to check the value of
+    value: The expected value.
+    This performs a very simple direct string comparison of the QueryResult
+    member with the given item name and the given value.
+    Fails if the item is unknown, or if its value does not match the expected
+    value.
+    """
     assert world.last_query_result is not None
     assert item in world.last_query_result.__dict__
     lq_val = world.last_query_result.__dict__[item]
@@ -157,6 +248,17 @@ def check_last_query(step, item, value):
 
 @step('([a-zA-Z]+) section of the last query response should be')
 def check_last_query_section(step, section):
+    """
+    Check the entire contents of the given section of the response of the last
+    query.
+    Parameters:
+    section ('<section> section'): The name of the section (QUESTION, ANSWER,
+                                   AUTHORITY or ADDITIONAL).
+    The expected response is taken from the multiline part of the step in the
+    scenario. Differing whitespace is ignored, but currently the order is
+    significant.
+    Fails if they do not match.
+    """
     response_string = None
     if section.lower() == 'question':
         response_string = "\n".join(world.last_query_result.question_section)
diff --git a/tests/lettuce/features/terrain/steps.py b/tests/lettuce/features/terrain/steps.py
index 2df5290..4050940 100644
--- a/tests/lettuce/features/terrain/steps.py
+++ b/tests/lettuce/features/terrain/steps.py
@@ -1,3 +1,18 @@
+# Copyright (C) 2011  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
 #
 # This file contains a number of common steps that are general and may be used
 # By a lot of feature files.
@@ -8,18 +23,50 @@ import os
 
 @step('stop process (\w+)')
 def stop_a_named_process(step, process_name):
+    """
+    Stop the process with the given name.
+    Parameters:
+    process_name ('process <name>'): Name of the process to stop.
+    """
     world.processes.stop_process(process_name)
 
 @step('wait for (new )?(\w+) stderr message (\w+)')
 def wait_for_message(step, new, process_name, message):
+    """
+    Block until the given message is printed to the given process's stderr
+    output.
+    Parameter:
+    new: (' new', optional): Only check the output printed since last time
+                             this step was used for this process.
+    process_name ('<name> stderr'): Name of the process to check the output of.
+    message ('message <message>'): Output (part) to wait for.
+    Fails if the message is not found after 10 seconds.
+    """
     world.processes.wait_for_stderr_str(process_name, [message], new)
 
 @step('wait for (new )?(\w+) stdout message (\w+)')
 def wait_for_message(step, process_name, message):
+    """
+    Block until the given message is printed to the given process's stdout
+    output.
+    Parameter:
+    new: (' new', optional): Only check the output printed since last time
+                             this step was used for this process.
+    process_name ('<name> stderr'): Name of the process to check the output of.
+    message ('message <message>'): Output (part) to wait for.
+    Fails if the message is not found after 10 seconds.
+    """
     world.processes.wait_for_stdout_str(process_name, [message], new)
 
 @step('the file (\S+) should (not )?exist')
 def check_existence(step, file_name, should_not_exist):
+    """
+    Check the existence of the given file.
+    Parameters:
+    file_name ('file <name>'): File to check existence of.
+    should_not_exist ('not', optional): Whether it should or should not exist.
+    Fails if the file should exist and does not, or vice versa.
+    """
     if should_not_exist is None:
         assert os.path.exists(file_name), file_name + " does not exist"
     else:
diff --git a/tests/lettuce/features/terrain/terrain.py b/tests/lettuce/features/terrain/terrain.py
index 1aaf20e..a815385 100644
--- a/tests/lettuce/features/terrain/terrain.py
+++ b/tests/lettuce/features/terrain/terrain.py
@@ -1,3 +1,18 @@
+# Copyright (C) 2011  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
 #
 # This is the 'terrain' in which the lettuce lives. By convention, this is
 # where global setup and teardown is defined.
@@ -7,6 +22,7 @@
 #
 # We also use it to provide scenario invariants, such as resetting data.
 #
+
 from lettuce import *
 import subprocess
 import os.path
@@ -46,6 +62,15 @@ OUTPUT_WAIT_MAX_INTERVALS = 20
 class RunningProcess:
     def __init__(self, step, process_name, args):
         # set it to none first so destructor won't error if initializer did
+        """
+        Initialize the long-running process structure, and start the process.
+        Parameters:
+        step: The scenario step it was called from. This is used for
+              determining the output files for redirection of stdout
+              and stderr.
+        process_name: The name to refer to this running process later.
+        args: Array of arguments to pass to Popen().
+        """
         self.process = None
         self.step = step
         self.process_name = process_name
@@ -55,6 +80,12 @@ class RunningProcess:
         self._start_process(args)
 
     def _start_process(self, args):
+        """
+        Start the process.
+        Parameters:
+        args:
+        Array of arguments to pass to Popen().
+        """
         stderr_write = open(self.stderr_filename, "w")
         stdout_write = open(self.stdout_filename, "w")
         self.process = subprocess.Popen(args, 1, None, subprocess.PIPE,
@@ -64,6 +95,16 @@ class RunningProcess:
         self.stdout = open(self.stdout_filename, "r")
 
     def mangle_filename(self, filebase, extension):
+        """
+        Remove whitespace and non-default characters from a base string,
+        and return the substituted value. Whitespace is replaced by an
+        underscore. Any other character that is not an ASCII letter, a
+        number, a dot, or a hyphen or underscore is removed.
+        Parameter:
+        filebase: The string to perform the substitution and removal on
+        extension: An extension to append to the result value
+        Returns the modified filebase with the given extension
+        """
         filebase = re.sub("\s+", "_", filebase)
         filebase = re.sub("[^a-zA-Z0-9.\-_]", "", filebase)
         return filebase + "." + extension
@@ -73,6 +114,12 @@ class RunningProcess:
         # through an environment variable. Since we currently expect
         # lettuce to be run from our lettuce dir, we shall just use
         # the relative path 'output/'
+        """
+        Make sure the output directory for stdout/stderr redirection
+        exists.
+        Fails if it exists but is not a directory, or if it does not
+        and we are unable to create it.
+        """
         self._output_dir = os.getcwd() + os.sep + "output"
         if not os.path.exists(self._output_dir):
             os.mkdir(self._output_dir)
@@ -80,6 +127,11 @@ class RunningProcess:
             self._output_dir + " is not a directory."
 
     def _create_filenames(self):
+        """
+        Derive the filenames for stdout/stderr redirection from the
+        feature, scenario, and process name. The base will be
+        "<Feature>-<Scenario>-<process name>.[stdout|stderr]"
+        """
         filebase = self.step.scenario.feature.name + "-" +\
                    self.step.scenario.name + "-" + self.process_name
         self.stderr_filename = self._output_dir + os.sep +\
@@ -88,6 +140,11 @@ class RunningProcess:
                                self.mangle_filename(filebase, "stdout")
 
     def stop_process(self):
+        """
+        Stop this process by calling terminate(). Blocks until process has
+        exited. If remove_files_on_exit is True, redirected output files
+        are removed.
+        """
         if self.process is not None:
             self.process.terminate()
             self.process.wait()
@@ -96,10 +153,30 @@ class RunningProcess:
             self._remove_files()
 
     def _remove_files(self):
+        """
+        Remove the files created for redirection of stdout/stderr output.
+        """
         os.remove(self.stderr_filename)
         os.remove(self.stdout_filename)
 
     def _wait_for_output_str(self, filename, running_file, strings, only_new):
+        """
+        Wait for a line of output in this process. This will (if only_new is
+        False) first check all previous output from the process, and if not
+        found, check all output since the last time this method was called.
+        For each line in the output, the given strings array is checked. If
+        any output lines checked contains one of the strings in the strings
+        array, that string (not the line!) is returned.
+        Parameters:
+        filename: The filename to read previous output from, if applicable.
+        running_file: The open file to read new output from.
+        strings: Array of strings to look for.
+        only_new: If true, only check output since last time this method was
+                  called. If false, first check earlier output.
+        Returns the matched string.
+        Fails if none of the strings was read after 10 seconds
+        (OUTPUT_WAIT_INTERVAL * OUTPUT_WAIT_MAX_INTERVALS).
+        """
         if not only_new:
             full_file = open(filename, "r")
             for line in full_file:
@@ -122,10 +199,30 @@ class RunningProcess:
         assert False, "Timeout waiting for process output: " + str(strings)
 
     def wait_for_stderr_str(self, strings, only_new = True):
+        """
+        Wait for one of the given strings in this processes stderr output.
+        Parameters:
+        strings: Array of strings to look for.
+        only_new: If true, only check output since last time this method was
+                  called. If false, first check earlier output.
+        Returns the matched string.
+        Fails if none of the strings was read after 10 seconds
+        (OUTPUT_WAIT_INTERVAL * OUTPUT_WAIT_MAX_INTERVALS).
+        """
         return self._wait_for_output_str(self.stderr_filename, self.stderr,
                                          strings, only_new)
 
     def wait_for_stdout_str(self, strings, only_new = True):
+        """
+        Wait for one of the given strings in this processes stdout output.
+        Parameters:
+        strings: Array of strings to look for.
+        only_new: If true, only check output since last time this method was
+                  called. If false, first check earlier output.
+        Returns the matched string.
+        Fails if none of the strings was read after 10 seconds
+        (OUTPUT_WAIT_INTERVAL * OUTPUT_WAIT_MAX_INTERVALS).
+        """
         return self._wait_for_output_str(self.stdout_filename, self.stdout,
                                          strings, only_new)
 
@@ -134,51 +231,96 @@ class RunningProcess:
 # one-shot programs like dig or bindctl are started and closed separately
 class RunningProcesses:
     def __init__(self):
+        """
+        Initialize with no running processes.
+        """
         self.processes = {}
     
     def add_process(self, step, process_name, args):
+        """
+        Start a process with the given arguments, and store it under the given
+        name.
+        Parameters:
+        step: The scenario step it was called from. This is used for
+              determining the output files for redirection of stdout
+              and stderr.
+        process_name: The name to refer to this running process later.
+        args: Array of arguments to pass to Popen().
+        Fails if a process with the given name is already running.
+        """
         assert process_name not in self.processes,\
             "Process " + name + " already running"
         self.processes[process_name] = RunningProcess(step, process_name, args)
 
     def get_process(self, process_name):
+        """
+        Return the Process with the given process name.
+        Parameters:
+        process_name: The name of the process to return.
+        Fails if the process is not running.
+        """
         assert process_name in self.processes,\
             "Process " + name + " unknown"
         return self.processes[process_name]
 
     def stop_process(self, process_name):
+        """
+        Stop the Process with the given process name.
+        Parameters:
+        process_name: The name of the process to return.
+        Fails if the process is not running.
+        """
         assert process_name in self.processes,\
             "Process " + name + " unknown"
         self.processes[process_name].stop_process()
         del self.processes[process_name]
         
     def stop_all_processes(self):
+        """
+        Stop all running processes.
+        """
         for process in self.processes.values():
             process.stop_process()
     
     def keep_files(self):
+        """
+        Keep the redirection files for stdout/stderr output of all processes
+        instead of removing them when they are stopped later.
+        """
         for process in self.processes.values():
             process.remove_files_on_exit = False
 
     def wait_for_stderr_str(self, process_name, strings, only_new = True):
-        """Wait for any of the given strings in the given processes stderr 
-        output. If only_new is True, it will only look at the lines that are 
-        printed to stderr since the last time this method was called. If 
-        False, it will also look at the previously printed lines. This will 
-        block until one of the strings is found. TODO: we may want to put in 
-        a timeout for this... Returns the string that is found"""
+        """
+        Wait for one of the given strings in the given processes stderr output.
+        Parameters:
+        process_name: The name of the process to check the stderr output of.
+        strings: Array of strings to look for.
+        only_new: If true, only check output since last time this method was
+                  called. If false, first check earlier output.
+        Returns the matched string.
+        Fails if none of the strings was read after 10 seconds
+        (OUTPUT_WAIT_INTERVAL * OUTPUT_WAIT_MAX_INTERVALS).
+        Fails if the process is unknown.
+        """
         assert process_name in self.processes,\
            "Process " + process_name + " unknown"
         return self.processes[process_name].wait_for_stderr_str(strings,
                                                                 only_new)
 
     def wait_for_stdout_str(self, process_name, strings, only_new = True):
-        """Wait for any of the given strings in the given processes stderr 
-        output. If only_new is True, it will only look at the lines that are 
-        printed to stderr since the last time this method was called. If 
-        False, it will also look at the previously printed lines. This will 
-        block until one of the strings is found. TODO: we may want to put in 
-        a timeout for this... Returns the string that is found"""
+        """
+        Wait for one of the given strings in the given processes stdout output.
+        Parameters:
+        process_name: The name of the process to check the stdout output of.
+        strings: Array of strings to look for.
+        only_new: If true, only check output since last time this method was
+                  called. If false, first check earlier output.
+        Returns the matched string.
+        Fails if none of the strings was read after 10 seconds
+        (OUTPUT_WAIT_INTERVAL * OUTPUT_WAIT_MAX_INTERVALS).
+        Fails if the process is unknown.
+        """
         assert process_name in self.processes,\
            "Process " + process_name + " unknown"
         return self.processes[process_name].wait_for_stdout_str(strings,
@@ -186,6 +328,9 @@ class RunningProcesses:
 
 @before.each_scenario
 def initialize(scenario):
+    """
+    Global initialization for each scenario.
+    """
     # Keep track of running processes
     world.processes = RunningProcesses()
 
@@ -204,6 +349,9 @@ def initialize(scenario):
 
 @after.each_scenario
 def cleanup(scenario):
+    """
+    Global cleanup for each scenario.
+    """
     # Keep output files if the scenario failed
     if not scenario.passed:
         world.processes.keep_files()




More information about the bind10-changes mailing list