Page MenuHomePhabricator

Suggest lldb-dotest to reproduce a failure.
AbandonedPublic

Authored by JDevlieghere on May 18 2018, 6:40 AM.

Details

Summary

Rather than trying to figure out what arguments to pass to dotest.py, suggest using lldb-dotest. Athough this obviously won't work for all invocations (i.e. when you're passing custom arguments to lldb-dotest) it will work in the most common case (which is unfortunately not true today).

Diff Detail

Event Timeline

JDevlieghere created this revision.May 18 2018, 6:40 AM

Can you commit the whitespace fixes separately?

aprantl added inline comments.May 18 2018, 8:28 AM
packages/Python/lldbsuite/test/dosep.py
123–126

What do you think about adding the full path to lldb-dotest? I usually have at least three concurrent checkouts of lldb at any time.

JDevlieghere marked an inline comment as done.May 18 2018, 8:38 AM

Can you commit the whitespace fixes separately?

Sure.

packages/Python/lldbsuite/test/dosep.py
123–126

Me too, but how would we get that information here? On of lldb-dotest's goals is that you can execute it from anywhere, so you'd either have to pass it explicitly to dotest.py (either using a custom argument and cmake variable combo, or by assuming it lives in the same directory as the current lldb and appending -dotest) but that feels like it might not be worth it. Maybe there's another solution you had in mind?

aprantl added inline comments.May 18 2018, 10:47 AM
packages/Python/lldbsuite/test/dosep.py
123–126

The --executable option that dotest receives should point to an lldb that ought to have a matching lldb-dotest next to it. I don't think that that is particularly ugly.

JDevlieghere marked an inline comment as done.
  • Use absolute path to lldb-dotest when --executable is passed.
JDevlieghere marked an inline comment as done.May 20 2018, 9:02 AM

Looks fine to me, just make the path computation windows-compatible.

packages/Python/lldbsuite/test/dotest.py
386

On windows, this will end up being lldb.exe-dotest

Make this work for Windows

JDevlieghere marked 2 inline comments as done.May 21 2018, 7:59 AM

I'm going to hold off on this until we have decided what to do for D46005. If we run the test cases as separate lit invocations, then the test format is in a better position to report this information.

labath added a comment.Jun 5 2018, 4:37 AM

I'm going to hold off on this until we have decided what to do for D46005. If we run the test cases as separate lit invocations, then the test format is in a better position to report this information.

It looks like that patch has gotten a bit stuck, and I'm not sure when/if I'll have the time to unstuck it. So, if you still want to go ahead with this approach, then feel free to commit it.

JDevlieghere abandoned this revision.Mar 7 2019, 12:53 PM

Abandoning this until we can replace the dotest driver with lit.