[MERGE] Adding --starting-with <test_id> option to selftest make it run 5x faster

[John Arbash Meinel]

John Arbash Meinel has voted tweak.
Status is now: Conditionally approved
Comment:
-    if keep_only is None:
-        loader = TestUtil.TestLoader()
-    else:
+    if starting_with is not None:
+        # We take precedence over keep_only because *at loading time* 
using
+        # both options means we will load less tests for the same final 
result.
+        loader = 
TestUtil.FilteredByModuleTestLoader(starting_with.startswith)
+    elif keep_only is not None:
          id_filter = TestIdList(keep_only)
          loader = 
TestUtil.FilteredByModuleTestLoader(id_filter.refers_to)
+    else:
+        loader = TestUtil.TestLoader()
      suite = loader.suiteClass()

      # modules building their suite with loadTestsFromModuleNames
@@ -2807,8 +2839,16 @@
          'bzrlib.version_info_formats.format_custom',
          ]

+    def interesting_module(name):
+        if starting_with is not None:
+            return name.startswith(starting_with)
+        elif keep_only is not None:
+            return id_filter.refers_to(name)
+        else:
+            return True
+

This seems like it would be better said by creating a custom function in 
the first if/else block. So it would be:

if starting_with:
   def interesting(name):
     return starting_with(name)

That also means the 'if' test isn't run for every test being checked.

It also seems like Robert's recent condition test patch is somewhat 
related/conflicting. His is a bit different because it is a splitting 
filter, while filter-at-loading time intentionally doesn't want to 
determine the full names unless they are likely to be in the included 
set.


...

-        for id in not_found:
-            bzrlib.trace.warning('"%s" not found in the test suite', 
id)
+        if starting_with is not None:
+            # No need to annoy the tester with tests not found since 
when both
+            # options are used *there will be* tests excluded from the 
list.
+            pass
+        else:
+            for id in not_found:
+                bzrlib.trace.warning('"%s" not found in the test 
suite', id)

^- This seems like more of an odd interaction between "keep-only" and 
"startswith". Is there really a reason to support both? IMO they could 
be exclusive options.

=== modified file 'bzrlib/tests/blackbox/test_selftest.py'
--- bzrlib/tests/blackbox/test_selftest.py      2008-01-21 10:51:02 
+0000
+++ bzrlib/tests/blackbox/test_selftest.py      2008-04-23 19:43:13 
+0000
@@ -575,3 +575,12 @@
      def test_load_unknown(self):
          out, err = self.run_bzr('selftest --load-list I_do_not_exist ',
                                  retcode=3)
+
+
+class TestSelftestStartingWith(TestCase):
+
+    def test_starting_with(self):
+        out, err = self.run_bzr(
+            ['selftest', '--starting-with', self.id(), '--list'])
+        self.assertContainsRe(out, "Listed 1 test in")
+

^- This seems a bit heavy for testing --starting-with. I don't know how 
slow it is, though, if it isn't loading a lot of the suite.

It might be more interesting to have 2 functions

def test_starting_with(self):
   ...

def test_starting_with_a_longer_name(self):

At the least, I think we should test the name of the test that shows up, 
rather than just 1 showing up. I can respect that there is some 
difficulty because of screen width, etc, though.


+    def test_condition_id_startswith(self):
+        klass = 'bzrlib.tests.test_selftest.TestSelftestFiltering.'
+        start = klass + 'test_condition_id_starts'
+        test_names = [klass + 'test_condition_id_startswith']
+        filtered_suite = filter_suite_by_condition(
+            self.suite, tests.condition_id_startswith(start))
+        my_pattern = 
'TestSelftestFiltering.*test_condition_id_startswith'
+        re_filtered = filter_suite_by_re(self.suite, my_pattern)
+        self.assertEqual(_test_ids(re_filtered), 
_test_ids(filtered_suite))
+

^- this certainly seems like something that would be changed with 
"filter_suite_by_condition" from Robert.

For details, see: 
http://bundlebuggy.aaronbentley.com/request/%3Cm24p9rjm8z.fsf%40free.fr%3E