This is an archive of the discontinued LLVM Phabricator instance.

[lldb-server unittest] Add missing teardown logic
ClosedPublic

Authored by aadsm on Jun 1 2019, 9:41 PM.

Details

Summary

This test base class is missing the teardown making the second set of tests extending it to fail in an assertion in the FileSystem::Initialize() (as it's being initialized twice).
Not sure why this isn't failing the build bots.. (unless they're running without asserts?).

With this fix ninja LLDBServerTests && ./tools/lldb/unittests/tools/lldb-server/tests/LLDBServerTests successfully runs and passes all tests.

Diff Detail

Repository
rL LLVM

Event Timeline

aadsm created this revision.Jun 1 2019, 9:41 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 1 2019, 9:41 PM
xiaobai accepted this revision.Jun 1 2019, 9:50 PM

Thanks for fixing this.

This revision is now accepted and ready to land.Jun 1 2019, 9:50 PM
labath accepted this revision.Jun 3 2019, 12:51 AM

Not sure why this isn't failing the build bots.. (unless they're running without asserts?).

The build bots (and lit, in general) runs each of the unit tests in isolation (so it first gets a list of all tests, then runs the executable with --gtest_filter=TestName for each of the tests in turn). This means issues like this don't show up there.

However, it's good to fix them nonetheless.

labath added a comment.Jun 3 2019, 1:05 AM

I should add (since I'm guessing you found this problem while trying to add unit tests for your lldb-server changes) a bit of history about these tests. There are two kinds of tests for lldb-server. The python tests (test/testcases/tools/lldb-server) are older ones. The c++ ones were added in an attempt to fix the issues which made the python tests hard to understand. However, the person who was doing that left the project, so the were never quite finished. In fact, I've been lately considering just removing them and instead to try to fix the python issues incrementally in-place.

I don't think this has any immediate impact on what you're doing now -- feel free to use whichever method you find the easiest to write the tests you need. But if, after doing that, you have any feedback about one test method or the other, I'd very much like to hear it.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 3 2019, 8:15 AM