[PATCH] [LIT] Add SuffixDispatchTest for use in libc++ and libc++abi tests.

[Jonathan Roelofs]

Excellent, I like it!

You've got a "thumbs up" from me, but I'd like to hear @ddunbar and @danalbert's opinions on it too.

Also, can you post the corresponding libcxxabi and libcxx parts of this in another review?


Cheers,

Jon

================
Comment at: utils/lit/lit/formats/suffixdispatchtest.py:38
@@ +37,3 @@
+    @staticmethod
+    def _handle_metadata_line(ln, tag, out):
+        if tag not in ln:
----------------
_parse_metadata_line or _metadata_line_matches perhaps? 'handle' is a bit hand-wavey.

================
Comment at: utils/lit/lit/formats/suffixdispatchtest.py:90
@@ +89,3 @@
+                    "Test is unsupported with the following features: %s" % (
+                       ', '.join(unsupported_features),))
+
----------------
Trailing comma.

================
Comment at: utils/lit/lit/formats/suffixdispatchtest.py:106
@@ +105,3 @@
+        test_runner = None
+        for suffix_test in self.suffix_tests:
+            if name.endswith(suffix_test[0]):
----------------
I think you can do this in python....

  for (suffix, test) in self.suffix_tests:
    if name.endswith(suffix):
      test_runner = test
      break

================
Comment at: utils/lit/lit/formats/suffixdispatchtest.py:111
@@ +110,3 @@
+        if test_runner is None:
+            lit_config.fatal('Unrecognized test suffix: %s' % name)
+
----------------
When does this happen? Should it be an 'assert False'?

================
Comment at: utils/lit/lit/util.py:149
@@ +148,3 @@
+
+def _to_string(str_bytes):
+    if isinstance(str_bytes, str):
----------------
Some comments on these two functions would be nice here to explain the "why" behind them. At the moment I don't understand why these are needed.

http://reviews.llvm.org/D6206