Merge branch 'baserock/pedroalvarez/update' into baserock/morph

18 files changed, 301 insertions, 10 deletions

diff --git a/NEWS b/NEWS
index cb4584d..47fbd88 100644
--- a/NEWS
+++ b/NEWS
@@ -3,7 +3,39 @@ NEWS for cmdtest
 
 This file summarizes changes between releases of cmdtest.
 
-Version 0.X, released UNRELEASED
+Version 0.10, released 2013-10-05
+---------------------------------
+
+* Yarn now cleans the environment when it runs shell commands for the
+  implementation steps. The PATH variable is kept from the user's
+  environment, every other variable is either removed or hardcoded to
+  a specific value. More variables can be added explicitly to the test
+  environment with the new `--env NAME=VALUE` option. Additionally
+  yarn sets the `SRCDIR` environment variable to point at the root of
+  the source tree (the directory where yarn was invoked from).
+
+* A new option, `--timings`, has been added to yarn to report how long
+  each scenario and each step took.
+
+* Yarn now reports scenarios skipped because of ASSUMING failing.
+
+* Yarn manual page now documents DATADIR and SRCDIR environment
+  variables.
+
+Bug fixes:
+
+* Yarn now complains if a scenario has no THEN steps. Suggested by
+  Richard Maw.
+
+* Yarn now gives an error if there are no scenarios. Suggested by
+  Daniel Silverstone and others.
+
+* Yarn now checks for duplicate scenario names.
+
+* Yarn now always checks for IMPLEMENTS sections with case-insensitive
+  matching. Reported, with test case, by Jannis Pohlmann.
+
+Version 0.9, released 2013-07-23
 --------------------------------
 
 * Yarn now warns if an input file has no code blocks.
diff --git a/README.yarn b/README.yarn
index 76d57b5..16f7522 100644
--- a/README.yarn
+++ b/README.yarn
@@ -136,6 +136,15 @@ The following keywords are defined.
   The test runner creates a temporary directory, whose name is
   given to the shell code in the `DATADIR` environment variable.
 
+  The test runner sets the `SRCDIR` environment variable to the
+  path to the directory it was invoked from (by convention, the
+  root of the source tree of the project).
+
+  The test runner removes all other environment variables, except
+  `TERM`, `USER`, `USERNAME`, `LOGNAME`, `HOME`, and `PATH`. It also
+  forces `SHELL` set to `/bin/sh`, and `LC_ALL` set to `C`, in order
+  to have as clean an environment as possible for tests to run in.
+
   The shell commands get invoked with `/bin/sh -eu`, and need to
   be written accordingly. Be careful about commands that return a
   non-zero exit code. There will eventually be a library of shell
diff --git a/cmdtestlib.py b/cmdtestlib.py
index e6ca6bf..66cb9e9 100644
--- a/cmdtestlib.py
+++ b/cmdtestlib.py
@@ -14,7 +14,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 
-__version__ = '0.8.3'
+__version__ = '0.10'
 
 
 import os
diff --git a/debian/changelog b/debian/changelog
index 46eb065..2bd069f 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,15 @@
+cmdtest (0.10-1) unstable; urgency=low
+
+  * New upstream release.
+
+ -- Lars Wirzenius <liw@liw.fi>  Sat, 05 Oct 2013 15:07:43 +0100
+
+cmdtest (0.9-1) unstable; urgency=low
+
+  * New upstream release.
+
+ -- Lars Wirzenius <liw@liw.fi>  Tue, 23 Jul 2013 21:56:29 +0100
+
 cmdtest (0.8.3-1) unstable; urgency=low
 
   * New upstream.
diff --git a/yarn b/yarn
index 842f308..f4abcdc 100755
--- a/yarn
+++ b/yarn
@@ -18,6 +18,7 @@
 
 
 import cliapp
+import collections
 import logging
 import os
 import re
@@ -63,12 +64,21 @@ class YarnRunner(cliapp.Application):
                 'it should be empty or not exist',
             metavar='DIR')
 
+        self.settings.string_list(
+            ['env'],
+            'add NAME=VALUE to the environment when tests are run',
+            metavar='NAME=VALUE')
+
         self.settings.boolean(
             ['snapshot'],
             'make snapshots of test working directory '
                 'after each scenario step; you probably '
                 'want to use this with --tempdir')
 
+        self.settings.boolean(
+            ['timings'],
+            'report wall clock time for each scenario and step')
+
     def info(self, msg):
         if self.settings['verbose']:
             logging.info(msg)
@@ -104,6 +114,9 @@ class YarnRunner(cliapp.Application):
                 'step %Index(step,steps): %String(step_name)')
 
         scenarios, implementations = self.parse_scenarios(args)
+        self.check_there_are_scenarios(scenarios)
+        self.check_for_duplicate_scenario_names(scenarios)
+        self.check_for_thens(scenarios)
         self.connect_implementations(scenarios, implementations)
         shell_prelude = self.load_shell_libraries()
 
@@ -111,6 +124,10 @@ class YarnRunner(cliapp.Application):
         self.ts['num_scenarios'] = len(scenarios)
         self.info('Found %d scenarios' % len(scenarios))
 
+        self.scenarios_run = 0
+        self.steps_run = 0
+        self.timings = []
+
         start_time = time.time()
         failed_scenarios = []
         for scenario in self.select_scenarios(scenarios):
@@ -128,9 +145,13 @@ class YarnRunner(cliapp.Application):
 
         if not self.settings['quiet']:
             print (
-                'Scenario test suite PASS, with %d scenarios, '
+                'Scenario test suite PASS, with %d scenarios '
+                '(%d total steps), '
                 'in %.1f seconds' %
-                (len(scenarios), duration))
+                (self.scenarios_run, self.steps_run, duration))
+
+        if self.settings['timings']:
+            self.report_timings()
 
     def parse_scenarios(self, filenames):
         mdparser = yarnlib.MarkdownParser()
@@ -145,6 +166,36 @@ class YarnRunner(cliapp.Application):
 
         return block_parser.scenarios, block_parser.implementations
 
+    def check_there_are_scenarios(self, scenarios):
+        if not scenarios:
+            raise cliapp.AppException(
+                'There are no scenarios; must have at least one.')
+
+    def check_for_duplicate_scenario_names(self, scenarios):
+        counts = collections.Counter()
+        for s in scenarios:
+            counts[s.name] += 1
+
+        duplicates = [name for name in counts if counts[name] > 1]
+        if duplicates:
+            duplist = ''.join('  %s\n' % name for name in duplicates)
+            raise cliapp.AppException(
+                'There are scenarios with duplicate names:\n%s' % duplist)
+
+    def check_for_thens(self, scenarios):
+        no_thens = []
+        for scenario in scenarios:
+            for step in scenario.steps:
+                if step.what == 'THEN':
+                    break
+            else:
+                no_thens.append(scenario)
+
+        if no_thens:
+            raise cliapp.AppException(
+                'Some scenarios have no THENs:\n%s' %
+                ''.join('  "%s"\n' % s.name for s in scenarios))
+
     def connect_implementations(self, scenarios, implementations):
         for scenario in scenarios:
             for step in scenario.steps:
@@ -153,11 +204,11 @@ class YarnRunner(cliapp.Application):
     def connect_implementation(self, scenario, step, implementations):
         matching = [i for i in implementations
                     if step.what == i.what and
-                       re.match('(%s)$' % i.regexp, step.text, re.I)]
+                       self.implements_matches_step(i, step)]
 
         if len(matching) == 0:
             raise cliapp.AppException(
-                'Scenario %s, step "%s %s" has no matching '
+                'Scenario "%s", step "%s %s" has no matching '
                 'implementation' %
                 (scenario.name, step.what, step.text))
         if len(matching) > 1:
@@ -206,17 +257,22 @@ class YarnRunner(cliapp.Application):
         return scenarios
 
     def run_scenario(self, scenario, shell_prelude):
+        self.start_scenario_timing(scenario.name)
+        started = time.time()
+
         self.info('Running scenario %s' % scenario.name)
         self.ts['scenario'] = scenario
         self.ts['scenario_name'] = scenario.name
         self.ts['steps'] = scenario.steps
+        self.scenarios_run += 1
 
         if self.settings['no-act']:
             self.info('Pretending everything went OK')
+            self.remember_scenario_timing(time.time() - started)
             return True
 
         if self.settings['tempdir']:
-            tempdir = self.settings['tempdir']
+            tempdir = os.path.abspath(self.settings['tempdir'])
             if not os.path.exists(tempdir):
                 os.mkdir(tempdir)
         else:
@@ -240,6 +296,9 @@ class YarnRunner(cliapp.Application):
             self.snapshot_datadir(
                 tempdir, datadir, scenario, step_number, step)
             if exit != 0:
+                self.ts.notify(
+                    'Skipping "%s" because "%s %s" failed' %
+                    (scenario.name, step.what, step.text))
                 break
         else:
             for step in normal:
@@ -265,24 +324,65 @@ class YarnRunner(cliapp.Application):
         if not self.settings['snapshot']:
             shutil.rmtree(tempdir)
 
+        self.remember_scenario_timing(time.time() - started)
         return ok
 
+    def clean_env(self):
+        '''Return a clean environment for running tests.'''
+
+        whitelisted = [
+            'PATH',
+        ]
+
+        hardcoded = {
+            'TERM': 'dumb',
+            'SHELL': '/bin/sh',
+            'LC_ALL': 'C',
+            'USER': 'tomjon',
+            'USERNAME': 'tomjon',
+            'LOGNAME': 'tomjon',
+            'HOME': '/this/path/does/not/exist',
+        }
+
+        env = {}
+
+        for key in whitelisted:
+            if key in os.environ:
+                env[key] = os.environ[key]
+
+        for key in hardcoded:
+            env[key] = hardcoded[key]
+
+        for option_arg in self.settings['env']:
+            if '=' not in option_arg:
+                raise cliapp.AppException(
+                    '--env argument must contain "=" '
+                    'to separate environment variable name and value')
+            key, value = option_arg.split('=', 1)
+            env[key] = value
+
+        return env
+
     def run_step(self, datadir, scenario, step, shell_prelude, report_error):
+        started = time.time()
+
         self.info('Running step "%s %s"' % (step.what, step.text))
         self.ts['step'] = step
         self.ts['step_name'] = '%s %s' % (step.what, step.text)
+        self.steps_run += 1
 
-        m = re.match(step.implementation.regexp, step.text)
+        m = self.implements_matches_step(step.implementation, step)
         assert m is not None
-        env = os.environ.copy()
+        env = self.clean_env()
         env['DATADIR'] = datadir
+        env['SRCDIR'] = os.getcwd()
         for i, match in enumerate(m.groups('')):
             env['MATCH_%d' % (i+1)] = match
 
         shell_script = '%s\n\n%s\n' % (
             shell_prelude, step.implementation.shell)
         exit, stdout, stderr = cliapp.runcmd_unchecked(
-            ['sh', '-euc', shell_script], env=env)
+            ['sh', '-xeuc', shell_script], env=env)
 
         logging.debug('Exit code: %d' % exit)
         if stdout:
@@ -303,6 +403,9 @@ class YarnRunner(cliapp.Application):
                 (scenario.name, step.what, step.text, exit,
                  self.indent(stdout), self.indent(stderr)))
 
+        self.remember_step_timing(
+            '%s %s' % (step.what, step.text), time.time() - started)
+
         return exit
 
     def scenario_dir(self, tempdir, scenario):
@@ -336,8 +439,38 @@ class YarnRunner(cliapp.Application):
         nice = ''.join(nice)
         return nice
 
+    def implements_matches_step(self, implements, step):
+        '''Return re.Match if implements matches the step.
+
+        Otherwise, return None.
+
+        '''
+
+        m = re.match(implements.regexp, step.text, re.I)
+        if m and m.end() != len(step.text):
+            return None
+        return m
+
     def indent(self, s):
         return ''.join('    %s\n' % line for line in s.splitlines())
 
+    def start_scenario_timing(self, scenario_name):
+        self.timings.append((scenario_name, None, []))
+
+    def remember_scenario_timing(self, duration):
+        scenario_name, _, step_tuples = self.timings[-1]
+        self.timings[-1] = (scenario_name, duration, step_tuples)
+
+    def remember_step_timing(self, step_name, step_duration):
+        scenario_name, scenario_duration, step_tuples = self.timings[-1]
+        step_tuples = step_tuples + [(step_name, step_duration)]
+        self.timings[-1] = (scenario_name, scenario_duration, step_tuples)
+
+    def report_timings(self):
+        for scenario_name, scenario_duration, step_tuples in self.timings:
+            print '%5.1f %s' % (scenario_duration, scenario_name)
+            for step_name, step_duration in step_tuples:
+                print '  %5.1f %s' % (step_duration, step_name)
+
 
 YarnRunner(version=cmdtestlib.__version__).run()
diff --git a/yarn.1.in b/yarn.1.in
index 85d2d8e..f1708d9 100644
--- a/yarn.1.in
+++ b/yarn.1.in
@@ -109,7 +109,26 @@ by a programmer,
 but given a well-designed set of steps,
 with enough flexibility in their implementation,
 that quite a good test suite can be written.
+.PP
+The shell commands in an IMPLEMENTS section are run in the directory
+in which the user ran
+.BR yarn .
+The environment variable
+.B SRCDIR
+is set to the fully qualified path to that directory.
 .SH OPTIONS
+.SH ENVIRONMENT
+.TP
+.B DATADIR
+Fully qualified pathname to a temporary directory,
+in which the tests can use files.
+The temporary directory is removed at the end of the test execution,
+unless the user specifies otherwise with \-\-snapshot.
+.TP
+.B SRCDIR
+Fully qualitifed pathname to the directory in which the user ran
+.BR yarn .
+This is useful when the tests want to change the directory.
 .SH EXAMPLE
 To run
 .B yarn
diff --git a/yarn.tests/assuming-failure.stdout b/yarn.tests/assuming-failure.stdout
new file mode 100644
index 0000000..d7494ec
--- /dev/null
+++ b/yarn.tests/assuming-failure.stdout
@@ -0,0 +1 @@
+Skipping "foo" because "ASSUMING something" failed
diff --git a/yarn.tests/duplicate-scenario-names.exit b/yarn.tests/duplicate-scenario-names.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/yarn.tests/duplicate-scenario-names.exit
@@ -0,0 +1 @@
+1
diff --git a/yarn.tests/duplicate-scenario-names.script b/yarn.tests/duplicate-scenario-names.script
new file mode 100755
index 0000000..770f00a
--- /dev/null
+++ b/yarn.tests/duplicate-scenario-names.script
@@ -0,0 +1,18 @@
+#!/bin/sh
+
+set -eu
+
+
+cat <<EOF > "$DATADIR/test.yarn"
+    SCENARIO    foo
+    THEN        nop
+
+    SCENARIO    foo
+    THEN        nop
+
+    IMPLEMENTS THEN nop
+    true
+EOF
+
+./run-yarn "$DATADIR/test.yarn"
+
diff --git a/yarn.tests/duplicate-scenario-names.stderr b/yarn.tests/duplicate-scenario-names.stderr
new file mode 100644
index 0000000..d3fbc95
--- /dev/null
+++ b/yarn.tests/duplicate-scenario-names.stderr
@@ -0,0 +1,3 @@
+ERROR: There are scenarios with duplicate names:
+  foo
+
diff --git a/yarn.tests/env-option.script b/yarn.tests/env-option.script
new file mode 100755
index 0000000..9724ffc
--- /dev/null
+++ b/yarn.tests/env-option.script
@@ -0,0 +1,15 @@
+#!/bin/sh
+
+set -eu
+
+
+cat <<EOF > "$DATADIR/test.yarn"
+    SCENARIO    foo
+    THEN        yoyo is set
+
+    IMPLEMENTS THEN yoyo is set
+    env | grep '^yoyo='
+EOF
+
+./run-yarn --env yoyo=something "$DATADIR/test.yarn"
+
diff --git a/yarn.tests/env.script b/yarn.tests/env.script
new file mode 100755
index 0000000..d14a21a
--- /dev/null
+++ b/yarn.tests/env.script
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+set -eu
+
+cat << 'EOF' > "$DATADIR/env.yarn"
+    SCENARIO check environment
+    THEN DATADIR is set
+    AND SRCDIR is set
+    AND NOTSET is not set
+
+    IMPLEMENTS THEN (\S+) is set
+    env
+    env | grep "^$MATCH_1="
+
+    IMPLEMENTS THEN (\S+) is not set
+    ! env | grep "^$MATCH_1="
+EOF
+
+NOTSET=foo ./run-yarn "$DATADIR/env.yarn"
diff --git a/yarn.tests/no-scenarios.exit b/yarn.tests/no-scenarios.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/yarn.tests/no-scenarios.exit
@@ -0,0 +1 @@
+1
diff --git a/yarn.tests/no-scenarios.script b/yarn.tests/no-scenarios.script
new file mode 100755
index 0000000..ffd8e96
--- /dev/null
+++ b/yarn.tests/no-scenarios.script
@@ -0,0 +1,9 @@
+#!/bin/sh
+
+set -eu
+
+
+cat <<EOF > "$DATADIR/test.yarn"
+EOF
+
+./run-yarn "$DATADIR/test.yarn"
diff --git a/yarn.tests/no-scenarios.stderr b/yarn.tests/no-scenarios.stderr
new file mode 100644
index 0000000..66311c1
--- /dev/null
+++ b/yarn.tests/no-scenarios.stderr
@@ -0,0 +1 @@
+ERROR: There are no scenarios; must have at least one.
diff --git a/yarn.tests/no-then.exit b/yarn.tests/no-then.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/yarn.tests/no-then.exit
@@ -0,0 +1 @@
+1
diff --git a/yarn.tests/no-then.script b/yarn.tests/no-then.script
new file mode 100755
index 0000000..491853f
--- /dev/null
+++ b/yarn.tests/no-then.script
@@ -0,0 +1,14 @@
+#!/bin/sh
+
+set -eu
+
+
+cat <<EOF > "$DATADIR/1.yarn"
+    SCENARIO    foo
+    WHEN        doing ok
+
+    IMPLEMENTS WHEN doing ok
+    true
+EOF
+
+./run-yarn "$DATADIR/1.yarn"
diff --git a/yarn.tests/no-then.stderr b/yarn.tests/no-then.stderr
new file mode 100644
index 0000000..b018036
--- /dev/null
+++ b/yarn.tests/no-then.stderr
@@ -0,0 +1,3 @@
+ERROR: Some scenarios have no THENs:
+  "foo"
+