* When test listing fails, testr will now report an error rather than

  incorrectly trying to run zero tests. A test listing failure is detected by
  the returncode of the test listing process. (Robert Collins, #1185231)

4 files changed, 21 insertions, 3 deletions

diff --git a/NEWS b/NEWS
index 16d5498..7feea63 100644
--- a/NEWS
+++ b/NEWS
@@ -27,6 +27,10 @@ CHANGES
   co-dependent tests to be executed, so partial test runs (e.g. --failing)
   may still fail.  (Matthew Treinish, Robert Collins)
 
+* When test listing fails, testr will now report an error rather than
+  incorrectly trying to run zero tests. A test listing failure is detected by
+  the returncode of the test listing process. (Robert Collins, #1185231)
+
 0.0.15
 ++++++
 
diff --git a/doc/MANUAL.txt b/doc/MANUAL.txt
index 788f93c..28b286b 100644
--- a/doc/MANUAL.txt
+++ b/doc/MANUAL.txt
@@ -137,8 +137,10 @@ All the normal rules for invoking test program commands apply: extra parameters
 will be passed through, if a test list is being supplied test_option can be
 used via $IDOPTION.
 
-The output of the test command when this option is supplied should be a series
-of test ids, in any order, ``\n`` separated on stdout.
+The output of the test command when this option is supplied should be a subunit
+test enumeration. For subunit v1 that is a series of test ids, in any order,
+``\n`` separated on stdout. For v2 use the subunit protocol and emit one event
+per test with each test having status 'exists'.
 
 To test whether this is working the `testr list-tests` command can be useful.
 
diff --git a/testrepository/testcommand.py b/testrepository/testcommand.py
index 2cb4d50..267f4ef 100644
--- a/testrepository/testcommand.py
+++ b/testrepository/testcommand.py
@@ -293,7 +293,10 @@ class TestListingFixture(Fixture):
             run_proc = self.ui.subprocess_Popen(list_cmd, shell=True,
                 stdout=subprocess.PIPE, stdin=subprocess.PIPE)
             out, err = run_proc.communicate()
-            # Should we raise on non-zero exit?
+            if run_proc.returncode != 0:
+                raise ValueError(
+                    "Non-zero exit code (%d) from test listing."
+                    " stdout=%r, stderr=%r" % (run_proc.returncode, out, err))
             ids = parse_enumeration(out)
             return ids
         finally:
diff --git a/testrepository/tests/test_testcommand.py b/testrepository/tests/test_testcommand.py
index 605d490..cdaeaa8 100644
--- a/testrepository/tests/test_testcommand.py
+++ b/testrepository/tests/test_testcommand.py
@@ -321,6 +321,15 @@ class TestTestCommand(ResourcedTestCase):
         fixture = self.useFixture(command.get_run_command())
         self.assertEqual(set(['returned', 'ids']), set(fixture.list_tests()))
 
+    def test_list_tests_nonzero_exit(self):
+        ui, command = self.get_test_ui_and_cmd()
+        ui.proc_results = [1]
+        self.set_config(
+            '[DEFAULT]\ntest_command=foo $LISTOPT $IDLIST\ntest_id_list_default=whoo yea\n'
+            'test_list_option=--list\n')
+        fixture = self.useFixture(command.get_run_command())
+        self.assertThat(lambda:fixture.list_tests(), raises(ValueError))
+
     def test_partition_tests_smoke(self):
         repo = memory.RepositoryFactory().initialise('memory:')
         # Seed with 1 slow and 2 tests making up 2/3 the time.