Page MenuHomePhabricator

Modify dotest.py to be able to run without an lldb build.
ClosedPublic

Authored by sas on Dec 5 2014, 2:37 PM.

Details

Summary

This will ease llgs development a bit by not requiring an lldb/lldb.py build to launch the tests. Also, we can now use LLDB_DEBUGSERVER_PATH to point to a debug server to use to run the tests. I used that to point to a ds2 build and run llgs tests against ds2.

Diff Detail

Repository
rL LLVM

Event Timeline

sas updated this revision to Diff 17000.Dec 5 2014, 2:37 PM
sas retitled this revision from to Modify dotest.py to be able to run without an lldb build..
sas updated this object.
sas edited the test plan for this revision. (Show Details)
sas added reviewers: clayborg, tfiala, vharron.
sas added a subscriber: Unknown Object (MLST).
compnerd requested changes to this revision.Dec 21 2014, 1:51 PM
compnerd edited edge metadata.
compnerd added inline comments.
test/dotest.py
950 ↗(On Diff #17000)

Whats the point of running tests if they are going to fail since you don't have the necessary dependencies?

1102 ↗(On Diff #17000)

Not your change, but this is equivalent to:

if frameWithVersion:
  lldbPath = before + "LLDB.framework" + after
test/tools/lldb-gdbserver/lldbgdbserverutils.py
60 ↗(On Diff #17000)

Might be nice to do a check similar to the one below to ensure that the DebugServer is present.

66 ↗(On Diff #17000)

This might be simpler to read:

for var in ("LLDB_DEBUGSERVER_PATH", "LLDB_EXEC"):
  if os.environ.get(var, None):
    return _get_debug_monitor_from_lldb(os.environ[var], "lldb-gdbserver")
return None
86 ↗(On Diff #17000)

Similar to above. This might be simpler to read:

for var in ("LLDB_DEBUGSERVER_PATH", "LLDB_EXEC"):
  if os.environ.get(var, None):
    return _get_debug_monitor_from_lldb(os.environ[var], "debugserver")
return None
87 ↗(On Diff #17000)

In fact, I think you can probably collapse both of these functions into a single one that takes a second parameter which is the debug monitor name and pass that along to the _get_debug_monitor_from_lldb.

This revision now requires changes to proceed.Dec 21 2014, 1:51 PM
sas added a comment.Dec 22 2014, 11:39 AM

Thanks for the review Saleem. See comments inline.

test/dotest.py
950 ↗(On Diff #17000)

Because you can run other tests (not lldb) with this tool. Only the lldb tests will fail if you choose to run them.

test/tools/lldb-gdbserver/lldbgdbserverutils.py
60 ↗(On Diff #17000)

The existence of the debug server is checked elsewhere in the test suite (in lldbgdbserverutils.py).

66 ↗(On Diff #17000)

Not really, because when the user specifies LLDB_DEBUGSERVER_PATH, we don't want to go through _gert_debug_monitor_from_lldb again, we just want to use the path directly.

86 ↗(On Diff #17000)

Ditto.

compnerd added inline comments.Jan 7 2015, 6:02 PM
test/dotest.py
950 ↗(On Diff #17000)

Not sure I understand. What are these other tests?

test/tools/lldb-gdbserver/lldbgdbserverutils.py
66 ↗(On Diff #17000)

Why do we want the different behavior for the two?

sas added inline comments.Jan 8 2015, 12:08 AM
test/dotest.py
950 ↗(On Diff #17000)

lldb-mi and llgs for instance.

test/tools/lldb-gdbserver/lldbgdbserverutils.py
66 ↗(On Diff #17000)

The function name is pretty explicit: _get_debug_monitor_from_lldb gets the debug monitor path by using the lldb path provided. If the user already specified a path to a debug monitor, why would we go through a function that tries to apply intelligence to guess it?

compnerd accepted this revision.Jan 8 2015, 8:23 PM
compnerd edited edge metadata.
This revision is now accepted and ready to land.Jan 8 2015, 8:23 PM
This revision was automatically updated to reflect the committed changes.