[1290] clean up steps, and add documentation and comments

tests/lettuce/README

@@ -23,6 +23,8 @@ Running the tests
 At this moment, we have a fixed port for local tests in our setups, port 47806.
 This port must be free. (TODO: can we make this run-time discovered?).
 Port 47805 is used for cmdctl, and must also be available.
+(note, we will need to extend this to a range, or if possible, we will need to
+do some on-the-fly available port finding)
 
 The bind10 main script, bindctl script, and dig must all be in the default
 search path of your environment, and BIND 10 must not be running if you use
@@ -35,6 +37,27 @@ with the build tree version of bind. If your shell uses export to set
 environment variables, you can source the script setup_intree_bind10.sh, then
 run lettuce.
 
+Due to the default way lettuce prints its output, it is advisable to run it
+in a terminal that is wide than the default. If you see a lot of lines twice
+in different colors, the terminal is not wide enough.
+
+If you just want to run one specific feature test, use
+lettuce features/<feature file>
+
+To run a specific scenario from a feature, use
+lettuce features/<feature file> -s <scenario number>
+
+If any scenario fails, the output from long-running processes will be stored
+in the output directory. The name of the files will be
+<Feature name>-<Scenario name>-<Process name>.stdout and
+<Feature name>-<Scenario name>-<Process name>.stderr
+Where spaces and other non-standard characters are replaced by an underscore.
+The process name is either the standard name for said process (e.g. 'bind10'),
+or the name given to it by the test ('when i run bind10 as <name>').
+
+These files *will* be overwritten or deleted if the tests are run again, so
+if you want to inspect them after a failed test, either do so immediately or
+move the files.
 
 Extending tests
 ---------------

tests/lettuce/configurations/no_db_file.config

@@ -1 +1 @@
-{"version": 2, "Auth": {"database_file": "test.db", "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"}]}}

tests/lettuce/features/server_from_sqlite3.feature

@@ -1,45 +1,94 @@
 Feature: SQLite3 backend
-    In order to support SQLite3
-    As administrators
-    We test serving an sqlite3 backend
+    This is an example Feature set. Is is mainly intended to show
+    our use of the lettuce tool and our own framework for it
+    The first scenario is to show what a simple test would look like, and
+    is intentionally uncommented.
+    The later scenarios have comments to show what the test steps do and
+    support
+    
+    Scenario: A simple example
+        Given I have bind10 running with configuration example.org.config
+        A query for www.example.org should have rcode NOERROR
+        A query for www.doesnotexist.org should have rcode REFUSED
+        The SOA serial for example.org should be 1234
 
     Scenario: New database
-        Given I have no database
+        # This test checks whether a database file is automatically created
+        # Underwater, we take advantage of our intialization routines so
+        # that we are sure this file does not exist, see
+        # features/terrain/terrain.py
+        
+        # Standard check to test (non-)existance of a file
+        # This file is actually automatically
+        The file data/test_nonexistent_db.sqlite3 should not exist
+
+        # In the first scenario, we used 'given I have bind10 running', which
+        # is actually a compound step consisting of the following two
+        # one to start the server
         When I start bind10 with configuration no_db_file.config
+        # And one to wait until it reports that b10-auth has started
         Then wait for bind10 auth to start
+
+        # This is a general step to stop a named process. By convention,
+        # the default name for any process is the same as the one we
+        # use in the start step (for bind 10, that is 'I start bind10 with')
+        # See scenario 'Multiple instances' for more.
         Then stop process bind10
-        I should see a database file
+        
+        # Now we use the first step again to see if the file has been created
+        The file data/test_nonexistent_db.sqlite3 should exist
 
     Scenario: example.org queries
         # This scenario performs a number of queries and inspects the results
-        # This is not only to test, but also to show the different options
-        # we have to inspect the data
+        # Simple queries have already been show, but after we have sent a query,
+        # we can also do more extensive checks on the result.
+        # See querying.py for more information on these steps.
+        
+        # note: lettuce can group similar checks by using tables, but we
+        # intentionally do not make use of that here
 
         # This is a compound statement that starts and waits for the
         # started message
         Given I have bind10 running with configuration example.org.config
 
-        # A simple query that is not examined further
+        # Some simple queries that is not examined further
         A query for www.example.com should have rcode REFUSED
+        A query for www.example.org should have rcode NOERROR
 
         # A query where we look at some of the result properties
         A query for www.example.org should have rcode NOERROR
-        The last query should have qdcount 1
-        The last query should have ancount 1
-        The last query should have nscount 3
-        The last query should have adcount 0
+        The last query response should have qdcount 1
+        The last query response should have ancount 1
+        The last query response should have nscount 3
+        The last query response should have adcount 0
+        # The answer section can be inspected in its entirety; in the future
+        # we may add more granular inspection steps
+        The answer section of the last query response should be
+        """
+        www.example.org.   3600    IN    A      192.0.2.1
+        """
+
+        A query for example.org type NS should have rcode NOERROR
+        The answer section of the last query response should be
+        """
+        example.org. 3600 IN NS ns1.example.org.
+        example.org. 3600 IN NS ns2.example.org.
+        example.org. 3600 IN NS ns3.example.org.
+        """
+
+        # We have a specific step for checking SOA serial numbers
         The SOA serial for example.org should be 1234
 
         # Another query where we look at some of the result properties
         A query for doesnotexist.example.org should have rcode NXDOMAIN
-        The last query should have qdcount 1
-        The last query should have ancount 0
-        The last query should have nscount 1
-        The last query should have adcount 0
-        The last query should have flags qr aa rd
+        The last query response should have qdcount 1
+        The last query response should have ancount 0
+        The last query response should have nscount 1
+        The last query response should have adcount 0
+        The last query response should have flags qr aa rd
 
         A query for www.example.org type TXT should have rcode NOERROR
-        The last query should have ancount 0
+        The last query response should have ancount 0
 
         # Some queries where we specify more details about what to send and
         # where

tests/lettuce/features/terrain/bind10_control.py

@@ -39,7 +39,8 @@ def start_bind10(step, config_file, cmdctl_port, process_name):
 def wait_for_auth(step, process_name):
     if process_name is None:
         process_name = "bind10"
-    world.processes.wait_for_stderr_str(process_name, ['AUTH_SERVER_STARTED'])
+    world.processes.wait_for_stderr_str(process_name, ['AUTH_SERVER_STARTED'],
+                                        False)
 
 @step('have bind10 running(?: with configuration ([\w.]+))?')
 def have_bind10_running(step, config_file):

tests/lettuce/features/terrain/querying.py

@@ -64,44 +64,55 @@ class QueryResult(object):
             for out in dig_process.stdout:
                 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.
+        """
+        if line == ";; ANSWER SECTION:\n":
+            self.line_handler = self.parse_answer
+        elif line == ";; AUTHORITY SECTION:\n":
+            self.line_handler = self.parse_authority
+        elif line == ";; ADDITIONAL SECTION:\n":
+            self.line_handler = self.parse_additional
+        elif line.startswith(";; Query time"):
+            self.line_handler = self.parse_footer
+        else:
+            return False
+        return True
+
     def parse_header(self, line):
-        status_match = self.status_re.search(line)
-        flags_match = self.flags_re.search(line)
-        if status_match is not None:
-            self.opcode = status_match.group(1)
-            self.rcode = status_match.group(2)
-        elif flags_match is not None:
-            self.flags = flags_match.group(1)
-            self.qdcount = flags_match.group(2)
-            self.ancount = flags_match.group(3)
-            self.nscount = flags_match.group(4)
-            self.adcount = flags_match.group(5)
-        elif line == ";; QUESTION SECTION:\n":
-            self.line_handler = self.parse_question
+        if not self._check_next_header(line):
+            status_match = self.status_re.search(line)
+            flags_match = self.flags_re.search(line)
+            if status_match is not None:
+                self.opcode = status_match.group(1)
+                self.rcode = status_match.group(2)
+            elif flags_match is not None:
+                self.flags = flags_match.group(1)
+                self.qdcount = flags_match.group(2)
+                self.ancount = flags_match.group(3)
+                self.nscount = flags_match.group(4)
+                self.adcount = flags_match.group(5)
 
     def parse_question(self, line):
-        if line == ";; ANSWER SECTION:\n":
-            self.line_handler = self.parse_answer
-        elif line != "\n":
-            self.question_section.append(line)
+        if not self._check_next_header(line):
+            if line != "\n":
+                self.question_section.append(line.strip())
 
     def parse_answer(self, line):
-        if line == ";; AUTHORITY SECTION:\n":
-            self.line_handler = self.parse_authority
-        elif line != "\n":
-            self.answer_section.append(line)
+        if not self._check_next_header(line):
+            if line != "\n":
+                self.answer_section.append(line.strip())
 
     def parse_authority(self, line):
-        if line == ";; ADDITIONAL SECTION:\n":
-            self.line_handler = self.parse_additional
-        elif line != "\n":
-            self.additional_section.append(line)
+        if not self._check_next_header(line):
+            if line != "\n":
+                self.authority_section.append(line.strip())
 
     def parse_authority(self, line):
-        if line.startswith(";; Query time"):
-            self.line_handler = self.parse_footer
-        elif line != "\n":
-            self.additional_section.append(line)
+        if not self._check_next_header(line):
+            if line != "\n":
+                self.additional_section.append(line.strip())
 
     def parse_footer(self, line):
         pass
@@ -133,10 +144,31 @@ def query_soa(step, query_name, serial):
     assert serial == soa_parts[6],\
         "Got SOA serial " + soa_parts[6] + ", expected " + serial
 
-@step('last query should have (\S+) (.+)')
+@step('last query response should have (\S+) (.+)')
 def check_last_query(step, item, 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]
     assert str(value) == str(lq_val),\
            "Got: " + str(lq_val) + ", expected: " + str(value)
+
+@step('([a-zA-Z]+) section of the last query response should be')
+def check_last_query_section(step, section):
+    response_string = None
+    if section.lower() == 'question':
+        response_string = "\n".join(world.last_query_result.question_section)
+    elif section.lower() == 'answer':
+        response_string = "\n".join(world.last_query_result.answer_section)
+    elif section.lower() == 'authority':
+        response_string = "\n".join(world.last_query_result.answer_section)
+    elif section.lower() == 'additional':
+        response_string = "\n".join(world.last_query_result.answer_section)
+    else:
+        assert False, "Unknown section " + section
+    # replace whitespace of any length by one space
+    response_string = re.sub("[ \t]+", " ", response_string)
+    expect = re.sub("[ \t]+", " ", step.multiline)
+    assert response_string.strip() == expect.strip(),\
+        "Got:\n'" + response_string + "'\nExpected:\n'" + step.multiline +"'"
+    
+    

tests/lettuce/features/terrain/steps.py

@@ -18,12 +18,9 @@ def wait_for_message(step, new, process_name, message):
 def wait_for_message(step, process_name, message):
     world.processes.wait_for_stdout_str(process_name, [message], new)
 
-@step('Given I have no database')
-def given_i_have_no_database(step):
-    if os.path.exists("test.db"):
-        os.remove("test.db")
-
-@step('I should see a database file')
-def i_should_see_a_database_file(step):
-    assert os.path.exists("test.db")
-    os.remove("test.db")
+@step('the file (\S+) should (not )?exist')
+def check_existence(step, file_name, should_not_exist):
+    if should_not_exist is None:
+        assert os.path.exists(file_name), file_name + " does not exist"
+    else:
+        assert not os.path.exists(file_name), file_name + " exists"

tests/lettuce/features/terrain/terrain.py

@@ -14,6 +14,14 @@ import shutil
 import re
 import time
 
+# In order to make sure we start all tests with a 'clean' environment,
+# We perform a number of initialization steps, like restoring configuration
+# files, and removing generated data files.
+
+# This approach may not scale; if so we should probably provide specific
+# initialization steps for scenarios. But until that is shown to be a problem,
+# It will keep the scenarios cleaner.
+
 # This is a list of files that are freshly copied before each scenario
 # The first element is the original, the second is the target that will be
 # used by the tests that need them
@@ -21,12 +29,20 @@ copylist = [
 ["configurations/example.org.config.orig", "configurations/example.org.config"]
 ]
 
+# This is a list of files that, if present, will be removed before a scenario
+removelist = [
+"data/test_nonexistent_db.sqlite3"
+]
+
+# When waiting for output data of a running process, use OUTPUT_WAIT_INTERVAL
+# as the interval in which to check again if it has not been found yet.
+# If we have waited OUTPUT_WAIT_MAX_INTERVALS times, we will abort with an
+# error (so as not to hang indefinitely)
 OUTPUT_WAIT_INTERVAL = 0.5
-OUTPUT_WAIT_MAX_INTERVALS = 10
+OUTPUT_WAIT_MAX_INTERVALS = 20
 
 # class that keeps track of one running process and the files
-# we created for it. This needs to be moved to our framework-framework
-# as it is not specifically for bind10
+# we created for it.
 class RunningProcess:
     def __init__(self, step, process_name, args):
         # set it to none first so destructor won't error if initializer did
@@ -49,7 +65,7 @@ class RunningProcess:
 
     def mangle_filename(self, filebase, extension):
         filebase = re.sub("\s+", "_", filebase)
-        filebase = re.sub("[^a-zA-Z.\-_]", "", filebase)
+        filebase = re.sub("[^a-zA-Z0-9.\-_]", "", filebase)
         return filebase + "." + extension
 
     def _check_output_dir(self):
@@ -182,6 +198,10 @@ def initialize(scenario):
     for item in copylist:
         shutil.copy(item[0], item[1])
 
+    for item in removelist:
+        if os.path.exists(item):
+            os.remove(item)
+
 @after.each_scenario
 def cleanup(scenario):
     # Keep output files if the scenario failed
@@ -189,3 +209,4 @@ def cleanup(scenario):
         world.processes.keep_files()
     # Stop any running processes we may have had around
     world.processes.stop_all_processes()
+