MDL-69688 phpunit: Improve test_context_not_used

This adds a few changes to the old test_context_not_used test:

- Move it to become an advanced_test, because its mission
  is to verify that the assertEventContextNotUsed() assertion
  works as expected.
- For consistency, also move the fixtures to own phpunit fixtures.
- Add proper coverage tags, to verify that the assertion is being
  covered.
- Add a data provider to provide all the current cases and ease
  any future case that may be needed in the future. One by one
  because previously there was code never executed with the
  warning expectation causing the test to stop.
- Run them in isolation, while this is not strictly required, it's
  including external fixtures and, we'll need that isolation soon
  (for changes coming when moving the test to PHPUnit 9.6 in MDL-81266).

lib/phpunit/classes/advanced_testcase.php

@@ -390,7 +390,8 @@ abstract class advanced_testcase extends base_testcase {
     }
 
     /**
-     * Assert that an event is not using event->contxet.
+     * Assert that various event methods are not using event->context
+     *
      * While restoring context might not be valid and it should not be used by event url
      * or description methods.
      *
@@ -410,7 +411,7 @@ abstract class advanced_testcase extends base_testcase {
         $event->get_url();
         $event->get_description();
 
-        // Restore event->context.
+        // Restore event->context (note that this is unreachable when the event uses context). But ok for correct events.
         phpunit_event_mock::testable_set_event_context($event, $eventcontext);
     }
 

lib/phpunit/tests/advanced_test.php

@@ -339,6 +339,54 @@ class advanced_test extends \advanced_testcase {
         }
     }
 
+    /**
+     * Test the assertEventContextNotUsed() assertion.
+     *
+     * Verify that events using the event context in some of their
+     * methods are detected properly (will throw a warning if they are).
+     *
+     * To do so, we'll be using some fixture events (context_used_in_event_xxxx),
+     * that, on purpose, use the event context (incorrectly) in their methods.
+     *
+     * Note that because we are using imported fixture classes, and because we
+     * are testing for warnings, better we run the tests in a separate process.
+     *
+     * @param string $fixture The fixture class to use.
+     * @param bool $phpwarn Whether a PHP warning is expected.
+     *
+     * @runInSeparateProcess
+     * @dataProvider assert_event_context_not_used_provider
+     * @covers ::assertEventContextNotUsed
+     */
+    public function test_assert_event_context_not_used($fixture, $phpwarn): void {
+        require(__DIR__ . '/fixtures/event_fixtures.php');
+        // Create an event that uses the event context in its get_url() and get_description() methods.
+        $event = $fixture::create([
+            'other' => [
+                'sample' => 1,
+                'xx' => 10,
+            ],
+        ]);
+
+        if ($phpwarn) {
+            $this->expectWarning();
+        }
+        $this->assertEventContextNotUsed($event);
+    }
+
+    /**
+     * Data provider for test_assert_event_context_not_used().
+     *
+     * @return array
+     */
+    public static function assert_event_context_not_used_provider(): array {
+        return [
+            'correct' => ['\core\event\context_used_in_event_correct', false],
+            'wrong_get_url' => ['\core\event\context_used_in_event_get_url', true],
+            'wrong_get_description' => ['\core\event\context_used_in_event_get_description', true],
+        ];
+    }
+
     public function test_message_processors_reset() {
         global $DB;
 

lib/phpunit/tests/fixtures/event_fixtures.php

@@ -0,0 +1,80 @@
+<?php
+// This file is part of Moodle - https://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <https://www.gnu.org/licenses/>.
+
+/**
+ * Fixtures for advanced_testcase tests.
+ *
+ * @package   core
+ * @category  event
+ * @copyright 2024 onwards Eloy Lafuente (stronk7) {@link https://stronk7.com}
+ * @license   https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+namespace core\event;
+
+defined('MOODLE_INTERNAL') || die();
+
+/**
+ * Event to test that \advanced_testcase::assertEventContextNotUsed() passes ok when no context is used.
+ */
+class context_used_in_event_correct extends \core\event\base {
+
+    protected function init() {
+        $this->data['crud'] = 'u';
+        $this->data['edulevel'] = self::LEVEL_PARTICIPATING;
+        $this->context = \context_system::instance();
+    }
+
+    public function get_url() {
+        return new \moodle_url('/somepath/somefile.php'); // No context used.
+    }
+
+    public function get_description() {
+        return 'Description'; // No context used.
+    }
+}
+
+/**
+ * Event to test that \advanced_testcase::assertEventContextNotUsed() detects context usage on get_url().
+ */
+class context_used_in_event_get_url extends \core\event\base {
+
+    protected function init() {
+        $this->data['crud'] = 'u';
+        $this->data['edulevel'] = self::LEVEL_PARTICIPATING;
+        $this->context = \context_system::instance();
+    }
+
+    public function get_url() {
+        return new \moodle_url('/somepath/somefile.php', ['id' => $this->context->instanceid]); // Causes a PHP Warning.
+    }
+}
+
+/**
+ * Event to test that \advanced_testcase::assertEventContextNotUsed() detects context usage on get_description().
+ */
+class context_used_in_event_get_description extends \core\event\base {
+
+    protected function init() {
+        $this->data['crud'] = 'u';
+        $this->data['edulevel'] = self::LEVEL_PARTICIPATING;
+        $this->context = \context_system::instance();
+    }
+
+    public function get_description() {
+        return $this->context->instanceid . " Description"; // Causes a PHP Warning.
+    }
+}

lib/tests/event/base_test.php

@@ -853,27 +853,6 @@ class base_test extends \advanced_testcase {
         $this->assertSame($event->get_data(), $data);
     }
 
-    public function test_context_not_used() {
-        // TODO: MDL-69688 - This test is far away from my understanding. It throws a
-        // "Trying to get property 'instanceid' of non-object" notice, so
-        // it's not clear for me what the test is doing. This was detected
-        // when preparing tests for PHPUnit 8 (MDL-67673) and, at the end
-        // all that was done is to move the annotation (deprecated) to
-        // explicit expectation. Still try commenting it out and you'll see
-        // the notice.
-        if (PHP_VERSION_ID >= 80000) {
-            $this->expectWarning();
-        } else {
-            $this->expectNotice();
-        }
-        $event = \core_tests\event\context_used_in_event::create(array('other' => array('sample' => 1, 'xx' => 10)));
-        $this->assertEventContextNotUsed($event);
-
-        $eventcontext = phpunit_event_mock::testable_get_event_context($event);
-        phpunit_event_mock::testable_set_event_context($event, null);
-        $this->assertEventContextNotUsed($event);
-    }
-
     /**
      * Test that all observer information is returned correctly.
      */

lib/tests/fixtures/event_fixtures.php

@@ -244,25 +244,6 @@ class course_module_viewed_noinit extends \core\event\course_module_viewed {
 class grade_report_viewed extends \core\event\grade_report_viewed {
 }
 
-/**
- * Event to test context used in event functions
- */
-class context_used_in_event extends \core\event\base {
-    public function get_description() {
-        return $this->context->instanceid . " Description";
-    }
-
-    protected function init() {
-        $this->data['crud'] = 'u';
-        $this->data['edulevel'] = self::LEVEL_PARTICIPATING;
-        $this->context = \context_system::instance();
-    }
-
-    public function get_url() {
-        return new \moodle_url('/somepath/somefile.php', array('id' => $this->context->instanceid));
-    }
-}
-
 /**
  * This is an explanation of the event.
  *      - I'm making a point here.