Making tests more complete, with cleanup.

Author: rwgk

tests/test_class_sh_property.cpp

@@ -50,9 +50,11 @@ TEST_SUBMODULE(class_sh_property, m) {
         ;
 
     py::class_<ClassicOuter, std::unique_ptr<ClassicOuter>>(m, "ClassicOuter")
-        .def(py::init<>())                              //
-        .def_readwrite("m_mptr", &ClassicOuter::m_mptr) //
-        .def_readwrite("m_cptr", &ClassicOuter::m_cptr) //
+        .def(py::init<>())
+        .def_readonly("m_mptr_readonly", &ClassicOuter::m_mptr)
+        .def_readwrite("m_mptr_readwrite", &ClassicOuter::m_mptr)
+        .def_readwrite("m_cptr_readonly", &ClassicOuter::m_cptr)
+        .def_readwrite("m_cptr_readwrite", &ClassicOuter::m_cptr) //
         ;
 
     py::classh<Field>(m, "Field")          //
@@ -61,45 +63,41 @@ TEST_SUBMODULE(class_sh_property, m) {
         ;
 
     py::classh<Outer>(m, "Outer") //
-        .def(py::init<>())        //
-        .def_readonly("m_valu_readonly", &Outer::m_valu) //
-        .def_readwrite("m_valu", &Outer::m_valu) //
-        .def_property(                           //
-            "m_mptr_property",
-            [](const std::shared_ptr<Outer> &self) {
-                return std::shared_ptr<Field>(self, self->m_mptr);
-            },
-            [](Outer &self, Field *mptr) { self.m_mptr = mptr; })
-        .def_property( //
-            "m_cptr_property",
-            [](const std::shared_ptr<Outer> &self) {
-                return std::shared_ptr<const Field>(self, self->m_cptr);
-            },
-            [](Outer &self, const Field *cptr) { self.m_cptr = cptr; })
+        .def(py::init<>())
+
+        .def_readonly("m_valu_readonly", &Outer::m_valu)
+        .def_readwrite("m_valu_readwrite", &Outer::m_valu)
+
         .def_readonly("m_mptr_readonly", &Outer::m_mptr)
-        .def_readwrite("m_mptr", &Outer::m_mptr)
+        .def_readwrite("m_mptr_readwrite", &Outer::m_mptr)
         .def_readonly("m_cptr_readonly", &Outer::m_cptr)
-        .def_readwrite("m_cptr", &Outer::m_cptr)
+        .def_readwrite("m_cptr_readwrite", &Outer::m_cptr)
+
         .def_property( //
-            "m_uqmp_property",
+            "m_uqmp_readwrite",
             [](const std::shared_ptr<Outer> &self) {
                 return std::unique_ptr<Field>(std::move(self->m_uqmp));
             },
             [](Outer &self, std::unique_ptr<Field> uqmp) {
                 self.m_uqmp = std::move(uqmp); //
             })
         .def_property( //
-            "m_uqcp_property",
+            "m_uqcp_readwrite",
             [](const std::shared_ptr<Outer> &self) {
                 return std::unique_ptr<const Field>(std::move(self->m_uqcp));
             },
             [](Outer &self, std::unique_ptr<const Field> uqcp) {
                 self.m_uqcp = std::move(uqcp); //
             })
-        // .def_readwrite("m_uqmp", &Outer::m_uqmp) //
-        // .def_readwrite("m_uqcp", &Outer::m_uqcp) //
-        .def_readwrite("m_shmp", &Outer::m_shmp) //
-        .def_readwrite("m_shcp", &Outer::m_shcp) //
+        // .def_readonly("m_uqmp_readonly", &Outer::m_uqmp) // Compilation Error.
+        // .def_readwrite("m_uqmp_readwrite", &Outer::m_uqmp)
+        // .def_readonly("m_uqcp_readonly", &Outer::m_uqcp) // Compilation Error.
+        // .def_readwrite("m_uqcp_readwrite", &Outer::m_uqcp)
+
+        .def_readwrite("m_shmp_readonly", &Outer::m_shmp)
+        .def_readwrite("m_shmp_readwrite", &Outer::m_shmp)
+        .def_readwrite("m_shcp_readonly", &Outer::m_shcp)
+        .def_readwrite("m_shcp_readwrite", &Outer::m_shcp) //
         ;
 
     m.def("DisownOuter", DisownOuter);

tests/test_class_sh_property.py

@@ -6,19 +6,20 @@
 
 
 @pytest.mark.xfail("env.PYPY", reason="gc after `del field` is apparently deferred")
-def test_valu_getter(msg):
+@pytest.mark.parametrize("m_attr", ("m_valu_readonly", "m_valu_readwrite"))
+def test_valu_getter(msg, m_attr):
     # Reduced from PyCLIF test:
     # https://github.com/google/clif/blob/c371a6d4b28d25d53a16e6d2a6d97305fb1be25a/clif/testing/python/nested_fields_test.py#L56
     outer = m.Outer()
-    field = outer.m_valu
+    field = getattr(outer, m_attr)
     assert field.num == -99
     with pytest.raises(ValueError) as excinfo:
         m.DisownOuter(outer)
     assert msg(excinfo.value) == "Cannot disown use_count != 1 (loaded_as_unique_ptr)."
     del field
     m.DisownOuter(outer)
     with pytest.raises(ValueError) as excinfo:
-        outer.m_valu
+        getattr(outer, m_attr)
     assert (
         msg(excinfo.value)
         == "Missing value for wrapped C++ type: Python instance was disowned."
@@ -27,58 +28,65 @@ def test_valu_getter(msg):
 
 def test_valu_setter():
     outer = m.Outer()
-    assert outer.m_valu.num == -99
+    assert outer.m_valu_readonly.num == -99
+    assert outer.m_valu_readwrite.num == -99
     field = m.Field()
     field.num = 35
-    outer.m_valu = field
-    assert outer.m_valu.num == 35
+    outer.m_valu_readwrite = field
+    assert outer.m_valu_readonly.num == 35
+    assert outer.m_valu_readwrite.num == 35
 
 
 @pytest.mark.parametrize(
-    "field_type, num_default, outer_type, attr_suffix",  # TODO: rm attr_suffix
+    "field_type, num_default, outer_type",
     [
-        (m.ClassicField, -88, m.ClassicOuter, ""),
-        (m.Field, -99, m.Outer, ""),
+        (m.ClassicField, -88, m.ClassicOuter),
+        (m.Field, -99, m.Outer),
     ],
 )
 @pytest.mark.parametrize("m_attr", ("m_mptr", "m_cptr"))
-def test_ptr(field_type, num_default, outer_type, attr_suffix, m_attr):
-    m_attr += attr_suffix
+def test_ptr(field_type, num_default, outer_type, m_attr):
+    m_attr_readonly = m_attr + "_readonly"
+    m_attr_readwrite = m_attr + "_readwrite"
     outer = outer_type()
-    assert getattr(outer, m_attr) is None
+    assert getattr(outer, m_attr_readonly) is None
+    assert getattr(outer, m_attr_readwrite) is None
     field = field_type()
     assert field.num == num_default
-    setattr(outer, m_attr, field)
-    assert getattr(outer, m_attr).num == num_default
+    setattr(outer, m_attr_readwrite, field)
+    assert getattr(outer, m_attr_readonly).num == num_default
+    assert getattr(outer, m_attr_readwrite).num == num_default
     field.num = 76
-    assert getattr(outer, m_attr).num == 76
+    assert getattr(outer, m_attr_readonly).num == 76
+    assert getattr(outer, m_attr_readwrite).num == 76
     # Change to -88 or -99 to demonstrate Undefined Behavior (dangling pointer).
     if num_default == 88 and m_attr == "m_mptr":
         del field
-    assert getattr(outer, m_attr).num == 76
+    assert getattr(outer, m_attr_readonly).num == 76
+    assert getattr(outer, m_attr_readwrite).num == 76
 
 
-@pytest.mark.parametrize("m_attr_disown", ("m_uqmp_property", "m_uqcp_property"))
-def test_uqp(m_attr_disown, msg):
+@pytest.mark.parametrize("m_attr_readwrite", ("m_uqmp_readwrite", "m_uqcp_readwrite"))
+def test_uqp(m_attr_readwrite, msg):
     outer = m.Outer()
-    assert getattr(outer, m_attr_disown) is None
+    assert getattr(outer, m_attr_readwrite) is None
     field_orig = m.Field()
     field_orig.num = 39
-    setattr(outer, m_attr_disown, field_orig)
+    setattr(outer, m_attr_readwrite, field_orig)
     with pytest.raises(ValueError) as excinfo:
         field_orig.num
     assert (
         msg(excinfo.value)
         == "Missing value for wrapped C++ type: Python instance was disowned."
     )
-    field_retr1 = getattr(outer, m_attr_disown)
-    assert getattr(outer, m_attr_disown) is None
+    field_retr1 = getattr(outer, m_attr_readwrite)
+    assert getattr(outer, m_attr_readwrite) is None
     assert field_retr1.num == 39
     field_retr1.num = 93
-    setattr(outer, m_attr_disown, field_retr1)
+    setattr(outer, m_attr_readwrite, field_retr1)
     with pytest.raises(ValueError):
         field_retr1.num
-    field_retr2 = getattr(outer, m_attr_disown)
+    field_retr2 = getattr(outer, m_attr_readwrite)
     assert field_retr2.num == 93
 
 
@@ -108,21 +116,22 @@ def __delattr__(self, *args, **kwargs):
         return _dereference(self, delattr, *args, **kwargs)
 
 
-@pytest.mark.parametrize("m_attr_disown", ("m_uqmp_property", "m_uqcp_property"))
-def test_unique_ptr_field_proxy_poc(m_attr_disown, msg):
+@pytest.mark.parametrize("m_attr", ("m_uqmp", "m_uqcp"))
+def test_unique_ptr_field_proxy_poc(m_attr, msg):
+    m_attr_readwrite = m_attr + "_readwrite"
     outer = m.Outer()
     field_orig = m.Field()
     field_orig.num = 45
-    setattr(outer, m_attr_disown, field_orig)
-    field_proxy = unique_ptr_field_proxy_poc(outer, m_attr_disown)
+    setattr(outer, m_attr_readwrite, field_orig)
+    field_proxy = unique_ptr_field_proxy_poc(outer, m_attr_readwrite)
     assert field_proxy.num == 45
     assert field_proxy.num == 45
     with pytest.raises(AttributeError):
         field_proxy.xyz
     assert field_proxy.num == 45
     field_proxy.num = 82
     assert field_proxy.num == 82
-    field_proxy = unique_ptr_field_proxy_poc(outer, m_attr_disown)
+    field_proxy = unique_ptr_field_proxy_poc(outer, m_attr_readwrite)
     assert field_proxy.num == 82
     with pytest.raises(AttributeError):
         del field_proxy.num
@@ -131,13 +140,19 @@ def test_unique_ptr_field_proxy_poc(m_attr_disown, msg):
 
 @pytest.mark.parametrize("m_attr", ("m_shmp", "m_shcp"))
 def test_shp(m_attr):
+    m_attr_readonly = m_attr + "_readonly"
+    m_attr_readwrite = m_attr + "_readwrite"
     outer = m.Outer()
-    assert getattr(outer, m_attr) is None
+    assert getattr(outer, m_attr_readonly) is None
+    assert getattr(outer, m_attr_readwrite) is None
     field = m.Field()
     field.num = 43
-    setattr(outer, m_attr, field)
-    assert getattr(outer, m_attr).num == 43
-    getattr(outer, m_attr).num = 57
+    setattr(outer, m_attr_readwrite, field)
+    assert getattr(outer, m_attr_readonly).num == 43
+    assert getattr(outer, m_attr_readwrite).num == 43
+    getattr(outer, m_attr_readonly).num = 57
+    getattr(outer, m_attr_readwrite).num = 57
     assert field.num == 57
     del field
-    assert getattr(outer, m_attr).num == 57
+    assert getattr(outer, m_attr_readonly).num == 57
+    assert getattr(outer, m_attr_readwrite).num == 57