[PATCH] D45333: WIP: [LIT] Have lit run the lldb test suite

[Pavel Labath via Phabricator via llvm-commits]

labath added a comment.

In https://reviews.llvm.org/D45333#1058916, @JDevlieghere wrote:

> This isn't meant to be checked-in as is, however I'm looking for feedback as early as possible.
>
> There are currently two problems with the current diff:
>
> - `./bin/llvm-lit ../llvm/tools/lldb/lit/Suite/` doesn't work, and I haven't figured out why yet.
> - We'd run the (do)test-suite twice, once as part lit and once as part of `check-lldb`.


I think this is quite close to a working diff. The "running twice" issue can be fixed easily, I presume, and the rest of the issues I see are just minor details.

For the llvm-lit issue, I think it would be best if it worked like:

  llvm-lit  .../lldb/packages/Python/lldbsuite/test

as then (I hope) I could also specify some folder of that and run only the tests under that subfolder. However, I think we could live without that, initially.



================
Comment at: lit/Suite/lit.cfg:17
+# test_exec_root: The root path where tests should be run.
+config.test_source_root = os.path.join(config.lldb_src_root, 'test', 'testcases')
+config.test_exec_root = config.test_source_root
----------------
The testcases symlink thingy will not work on windows. I think you'll have to put the full path here.


================
Comment at: lit/Suite/lldbtest.py:45
+        testName, testExt = os.path.splitext(testFile)
+        cmd = self.dotest_cmd + [testPath, '-p', testName]
+        print ' '.join(cmd)
----------------
I think -p should match agains the full file name (i.e., you should be able to just pass `testFile` here).


================
Comment at: lit/Suite/lldbtest.py:46
+        cmd = self.dotest_cmd + [testPath, '-p', testName]
+        print ' '.join(cmd)
+
----------------
It looks like `executeCommand` accepts a list of args for a command as well, so it might be best to keep `cmd` as such.


https://reviews.llvm.org/D45333