[Lldb-commits] [PATCH] D70622: [cmake][lit] Follow symlinks when looking for lit tests & reconfigure lldb's tests.

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht abandoned this revision.
rupprecht added a comment.

Officially abandoning this

rG0d236d8b4f8aecc258e26ad53755a39d9b76032e 
 - fixes 
makefiles when separated from `Makefile.rules`
rG03a242bd41ee49e17f8da96af9787d13e7ea2b93 
 - fixes 
`.categories` lookup when tests are moved

I have one more to split up `os.environ["LLDB_TEST"]` into separate trees for 
test framework/test source, then the tests can be moved in a very mechanical 
way. (It will create heavy merge conflicts, I want it to be trivial to apply 
for downstream users).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70622/new/

https://reviews.llvm.org/D70622



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70622: [cmake][lit] Follow symlinks when looking for lit tests & reconfigure lldb's tests.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for looking into this, Jordan.

I'm not sure how hard it is, but I think it would be nice to not have the 
actual tests intermingled with the test framework stuff.

(Also, another reason for moving the tests is that the symlink trick will just 
not work on windows.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70622/new/

https://reviews.llvm.org/D70622



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70622: [cmake][lit] Follow symlinks when looking for lit tests & reconfigure lldb's tests.

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht added a comment.

In D70622#1771601 , @rupprecht wrote:

> In D70622#1765674 , @JDevlieghere 
> wrote:
>
> > No objections to moving the test. Jordan, let me know if you're up for it, 
> > otherwise I'm happy to take care of it.
>
>
> I tried moving it today. The rabbit hole goes deep :D
>  The test framework seems pretty tied to the directory layout (because 
> python), e.g. those must be in a directory called "lldbsuite" to be able to 
> "import lldbsuite"
>  The test infrastructure also seems to assume tests + test framework are in 
> the same tree (e.g. the LLDB_TEST env var is used for both), so moving the 
> individual test directories and leaving just the framework classes in 
> lldbsuite breaks a few things.
>
> I agree it's probably cleaner to move the tests, so I'll keep poking. 
> Suggestions welcome though.


I have it down to ~5 failing tests (some may be preexisting failures), so I'll 
start pulling apart my change into submittable pieces.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70622/new/

https://reviews.llvm.org/D70622



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70622: [cmake][lit] Follow symlinks when looking for lit tests & reconfigure lldb's tests.

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht added a comment.

In D70622#1765674 , @JDevlieghere 
wrote:

> No objections to moving the test. Jordan, let me know if you're up for it, 
> otherwise I'm happy to take care of it.


I tried moving it today. The rabbit hole goes deep :D
The test framework seems pretty tied to the directory layout (because python), 
e.g. those must be in a directory called "lldbsuite" to be able to "import 
lldbsuite"
The test infrastructure also seems to assume tests + test framework are in the 
same tree (e.g. the LLDB_TEST env var is used for both), so moving the 
individual test directories and leaving just the framework classes in lldbsuite 
breaks a few things.

I agree it's probably cleaner to move the tests, so I'll keep poking. 
Suggestions welcome though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70622/new/

https://reviews.llvm.org/D70622



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70622: [cmake][lit] Follow symlinks when looking for lit tests & reconfigure lldb's tests.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

No objections to moving the test. Jordan, let me know if you're up for it, 
otherwise I'm happy to take care of it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70622/new/

https://reviews.llvm.org/D70622



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70622: [cmake][lit] Follow symlinks when looking for lit tests & reconfigure lldb's tests.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Personally I'd prefer moving the test cases, but maybe there's a good 
historical reason for the current layout. I'll ask around during out team 
meeting later today and report back.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70622/new/

https://reviews.llvm.org/D70622



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70622: [cmake][lit] Follow symlinks when looking for lit tests & reconfigure lldb's tests.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for looking into this. This is something I've missed too, but never got 
around to checking it out.

Jonas, what do you think about this? Should we do this, or move the testcases 
themselves into the "test/API" folder?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70622/new/

https://reviews.llvm.org/D70622



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70622: [cmake][lit] Follow symlinks when looking for lit tests & reconfigure lldb's tests.

[pre-merge checks [bot] via Phabricator via lldb-commits]

merge_guards_bot added a comment.

Build result: pass - 60250 tests passed, 0 failed and 732 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70622/new/

https://reviews.llvm.org/D70622



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70622: [cmake][lit] Follow symlinks when looking for lit tests & reconfigure lldb's tests.

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht created this revision.
rupprecht added reviewers: JDevlieghere, labath.
Herald added subscribers: llvm-commits, lldb-commits, mgorny.
Herald added projects: LLDB, LLVM.

The lldb tree contains a symlink directory, `lldb/test/API/testcases`, which 
points to the tests in `../../packages/Python/lldbsuite/test`. Because the 
`add_lit_testsuites()` cmake rule does not follow symlinks, it doesn't find the 
nested directory tree to create make/ninja rules for.

Also, because the lldb lit config points the source root directly into the 
testcases tree, the `check-lldb-api-testcases` rule fails to find the 
`testcases` directory, and therefore finds no tests. Changing the source root 
to the containing lit file, like most lit configs do, fixes this.

After this patch, `ninja check-lldb-api-testcases-linux` works, and runs only 
the test cases in `lldb/packages/Python/lldbsuite/test/linux`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70622

Files:
  lldb/test/API/lit.cfg.py
  llvm/cmake/modules/AddLLVM.cmake


Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -1488,7 +1488,8 @@
 
 # Search recursively for test directories by assuming anything not
 # in a directory called Inputs contains tests.
-file(GLOB_RECURSE to_process LIST_DIRECTORIES true ${directory}/*)
+# Follow symlinks as some tests in lldb are structured that way.
+file(GLOB_RECURSE to_process FOLLOW_SYMLINKS LIST_DIRECTORIES true 
${directory}/*)
 foreach(lit_suite ${to_process})
   if(NOT IS_DIRECTORY ${lit_suite})
 continue()
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -17,8 +17,7 @@
 
 # test_source_root: The root path where tests are located.
 # test_exec_root: The root path where tests should be run.
-config.test_source_root = os.path.join(config.lldb_src_root, 'packages',
-   'Python', 'lldbsuite', 'test')
+config.test_source_root = os.path.dirname(__file__)
 config.test_exec_root = config.test_source_root
 
 if 'Address' in config.llvm_use_sanitizer:


Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -1488,7 +1488,8 @@
 
 # Search recursively for test directories by assuming anything not
 # in a directory called Inputs contains tests.
-file(GLOB_RECURSE to_process LIST_DIRECTORIES true ${directory}/*)
+# Follow symlinks as some tests in lldb are structured that way.
+file(GLOB_RECURSE to_process FOLLOW_SYMLINKS LIST_DIRECTORIES true ${directory}/*)
 foreach(lit_suite ${to_process})
   if(NOT IS_DIRECTORY ${lit_suite})
 continue()
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -17,8 +17,7 @@
 
 # test_source_root: The root path where tests are located.
 # test_exec_root: The root path where tests should be run.
-config.test_source_root = os.path.join(config.lldb_src_root, 'packages',
-   'Python', 'lldbsuite', 'test')
+config.test_source_root = os.path.dirname(__file__)
 config.test_exec_root = config.test_source_root
 
 if 'Address' in config.llvm_use_sanitizer:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits