[LIT] Add new LLDB test format

[JDevlieghere]

Summary

This adds a new test format to lit which will be used for LLDB.

(If it's possible to define a new format outside the LLVM repo, I'm more than happy to move it into the LLDB repo)

[JDevlieghere]

As discussed in D45215

[zturner]

It should be possible to define it outside the LLVM repo. Just in llvm/lldb/lit/lit.cfg replace this line:

config.test_format = lit.formats.ShTest(execute_external)

with something like this:

import lldb_format
config.test_format = lldb_format.LLDBTestFormat()

and that file can live in the lldb repo.

[JDevlieghere]

In D45332#1058914, @zturner wrote:

It should be possible to define it outside the LLVM repo. Just in llvm/lldb/lit/lit.cfg replace this line:

config.test_format = lit.formats.ShTest(execute_external)

with something like this:

import lldb_format
config.test_format = lldb_format.LLDBTestFormat()

and that file can live in the lldb repo.

Probably I'm doing something wrong then because I couldn't get that to work. I moved the lldbtest.py in the Suite dir and then I got an import error.

ImportError: No module named lldbtest

[zturner]

I don't think sys.path is set up correctly to be able to find the lldbtest package from the lldb/lit folder.

These things kind of evolved separately, and the lldb/lit folder was created as a place to start iterating on LLVM-style lit / FileCheck tests. These kind of tests -- by definition -- don't really use the SB API, so no work was ever done to set up paths correctly so that it could write import lldb or to re-use any of the other stuff from packages/Python.

I'm not sure what the best thing to do is, but usually the canonical structuring is to have the test files in the same tree as the lit configuration. So perhaps you could put a lit configuration file in lldb/packages/Python/lldbsuite and have that be separate from lldb/lit, with the goal of eventually (possibly) merging them. Then have a separate CMake target so you'd still have check-lldb-lit which goes into the lldb/lit directory, and another one like check-lldb-lit-dotest which starts from the lldb/packages/Python/lldbsuite directory.

On the other hand, if you want to see how dotest.py sets up its sys.path, have a look at lldb/test/dotest.py The magic is in this use_lldb_suite function, which walks backwards through the tree until it finds the root, then dives into the lldbsuite folder to manually add it to sys.path.

[JDevlieghere]

In D45332#1058970, @zturner wrote:

I don't think sys.path is set up correctly to be able to find the lldbtest package from the lldb/lit folder.

These things kind of evolved separately, and the lldb/lit folder was created as a place to start iterating on LLVM-style lit / FileCheck tests. These kind of tests -- by definition -- don't really use the SB API, so no work was ever done to set up paths correctly so that it could write import lldb or to re-use any of the other stuff from packages/Python.

I'm not sure what the best thing to do is, but usually the canonical structuring is to have the test files in the same tree as the lit configuration. So perhaps you could put a lit configuration file in lldb/packages/Python/lldbsuite and have that be separate from lldb/lit, with the goal of eventually (possibly) merging them. Then have a separate CMake target so you'd still have check-lldb-lit which goes into the lldb/lit directory, and another one like check-lldb-lit-dotest which starts from the lldb/packages/Python/lldbsuite directory.

On the other hand, if you want to see how dotest.py sets up its sys.path, have a look at lldb/test/dotest.py The magic is in this use_lldb_suite function, which walks backwards through the tree until it finds the root, then dives into the lldbsuite folder to manually add it to sys.path.

Do you feel all that outweighs the alternative of just having the format in llvm/Utils as is the case in this diff? We already have some LLDB specific stuff there and I would argue that conceptually it makes (at least a little) sense to have all the format living together.

[labath]

In D45332#1058976, @JDevlieghere wrote:

In D45332#1058970, @zturner wrote:

I don't think sys.path is set up correctly to be able to find the lldbtest package from the lldb/lit folder.

These things kind of evolved separately, and the lldb/lit folder was created as a place to start iterating on LLVM-style lit / FileCheck tests. These kind of tests -- by definition -- don't really use the SB API, so no work was ever done to set up paths correctly so that it could write import lldb or to re-use any of the other stuff from packages/Python.

I'm not sure what the best thing to do is, but usually the canonical structuring is to have the test files in the same tree as the lit configuration. So perhaps you could put a lit configuration file in lldb/packages/Python/lldbsuite and have that be separate from lldb/lit, with the goal of eventually (possibly) merging them. Then have a separate CMake target so you'd still have check-lldb-lit which goes into the lldb/lit directory, and another one like check-lldb-lit-dotest which starts from the lldb/packages/Python/lldbsuite directory.

On the other hand, if you want to see how dotest.py sets up its sys.path, have a look at lldb/test/dotest.py The magic is in this use_lldb_suite function, which walks backwards through the tree until it finds the root, then dives into the lldbsuite folder to manually add it to sys.path.

Do you feel all that outweighs the alternative of just having the format in llvm/Utils as is the case in this diff? We already have some LLDB specific stuff there and I would argue that conceptually it makes (at least a little) sense to have all the format living together.

I don't think it needs to be that complex.

You already have LLDB_SOURCE_DIR from lit.site.cfg.in, so it should only be a matter of doing something like this in lit.cfg:

sys.path.append(os.path.join(config.lldb_src_root, "whereever_is_the_format_file")
from something import LLDBTest
config.test_format = LLDBTest(...)

If we can make things as simple as this, then I think we should move this to the lldb repo.

utils/lit/lit/formats/lldbtest.py

32

Do we need the suffixes to be configurable? The whole testsuite is very specific to python and I don't see us running non-python files through this method any time soon.

60

This line only appears if you run dotest in --no-multiprocess mode, but I don't see where you are adding that. Am I missing something?

[JDevlieghere]

In D45332#1059429, @labath wrote:

In D45332#1058976, @JDevlieghere wrote:

In D45332#1058970, @zturner wrote:

I don't think sys.path is set up correctly to be able to find the lldbtest package from the lldb/lit folder.

These things kind of evolved separately, and the lldb/lit folder was created as a place to start iterating on LLVM-style lit / FileCheck tests. These kind of tests -- by definition -- don't really use the SB API, so no work was ever done to set up paths correctly so that it could write import lldb or to re-use any of the other stuff from packages/Python.

I'm not sure what the best thing to do is, but usually the canonical structuring is to have the test files in the same tree as the lit configuration. So perhaps you could put a lit configuration file in lldb/packages/Python/lldbsuite and have that be separate from lldb/lit, with the goal of eventually (possibly) merging them. Then have a separate CMake target so you'd still have check-lldb-lit which goes into the lldb/lit directory, and another one like check-lldb-lit-dotest which starts from the lldb/packages/Python/lldbsuite directory.

On the other hand, if you want to see how dotest.py sets up its sys.path, have a look at lldb/test/dotest.py The magic is in this use_lldb_suite function, which walks backwards through the tree until it finds the root, then dives into the lldbsuite folder to manually add it to sys.path.

Do you feel all that outweighs the alternative of just having the format in llvm/Utils as is the case in this diff? We already have some LLDB specific stuff there and I would argue that conceptually it makes (at least a little) sense to have all the format living together.

I don't think it needs to be that complex.

You already have LLDB_SOURCE_DIR from lit.site.cfg.in, so it should only be a matter of doing something like this in lit.cfg:

sys.path.append(os.path.join(config.lldb_src_root, "whereever_is_the_format_file")
from something import LLDBTest
config.test_format = LLDBTest(...)

If we can make things as simple as this, then I think we should move this to the lldb repo.

Thanks, that did the trick :-)

utils/lit/lit/formats/__init__.py

 from lit.formats.base import (  # noqa: F401
     TestFormat,
     FileBasedTest,
     OneCommandPerFileTest
 )
 
 from lit.formats.googletest import GoogleTest  # noqa: F401
+from lit.formats.lldbtest import LLDBTest  # noqa: F401
 from lit.formats.shtest import ShTest  # noqa: F401

utils/lit/lit/formats/lldbtest.py

+from __future__ import absolute_import
+import os
+
+import subprocess
+import sys
+
+import lit.Test
+import lit.TestRunner
+import lit.util
+from .base import TestFormat
+
+
+class LLDBTest(TestFormat):
+    def __init__(self, dotest_cmd):
+        self.dotest_cmd = dotest_cmd
+
+    def getTestsInDirectory(self, testSuite, path_in_suite, litConfig,
+                            localConfig):
+        source_path = testSuite.getSourcePath(path_in_suite)
+        for filename in os.listdir(source_path):
+            # Ignore dot files and excluded tests.
+            if (filename.startswith('.') or filename in localConfig.excludes):
+                continue
+
+            # Ignore files that don't start with 'Test'.
+            if not filename.startswith('Test'):
+                continue
+
+            filepath = os.path.join(source_path, filename)
+            if not os.path.isdir(filepath):
+                base, ext = os.path.splitext(filename)
+                if ext in localConfig.suffixes:
+                    yield lit.Test.Test(testSuite, path_in_suite +
+                                        (filename, ), localConfig)
+
+    def execute(self, test, litConfig):
+        if litConfig.noExecute:
+            return lit.Test.PASS, ''
+
+        if test.config.unsupported:
+            return (lit.Test.UNSUPPORTED, 'Test is unsupported')
+
+        testPath, testFile = os.path.split(test.getSourcePath())
+        testName, testExt = os.path.splitext(testFile)
+        cmd = self.dotest_cmd + [testPath, '-p', testName]
+        print ' '.join(cmd)
+
+        try:
+            out, err, exitCode = lit.util.executeCommand(
+                cmd,
+                env=test.config.environment,
+                timeout=litConfig.maxIndividualTestTime)
+        except lit.util.ExecuteCommandTimeoutException:
+            return (lit.Test.TIMEOUT, 'Reached timeout of {} seconds'.format(
+                litConfig.maxIndividualTestTime))
+
+        if exitCode:
+            return lit.Test.FAIL, out + err
+
+        passing_test_line = 'RESULT: PASSED'
+        if passing_test_line not in out:
+            msg = ('Unable to find %r in dotest output:\n\n%s%s' %
+                   (passing_test_line, out, err))
+            return lit.Test.UNRESOLVED, msg
+
+        return lit.Test.PASS, ''