Skip to content

Commit

Permalink
Add unit test for DM-21314.
Browse files Browse the repository at this point in the history
The garbage collection bug is a significant restriction on Storable,
and providing a non-passing unit test makes this fact more
self-documenting.
  • Loading branch information
kfindeisen committed Sep 20, 2019
1 parent e540aac commit d7581c2
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 0 deletions.
20 changes: 20 additions & 0 deletions tests/testGenericMapLib.cc
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,25 @@ void addCppStorable(MutableGenericMap<std::string>& testmap) {
testmap.insert("cppPointer", std::make_shared<CppStorable>("pointer"));
}

/**
* Store and retrieve a Storable in C++.
*
* To avoid cluttering testGenericMapLib with a special holder class, this
* function conflates two actions. When called with a Storable, it puts a
* pointer to it in internal storage. When called with no arguments, it
* returns the pointer it has been keeping.
*
* @param storable The Storable to assign to the pointer [optional].
* @returns The currently held pointer.
*/
std::shared_ptr<Storable> keepStaticStorable(std::shared_ptr<Storable> storable = nullptr) {
static std::shared_ptr<Storable> longLived = nullptr;
if (storable) {
longLived = storable;
}
return longLived;
}

} // namespace

namespace {
Expand Down Expand Up @@ -309,6 +328,7 @@ PYBIND11_MODULE(testGenericMapLib, mod) {
mod.def("makeInitialMap", &makeInitialMap);
mod.def("makeCppUpdates", &makeCppUpdates, "testmap"_a);
mod.def("addCppStorable", &addCppStorable, "testmap"_a);
mod.def("keepStaticStorable", &keepStaticStorable, "storable"_a = nullptr);

py::class_<CppStorable, std::shared_ptr<CppStorable>, Storable> cls(mod, "CppStorable");
cls.def(py::init<std::string>());
Expand Down
12 changes: 12 additions & 0 deletions tests/test_Storable.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
# along with this program. If not, see <https://www.gnu.org/licenses/>.

import copy
import gc
import unittest

import lsst.utils.tests
Expand Down Expand Up @@ -88,6 +89,17 @@ def testEq(self):
self.assertEqual(self.testbed, DemoStorable([42]))
self.assertNotEqual(self.testbed, DemoStorable(0))

@unittest.skip("TODO: Fix this bug in DM-21314")
def testGarbageCollection(self):
cppLib.keepStaticStorable(DemoStorable(3))

gc.collect()

retrieved = cppLib.keepStaticStorable()
self.assertIsInstance(retrieved, Storable)
self.assertIsInstance(retrieved, DemoStorable)
self.assertEqual(retrieved, DemoStorable(3))


class CppStorableTestSuite(lsst.utils.tests.TestCase):

Expand Down

0 comments on commit d7581c2

Please sign in to comment.