[Lldb-commits] [PATCH] D45332: [LIT] Add new LLDB test format

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere abandoned this revision.
JDevlieghere added a comment.

In https://reviews.llvm.org/D45332#1059429, @labath wrote:

> In https://reviews.llvm.org/D45332#1058976, @JDevlieghere wrote:
>
> > In https://reviews.llvm.org/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 :-)


Repository:
  rL LLVM

https://reviews.llvm.org/D45332