Page MenuHomePhabricator

[lldb/Test] Assert that no targets or global modules remain after a test completes.
ClosedPublic

Authored by JDevlieghere on Jun 10 2020, 1:51 PM.

Details

Summary

The reproducer intentionally leak every object allocated during replay, which means that modules never get orphaned. If this were to happen for another reason, we might not be testing what we think we are. Assert that there are no targets left at the end of a test and that the global module cache is empty in the non-reproducer scenario.

Diff Detail

Repository
rLLDB LLDB

Event Timeline

JDevlieghere created this revision.Jun 10 2020, 1:51 PM
vsk added inline comments.Jun 10 2020, 4:27 PM
lldb/bindings/interface/SBModule.i
347

Can we add a %feature("docstring", ...) blurb about this, advising script authors that it's probably not an API they're interested in?

Address @vsk's feedback.

JDevlieghere marked an inline comment as done.Jun 10 2020, 6:17 PM

I'm pretty indifferent about this functionality -- I don't think it hurts, but it also doesn't seem like a pressing problem that needs addressing.

Regarding the implementation, be aware that assertion failures during test teardown are reproted pretty weirdly -- IIRC at this point the test has already been declared "successful", and these failures manifest as "CLEANUP ERROR"s somewhere. And I have a feeling they don't actually fail the check-lldb command. I don't know if this is due to something we've done, or if it's just how the python unittest framework works...

I'm pretty indifferent about this functionality -- I don't think it hurts, but it also doesn't seem like a pressing problem that needs addressing.

Yeah. I agree it's not a problem we face right now, but the reproducer scenario showed that we can get into a situation where we're not testing what we thing we are. I took me and Jim a little time to understand why that was happening and I think this is a small price to pay to avoid that in the future and guarantee some sanity.

Regarding the implementation, be aware that assertion failures during test teardown are reproted pretty weirdly -- IIRC at this point the test has already been declared "successful", and these failures manifest as "CLEANUP ERROR"s somewhere. And I have a feeling they don't actually fail the check-lldb command. I don't know if this is due to something we've done, or if it's just how the python unittest framework works...

Okay, I was on the fence between using the unittest2 assert method or the built in Python assertion. The latter might be a better fit.

vsk accepted this revision.Jun 12 2020, 1:48 PM

Lgtm.

This revision is now accepted and ready to land.Jun 12 2020, 1:48 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 12 2020, 3:24 PM