What is the policy for using lsst.utils.tests.MemoryTestCase?

Data Management
#1

I keep hearing about lsst.utils.tests.MemoryTestCase?, but I don’t understand the need for this test in Python garbage-collected code.

Is there documentation available about the recommend use of

lsst.utils.tests.MemoryTestCase

Is there at least an undocumented consensus about its use?

http://developer.lsst.io/en/latest/coding/unit_test_policy.html
seems to be at a higher level and doesn’t seem to cover this. A search for MemoryTestCase at developer.lsst.io didn’t find any matches.

#2

I too am curious. The only time I see it trigger is when I use pdb to set tracepoints, and then the memory tests usually fail.

#3

It checks that objects allocated at the C++ level don’t leak memory. The python reference count will remove the top level object, but that’s not enough.

However, our extensive use of smart pointers (mostly shared_ptr, but we should be moving to unique_ptr) have meant that we have had very little trouble with C++ level leaks, so I can see why this appears unnecessary.

#4

Do you assert, claim, suggest that it should be used?

I’m really totally fine including it, I deeply don’t care. But if it’s the recommendation, then I would ask that it be documented, likely under the testing documentation page mentioned above:

http://developer.lsst.io/en/latest/coding/unit_test_policy.html

#5

I would suggest that we continue to use it. And that page be updated.

#6

DM-5156.

#7

Is this what y’all had in mind?

#8

I realise that ticket’s out to me for review, so perhaps I should comment on JIRA or GitHub. But anyway, my first thought is – a bit more detail would be nice. For example:

  • Should one always use MemoryTestCase, or only when you’re explicitly importing Swigged C++? The example doesn’t seem to have any C++ in it all.
  • How does one use it properly? My experience has been that we end up with tearDown methods with a whole bunch of dels in them just to keep MemoryTestCase happy. Is that the intention? Is there a rule of thumb for what you actually need to delete vs leaving for the garbage collector?
  • How does it actually work, anyway?

(The last point there is going beyond what DM-5156 rather narrowly asked for.)

#9

In that case we should re-assign DM-5156 to a domain expert who wrote MemoryTestCase or has experience using it.

#10

I don’t disagree with that – I believe it was assigned to you (@jsick) because the component was set to “Stack Documentation and UX”, rather than because you were carefully chosen as the most appropriate person to document it.

That said, what you’ve written is a vast improvement over the complete lack of information that we had before, so I’m inclined to say we should still go ahead and merge it. But that’s a discussion for JIRA…