Fix up tests except for actually failing one

Author: znicholls

src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90

@@ -33,7 +33,7 @@ subroutine get_derived_type_free_instance_number(instance_index, n_instances, in
         class(BaseFinalisable), dimension(n_instances), intent(inout) :: instance_array
         !! Array of instances
 
-        integer :: i
+        integer :: i = 1
 
         ! Default if no available models are found
         instance_index = invalid_instance_index

tests/unit/test_error_v.py

@@ -20,31 +20,49 @@ def test_build_finalise():
 
 
 def test_build_finalise_multiple_instances_same_index():
-    pytest.skip("TODO: turn back on")
     inst = ErrorV.from_build_args(code=2, message="Hello world")
 
+    original_instance_index = inst.instance_index
+
     assert inst.code == 2
     assert inst.message == "Hello world"
 
-    inst_same_ptr = ErrorV(inst.instance_ptr)
+    inst_same_index = ErrorV(inst.instance_index)
 
-    assert inst_same_ptr.code == 2
-    assert inst_same_ptr.message == "Hello world"
+    assert inst_same_index.code == 2
+    assert inst_same_index.message == "Hello world"
     assert inst.initialised
-    assert inst.is_associated
 
     inst.finalise()
-    inst_same_ptr.finalise()
+    # # Currently this causes a hard stop.
+    # # That's the right behaviour.
+    # # We will make it not be a hard fail when we switch to result types.
+    # inst_same_index.finalise()
     assert not inst.initialised
-    assert not inst.is_associated
-
-    # Fun mess you can get yourself into if you use the same pointer
-    # and it is finalised elsewhere
-    assert inst_same_ptr.code != 2
-    # This is true as the Python has no way of knowing
-    # that it has been finalised elsewhere
-    # (you basically need rust's borrow checker to not stuff this up)
-    assert inst_same_ptr.initialised
-    # This is how you can check for actual association
-    # (given this, maybe want to change how `initialised` works)
-    assert not inst_same_ptr.is_associated
+
+    ### Problem 1 ###
+    # # With the current implementation,
+    # # finalising via `inst` does not cause `inst_same_index` to also be finalised
+    # assert not inst_same_index.initialised
+
+    inst_new = ErrorV.from_build_args(code=3, message="Didn't expect this")
+    # New instance uses the newly freed index.
+    assert inst_new.instance_index == original_instance_index
+    # Which means the new instance and the instance
+    # which was initialised previously now have the same instance index
+    assert inst_new.instance_index == inst_same_index.instance_index
+
+    ### Problem 2 ###
+    # So, if we look at `inst_same_index`'s attribute values, we get a surprise
+    # The code isn't 2 as was set above.
+    # Instead, the value has changed 'in the background' i.e. our view is 'stale'.
+    assert inst_same_index.code == 3
+    assert inst_same_index.message == "Didn't expect this"
+
+    # Something like this is what we actually want to happen
+    # (if we try to access via inst_same_index again,
+    # we get told that our view is out of date).
+    # Could be warning or error, not sure what makes more sense...
+    with pytest.raises(StaleViewError):
+        inst_same_index.code
+        inst_same_index.message

tests/unit/test_error_v_creation.py

@@ -6,6 +6,8 @@
 (deliberately, just to see how it goes).
 """
 
+import re
+
 import pytest
 from IPython.lib.pretty import pretty
 
@@ -54,6 +56,9 @@ def test_error_too_many_instances():
         create_error(1)
 
 
+@pytest.mark.xfail(
+    reason="Not implemented yet - do in a future PR once we have a result type"
+)
 def test_increase_number_of_instances():
     raise NotImplementedError
     # - Make 4096 instances
@@ -66,16 +71,22 @@ def test_increase_number_of_instances():
 def test_error_str(file_regression):
     res = create_error(1.0)
 
-    file_regression.check(str(res))
+    # Don't worry about the value of instance_index
+    res_check = re.sub(r"instance_index=\d*", "instance_index=n", str(res))
+    file_regression.check(res_check)
 
 
 def test_error_pprint(file_regression):
     res = create_error(1.0)
 
-    file_regression.check(pretty(res))
+    # Don't worry about the value of instance_index
+    res_check = re.sub(r"instance_index=\d*", "instance_index=n", pretty(res))
+    file_regression.check(res_check)
 
 
 def test_error_html(file_regression):
     res = create_error(1.0)
 
-    file_regression.check(res._repr_html_(), extension=".html")
+    # Don't worry about the value of instance_index
+    res_check = re.sub(r"instance_index=\d*", "instance_index=n", res._repr_html_())
+    file_regression.check(res_check, extension=".html")

tests/unit/test_error_v_creation/test_error_html.html

@@ -31,7 +31,7 @@
   <div class='fgen-wrap'>
     <div class='fgen-header'>
       <div class='fgen-basefinalizable-cls'>ErrorV</div>
-        <div class='fgen-basefinalizable-instance-index'>instance_index=6</div>
+        <div class='fgen-basefinalizable-instance-index'>instance_index=n</div>
         <table><tbody>
           <tr><th>code</th><td style='text-align:left;'>0</td></tr>
           <tr><th>message</th><td style='text-align:left;'></td></tr>

tests/unit/test_error_v_creation/test_error_pprint.txt

@@ -1 +1 @@
-ErrorV(instance_index=5, code=0, message=)
+ErrorV(instance_index=n, code=0, message=)

tests/unit/test_error_v_creation/test_error_str.txt

@@ -1 +1 @@
-ErrorV(instance_index=4, code=0, message=)
+ErrorV(instance_index=n, code=0, message=)