Page MenuHomePhabricator

[lldb][test] Remove symlink for API tests.
ClosedPublic

Authored by rupprecht on Dec 6 2019, 3:03 PM.

Details

Summary

This is a largely mechanical change, moved with the following steps:

rm lldb/test/API/testcases
mkdir -p lldb/test/API/{test_runner/test,tools/lldb-{server,vscode}}
mv lldb/packages/Python/lldbsuite/test/test_runner/test lldb/test/API/test_runner
for d in $(find lldb/packages/Python/lldbsuite/test/* -maxdepth 0 -type d | egrep -v "make|plugins|test_runner|tools"); do mv $d lldb/test/API; done
for d in $(find lldb/packages/Python/lldbsuite/test/tools/lldb-vscode -maxdepth 1 -mindepth 1 | grep -v ".py"); do mv $d lldb/test/API/tools/lldb-vscode; done
for d in $(find lldb/packages/Python/lldbsuite/test/tools/lldb-server -maxdepth 1 -mindepth 1 | egrep -v "gdbremote_testcase.py|lldbgdbserverutils.py|socket_packet_pump.py"); do mv $d lldb/test/API/tools/lldb-server; done

lldb/packages/Python/lldbsuite/__init__.py and lldb/test/API/lit.cfg.py were also updated with the new directory structure.

Diff Detail

Event Timeline

rupprecht created this revision.Dec 6 2019, 3:03 PM
Herald added a reviewer: jfb. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript

Since the diff is large, I'll paste the only non-mechanical part here:

% git diff HEAD^1 -- lldb/packages/Python/lldbsuite/__init__.py lldb/test/API/lit.cfg.py
diff --git a/lldb/packages/Python/lldbsuite/__init__.py b/lldb/packages/Python/lldbsuite/__init__.py
index c98c668ed6b..188f6ed1532 100644
--- a/lldb/packages/Python/lldbsuite/__init__.py
+++ b/lldb/packages/Python/lldbsuite/__init__.py
@@ -22,11 +22,4 @@ lldb_root = find_lldb_root()
 
 # lldbsuite.lldb_test_src_root refers to the root of the python test case tree
 # (i.e. the actual unit tests).
-lldb_test_root = os.path.join(
-    lldb_root,
-    "packages",
-    "Python",
-    "lldbsuite",
-    "test")
-# TODO(rupprecht): update the above definition after moving test cases:
-# lldb_test_root = os.path.join(lldb_root, "test", "API", "test")
+lldb_test_root = os.path.join(lldb_root, "test", "API", "test")
diff --git a/lldb/test/API/lit.cfg.py b/lldb/test/API/lit.cfg.py
index 9b1c3c12f17..90d8a141e9a 100644
--- a/lldb/test/API/lit.cfg.py
+++ b/lldb/test/API/lit.cfg.py
@@ -17,8 +17,7 @@ config.suffixes = ['.py']
 
 # 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:

Build result: pass - 60571 tests passed, 0 failed and 726 were skipped.

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

Why don't we move the lldbsuite as well?

While I'm not opposed to moving "lldbsuite" to some place closer to the tests, one thing I'd like to get out of this is to have the actual tests stand out more prominently. For that reason, I wouldn't want to just take the "lldbsuite" folder and "drop" it into test/API. In fact, ideally I'd like to get rid of the redundant nested test subfolder, and put the tests directly into test/API.

However, like I said in the other review, if lua is going to become a thing, we should evaluate how will this layout work with additional scripting languages: I see several options here:

  • make a subfolder for each language (test/API/{Python,Lua})
  • ditch the "API" part and put these at the top level (test/{Python,Lua})
  • keep Python in "API", but put lua in Lua (test/{API,Lua}) -- besides backward compat, this would kind of signal that "python" is our preferred vessel for writing tests
  • ???

While I'm not opposed to moving "lldbsuite" to some place closer to the tests, one thing I'd like to get out of this is to have the actual tests stand out more prominently.

Ditto; my direct goal is just to be able to run something more fine grained than ninja check-lldb-api, but splitting tests & test framework is a nice side benefit. (Actually it might be simpler if I didn't split it, but now I've already done the work).

For that reason, I wouldn't want to just take the "lldbsuite" folder and "drop" it into test/API. In fact, ideally I'd like to get rid of the redundant nested test subfolder, and put the tests directly into test/API.

SGTM. I can adjust that paths so there is no shim "test" directory. Will await resolution of the specific paths as chosen below:

However, like I said in the other review, if lua is going to become a thing, we should evaluate how will this layout work with additional scripting languages: I see several options here:

  • make a subfolder for each language (test/API/{Python,Lua})
  • ditch the "API" part and put these at the top level (test/{Python,Lua})
  • keep Python in "API", but put lua in Lua (test/{API,Lua}) -- besides backward compat, this would kind of signal that "python" is our preferred vessel for writing tests
  • ???

Any of these options are fine with me. Actually, they seem most logical in the order you've listed -- the last one seems kind of weird that "API" implicitly means python, and there are other non-python API tests elsewhere.

labath accepted this revision.Jan 10 2020, 2:46 AM

Judging by the direction of the lua patches, it doesn't look like we will have lua "dotest" tests, so I think it's fine to just put this into API directly (without the extra "test" subfolder").

This revision is now accepted and ready to land.Jan 10 2020, 2:46 AM
rupprecht edited the summary of this revision. (Show Details)Feb 7 2020, 3:49 PM

Judging by the direction of the lua patches, it doesn't look like we will have lua "dotest" tests, so I think it's fine to just put this into API directly (without the extra "test" subfolder").

Updated the recipe in the description w/o the test subfolder.

Since this is a pretty large (though mechanical) change, I'll ping lldb-dev Monday to put it on peoples' radars.

rupprecht edited the summary of this revision. (Show Details)Feb 10 2020, 8:41 AM
This revision was automatically updated to reflect the committed changes.