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

[Dan Albert]

When we talked about this you had mentioned that this would make it easier to access the tests that were built for debugging. I don't see anything here that does that. Is that going to be a follow up patch?


================
Comment at: test/libcxx/util.py:34
@@ +33,3 @@
+ at contextmanager
+def withFilename(name):
+    # yeilds a filename within a with statement. No action is taken upon scope
----------------
This is really a `NullContextManager` (nothing is specific to a file name). I'm not sure what a good name for the function would be. Possibly just `nullContext`?

================
Comment at: utils/not/not.py:3
@@ +2,3 @@
+
+# not.py is a utility for inverting the return code of commands.
+# It acts similar to llvm/utils/not.
----------------
The block should be the module docstring.

================
Comment at: utils/not/not.py:12
@@ +11,3 @@
+    argv = sys.argv
+    del argv[0]
+    if len(argv) > 0 and argv[0] == '--crash':
----------------
`sys.argv` should be treated as constant, imo. Make a copy if you're going to use it this way.

================
Comment at: utils/not/not.py:20
@@ +19,3 @@
+        return 1
+    prog = lit.util.which(argv[0])
+    if prog is None:
----------------
Why does LIT use its own home grown version of this? https://docs.python.org/2/distutils/apiref.html#module-distutils.spawn

There's even a comment in the code that says "Would be nice if Python had a lib function for this". Was `distutils.spawn.find_executable` not available when that was written?

If you use the `distutils` API (and just use subprocess instead of `lit.util.executeCommand` for this you can get rid of the need for `--lit-site`, which makes up most of the gross in this file.

================
Comment at: utils/not/not.py:25
@@ +24,3 @@
+    out, err, rc = lit.util.executeCommand(argv)
+    if rc == 0 and not expectCrash:
+        return 1
----------------
This doesn't directly mirror the one in `llvm/utils`. The other one only considers negative values success if passed `--crash`.

Aside from that, it's kind of a weird flag imo. I would have made it a separate `expect_crash` command. `not --crash` reads to me as "doesn't crash".

http://reviews.llvm.org/D7073

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