From 9536ca738ec8de419d3f00a216979dad85de269e Mon Sep 17 00:00:00 2001
From: Andrew Nicols <andrew@nicols.co.uk>
Date: Wed, 16 Oct 2024 22:39:49 +0800
Subject: [PATCH] MDL-83468 phpunit: Do not throw exception in mocked
 destructor

PHPUnit removed the ability to mock a destructor, but our lock system
throws an exception if a lock has not been explicitly released in its
destructor.

Normally thhis is fine because the lock is released, and if not then we
want to know about it.

However, where we are mocking the lock, we do not actually obtain the
lock, and we may expect the test to fail.

This change moves the release and notification to a separate, reusable
public method, which is called from the destructor. This allows it to be
mocked at the appropriate time.
---
 lib/classes/lock/lock.php                 | 34 +++++++++++++++++------
 lib/phpunit/classes/advanced_testcase.php |  4 +--
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/lib/classes/lock/lock.php b/lib/classes/lock/lock.php
index f17abdaf4d1..44f78f60be6 100644
--- a/lib/classes/lock/lock.php
+++ b/lib/classes/lock/lock.php
@@ -106,19 +106,35 @@ class lock {
     }
 
     /**
-     * Print debugging if this lock falls out of scope before being released.
+     * Release a lock if it has not already been released.
+     *
+     * @param bool $withexception If true, throw an exception if the lock has not been released.
+     * @throws \coding_exception
      */
-    public function __destruct() {
-        if (!$this->released && defined('PHPUNIT_TEST')) {
+    public function release_if_not_released(bool $withexception = false): void {
+        if (!$this->released) {
             $key = $this->key;
+
             $this->release();
-            throw new \coding_exception("A lock was created but not released at:\n" .
-                                        $this->caller . "\n\n" .
-                                        " Code should look like:\n\n" .
-                                        " \$factory = \core\lock\lock_config::get_lock_factory('type');\n" .
-                                        " \$lock = \$factory->get_lock($key);\n" .
-                                        " \$lock->release();  // Locks must ALWAYS be released like this.\n\n");
+
+            if ($withexception) {
+                throw new \core\exception\coding_exception(<<<EOF
+                    A lock was created but not released at: {$this->caller}
+
+                    Code should look like:
+
+                        \$factory = \core\lock\lock_config::get_lock_factory('type');
+                        \$lock = \$factory->get_lock($key);
+                        \$lock->release();  // Locks must ALWAYS be released like this.
+                EOF);
+            }
         }
     }
 
+    /**
+     * Print debugging if this lock falls out of scope before being released.
+     */
+    public function __destruct() {
+        $this->release_if_not_released(defined('PHPUNIT_TEST'));
+    }
 }
diff --git a/lib/phpunit/classes/advanced_testcase.php b/lib/phpunit/classes/advanced_testcase.php
index 19667934d6c..9c4d6488cc8 100644
--- a/lib/phpunit/classes/advanced_testcase.php
+++ b/lib/phpunit/classes/advanced_testcase.php
@@ -685,8 +685,8 @@ abstract class advanced_testcase extends base_testcase {
             $params['userid'] = $matchuserid;
         }
 
-        $lock = $this->createMock(\core\lock\lock::class);
-        $cronlock = $this->createMock(\core\lock\lock::class);
+        $lock = $this->createStub(\core\lock\lock::class);
+        $cronlock = $this->createStub(\core\lock\lock::class);
 
         $tasks = $DB->get_recordset('task_adhoc', $params);
         foreach ($tasks as $record) {