Repository: aidotse/PASEOS (language: python)

Issue:
Termination function is executed twice if the constraint function has no return value
# Issue
## Problem Description
If the constraint function has no ~~argument~~ return value, the termination function might be executed twice, because activity_runner.stop() might be called twice.

## Expected Behavior
Termination shall be executed once.

## What Needs to be Done
~~Manage return values of constraint_function is activity_runner differently.~~
- [ ] Check whether constraint function return true or false. If not raise exception
- [ ] Double check termination is only called once then

## How Can It Be Tested or Reproduced
~~Current version of the example notebook in #38 will print "TERMINATION" twice if no return value is included in the constrain function.~~
- [ ] Add a test with a broken constraint function and check it raises an exception



Discussion / hints:
> if no return value is included in the constrain function.
this is however the user misdefining the constraint function. We should check that though somehow 🤔 (ideally without unnecessarily executing the constraint function to avoid computational overhead)
@GabrieleMeoni Is this still open or fixed?
It is fixed until the user returns a value in the constraint function. Otherwise, the problem is still there.  I haven't checked what happens when the function returns false. Please, doublecheck this and open an issue to add a check that the return value is not None.
I'll change the issue accordingly then

Fix patch:
diff --git a/paseos/activities/activity_runner.py b/paseos/activities/activity_runner.py
index 9bebc969..0e9a6196 100644
--- a/paseos/activities/activity_runner.py
+++ b/paseos/activities/activity_runner.py
@@ -96,22 +96,28 @@ async def check_constraint(self):
             bool: True if still valid.
         """
         if self.has_constraint():
-            logger.trace(f"Checking activity {self.name} constraints")
+            logger.debug(f"Checking activity {self.name} constraints")
             try:
                 is_satisfied = await self._constraint_func(self._constraint_args)
+
+                if is_satisfied is None:
+                    logger.error(
+                        f"An exception occurred running the checking the activity {self.name} constraint."
+                        + " The constraint function failed to return True or False."
+                    )
             except Exception as e:
                 logger.error(
-                    f"An exception occurred running the checking the activity's {self.name} constraint."
+                    f"An exception occurred running the checking the activity {self.name} constraint."
                 )
                 logger.error(str(e))
                 return False
-            if not is_satisfied or is_satisfied is None:
+            if not is_satisfied:
                 logger.debug(
                     f"Constraint of activity {self.name} is no longer satisfied, cancelling."
                 )
                 if not self._was_stopped:
                     await self.stop()
-                return False
+            return False
         else:
             logger.warning(
                 f"Checking activity {self.name} constraints even though activity has no constraints."


Test patch:
diff --git a/paseos/tests/activity_test.py b/paseos/tests/activity_test.py
index 2f6ea5a4..d73abd66 100644
--- a/paseos/tests/activity_test.py
+++ b/paseos/tests/activity_test.py
@@ -11,6 +11,36 @@ async def wait_for_activity(sim):
         await asyncio.sleep(0.1)
 
 
+# Below test can be used to check what happens when you formulate an invalid constraint function.
+# It is temporarily commented out as it doesn't really check right now because I could not figure
+# out a way to get an exception raised from the async code.
+# @pytest.mark.asyncio
+# async def test_faulty_constraint_function():
+#     """Check whether specifying a wrong constraint function leads to an error"""
+
+#     sim, _, _ = get_default_instance()
+
+#     # A pointless activity
+#     async def func(args):
+#         await asyncio.sleep(1.5)
+
+#     # A constraint function that fails to return True or False
+#     async def constraint(args):
+#         pass
+
+#     # Register an activity that draws 10 watt per second
+#     sim.register_activity(
+#         "Testing",
+#         activity_function=func,
+#         constraint_function=constraint,
+#         power_consumption_in_watt=10,
+#     )
+
+#     # Run the activity
+#     sim.perform_activity("Testing")
+#     await wait_for_activity(sim)
+
+
 # tell pytest to create an event loop and execute the tests using the event loop
 @pytest.mark.asyncio
 async def test_activity():