[PATCH] [libcxx] Allow use of ShTest in libc++ tests along with other changes.

[Dan Albert]

================
Comment at: test/libcxx/compiler.py:6
@@ -5,2 +5,3 @@
 class CXXCompiler(object):
-    def __init__(self, path, flags=[], compile_flags=[], link_flags=[], use_ccache=False):
+    def __init__(self, path, flags=[], compile_flags=[], link_flags=[],
+                 use_ccache=False):
----------------
pylint really hates [] as a default argument (and apparently for [[ http://stackoverflow.com/a/9526480/632035 | good reason ]]).

Making the defaults be `None` and using `self.flags = flags or []` looks nice.

(I understand that using `self.flags = list(flags)` actually works around this problem, but I'd like it if we could change the style so it cuts down on noise for those of us that do run linters.)

================
Comment at: test/libcxx/compiler.py:113
@@ -94,2 +112,3 @@
+
     def dumpMacros(self, infiles=None, flags=[], env=None, cwd=None):
         if infiles is None:
----------------
`source_files` (not a part of this change, but might as well cause some collateral cleanup).

================
Comment at: test/libcxx/test/config.py:336
@@ -287,1 +335,3 @@
+        self.cxx.compile_flags += ['-I' + self.libcxx_src_root
+                                        + '/test/support']
         libcxx_headers = self.get_lit_conf('libcxx_headers',
----------------
Use `os.path.join`.

================
Comment at: test/libcxx/test/config.py:511
@@ +510,3 @@
+        compile_str = (self.cxx.path + ' -o %t.o %s -c ' + flags_str
+                                                         + compile_flags_str)
+        link_str = (self.cxx.path + ' -o %t.exe %t.o ' + flags_str
----------------
This alignment isn't correct. Align with the first element.

================
Comment at: test/libcxx/test/config.py:535
@@ +534,3 @@
+        not_py = os.path.join(self.libcxx_src_root, 'utils', 'not', 'not.py')
+        not_str = '%s %s %s' % (python_exe, not_py, lit_location)
+        sub.append(('not', python_exe + ' ' + not_py + ' ' + lit_location))
----------------
Unused.

================
Comment at: test/libcxx/test/format.py:30
@@ +29,3 @@
+    # TODO: Move this into lit's FileBasedTest
+    def getTestsInDirectory(self, testSuite, path_in_suite,
+                            litConfig, localConfig):
----------------
`self` and `litConfig` are unused (this could be a function rather than a method). Is this "overriding" something from `lit.formats.FileBasedTest`?

================
Comment at: test/libcxx/test/format.py:35
@@ +34,3 @@
+            # Ignore dot files and excluded tests.
+            if (filename.startswith('.') or filename in localConfig.excludes):
+                continue
----------------
No parens surrounding conditionals.

================
Comment at: test/libcxx/test/format.py:76
@@ +75,3 @@
+        if is_sh_test:
+            return lit.TestRunner._runShTest(test, lit_config,
+                                             self.execute_external, script,
----------------
This is "private" to `lit.TestRunner`. Should this function be given a public name in `lit.TestRunner`? Is there some other API that should be used?

================
Comment at: test/libcxx/test/format.py:128
@@ -113,32 +127,3 @@
 
-    def _build(self, exec_path, source_path, compile_only=False,
-               use_verify=False):
-        if compile_only:
-            cmd, out, err, rc = self._compile(exec_path, source_path,
-                                              use_verify)
-        else:
-            assert not use_verify
-            cmd, out, err, rc = self._compile_and_link(exec_path, source_path)
-        return self._make_report(cmd, out, err, rc)
-
-    def _clean(self, exec_path):  # pylint: disable=no-self-use
-        try:
-            os.remove(exec_path)
-        except OSError:
-            pass
-
-    def _run(self, exec_path, lit_config, in_dir=None):
-        cmd = []
-        if self.exec_env:
-            cmd.append('env')
-            cmd.extend('%s=%s' % (name, value)
-                       for name, value in self.exec_env.items())
-        cmd.append(exec_path)
-        if lit_config.useValgrind:
-            cmd = lit_config.valgrindArgs + cmd
-        out, err, rc = lit.util.executeCommand(cmd, cwd=in_dir)
-        return self._make_report(cmd, out, err, rc)
-
-    def _evaluate_test(self, test, use_verify, lit_config):
-        name = test.path_in_suite[-1]
+    def _evaluate_fail_test(self, test, tmpBase, execDir, lit_config):
         source_path = test.getSourcePath()
----------------
The last three arguments aren't used.

================
Comment at: test/libcxx/util.py:5
@@ +4,3 @@
+
+import lit.util  # pylint: disable=import-error
+
----------------
Unused.

http://reviews.llvm.org/D7073

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/