From 9195c8867046de1a4a64f027b56be63826289abc Mon Sep 17 00:00:00 2001
From: "Dylan T." <dktapps@pmmp.io>
Date: Wed, 20 Nov 2024 14:56:52 +0000
Subject: [PATCH 1/4] ConsoleReader: Use proc_open()'s socket support to send
 commands back to the main server process (#5273)

Support for this was introduced in PHP 8.0, though not mentioned in any changelog: php/php-src#5777

This simplifies the subprocess handling considerably. However, there is a potential for problems if PHP generates any E_* errors, since these get written to STDOUT as well.
To avoid error messages being treated as a command, a hash is attached to each IPC message, seeded with an incrementing counter. This prevents error messages causing command replays or unintended commands.

Unfortunately, PHP doesn't support binding pipes other than stdin/stdout/stderr on Windows for the child process, so we have to use stdout for this. In the future, if it becomes possible, a dedicated pipe for the purpose should be introduced. We'd need something like php://fd/<number> to work on Windows.
---
 src/console/ConsoleReaderChildProcess.php     | 30 +++---
 .../ConsoleReaderChildProcessDaemon.php       | 54 ++++++-----
 .../ConsoleReaderChildProcessUtils.php        | 71 ++++++++++++++
 .../ConsoleReaderChildProcessUtilsTest.php    | 92 +++++++++++++++++++
 4 files changed, 209 insertions(+), 38 deletions(-)
 create mode 100644 src/console/ConsoleReaderChildProcessUtils.php
 create mode 100644 tests/phpunit/console/ConsoleReaderChildProcessUtilsTest.php

diff --git a/src/console/ConsoleReaderChildProcess.php b/src/console/ConsoleReaderChildProcess.php
index 0d5a30fdcc..c0c4f0048f 100644
--- a/src/console/ConsoleReaderChildProcess.php
+++ b/src/console/ConsoleReaderChildProcess.php
@@ -29,23 +29,21 @@ use pocketmine\utils\Process;
 use function cli_set_process_title;
 use function count;
 use function dirname;
-use function feof;
 use function fwrite;
-use function stream_socket_client;
+use function is_numeric;
+use const PHP_EOL;
+use const STDOUT;
+
+if(count($argv) !== 2 || !is_numeric($argv[1])){
+	echo "Usage: " . $argv[0] . " <command token seed>" . PHP_EOL;
+	exit(1);
+}
+
+$commandTokenSeed = (int) $argv[1];
 
 require dirname(__DIR__, 2) . '/vendor/autoload.php';
 
-if(count($argv) !== 2){
-	die("Please provide a server to connect to");
-}
-
 @cli_set_process_title('PocketMine-MP Console Reader');
-$errCode = null;
-$errMessage = null;
-$socket = stream_socket_client($argv[1], $errCode, $errMessage, 15.0);
-if($socket === false){
-	throw new \RuntimeException("Failed to connect to server process ($errCode): $errMessage");
-}
 
 /** @phpstan-var ThreadSafeArray<int, string> $channel */
 $channel = new ThreadSafeArray();
@@ -75,15 +73,15 @@ $thread = new class($channel) extends NativeThread{
 };
 
 $thread->start(NativeThread::INHERIT_NONE);
-while(!feof($socket)){
+while(true){
 	$line = $channel->synchronized(function() use ($channel) : ?string{
 		if(count($channel) === 0){
 			$channel->wait(1_000_000);
 		}
-		$line = $channel->shift();
-		return $line;
+		return $channel->shift();
 	});
-	if(@fwrite($socket, ($line ?? "") . "\n") === false){
+	$message = $line !== null ? ConsoleReaderChildProcessUtils::createMessage($line, $commandTokenSeed) : "";
+	if(@fwrite(STDOUT, $message . "\n") === false){
 		//Always send even if there's no line, to check if the parent is alive
 		//If the parent process was terminated forcibly, it won't close the connection properly, so feof() will return
 		//false even though the connection is actually broken. However, fwrite() will fail.
diff --git a/src/console/ConsoleReaderChildProcessDaemon.php b/src/console/ConsoleReaderChildProcessDaemon.php
index 138559f06e..f7300b7a52 100644
--- a/src/console/ConsoleReaderChildProcessDaemon.php
+++ b/src/console/ConsoleReaderChildProcessDaemon.php
@@ -29,19 +29,16 @@ use Symfony\Component\Filesystem\Path;
 use function base64_encode;
 use function fgets;
 use function fopen;
+use function mt_rand;
 use function preg_replace;
 use function proc_close;
 use function proc_open;
 use function proc_terminate;
+use function rtrim;
 use function sprintf;
 use function stream_select;
-use function stream_socket_accept;
-use function stream_socket_get_name;
-use function stream_socket_server;
-use function stream_socket_shutdown;
 use function trim;
 use const PHP_BINARY;
-use const STREAM_SHUT_RDWR;
 
 /**
  * This pile of shit exists because PHP on Windows is broken, and can't handle stream_select() on stdin or pipes
@@ -58,44 +55,44 @@ use const STREAM_SHUT_RDWR;
  * communication.
  */
 final class ConsoleReaderChildProcessDaemon{
+	public const TOKEN_DELIMITER = ":";
+	public const TOKEN_HASH_ALGO = "xxh3";
+
 	private \PrefixedLogger $logger;
 	/** @var resource */
 	private $subprocess;
 	/** @var resource */
 	private $socket;
+	private int $commandTokenSeed;
 
 	public function __construct(
 		\Logger $logger
 	){
 		$this->logger = new \PrefixedLogger($logger, "Console Reader Daemon");
+		$this->commandTokenSeed = mt_rand();
 		$this->prepareSubprocess();
 	}
 
 	private function prepareSubprocess() : void{
-		$server = stream_socket_server("tcp://127.0.0.1:0");
-		if($server === false){
-			throw new \RuntimeException("Failed to open console reader socket server");
-		}
-		$address = Utils::assumeNotFalse(stream_socket_get_name($server, false), "stream_socket_get_name() shouldn't return false here");
-
 		//Windows sucks, and likes to corrupt UTF-8 file paths when they travel to the subprocess, so we base64 encode
 		//the path to avoid the problem. This is an abysmally shitty hack, but here we are :(
 		$sub = Utils::assumeNotFalse(proc_open(
-			[PHP_BINARY, '-dopcache.enable_cli=0', '-r', sprintf('require base64_decode("%s", true);', base64_encode(Path::join(__DIR__, 'ConsoleReaderChildProcess.php'))), $address],
 			[
+				PHP_BINARY,
+				'-dopcache.enable_cli=0',
+				'-r',
+				sprintf('require base64_decode("%s", true);', base64_encode(Path::join(__DIR__, 'ConsoleReaderChildProcess.php'))),
+				(string) $this->commandTokenSeed
+			],
+			[
+				1 => ['socket'],
 				2 => fopen("php://stderr", "w"),
 			],
 			$pipes
 		), "Something has gone horribly wrong");
 
-		$client = stream_socket_accept($server, 15);
-		if($client === false){
-			throw new AssumptionFailedError("stream_socket_accept() returned false");
-		}
-		stream_socket_shutdown($server, STREAM_SHUT_RDWR);
-
 		$this->subprocess = $sub;
-		$this->socket = $client;
+		$this->socket = $pipes[1];
 	}
 
 	private function shutdownSubprocess() : void{
@@ -104,7 +101,6 @@ final class ConsoleReaderChildProcessDaemon{
 		//the first place).
 		proc_terminate($this->subprocess);
 		proc_close($this->subprocess);
-		stream_socket_shutdown($this->socket, STREAM_SHUT_RDWR);
 	}
 
 	public function readLine() : ?string{
@@ -112,13 +108,27 @@ final class ConsoleReaderChildProcessDaemon{
 		$w = null;
 		$e = null;
 		if(stream_select($r, $w, $e, 0, 0) === 1){
-			$command = fgets($this->socket);
-			if($command === false){
+			$line = fgets($this->socket);
+			if($line === false){
 				$this->logger->debug("Lost connection to subprocess, restarting (maybe the child process was killed from outside?)");
 				$this->shutdownSubprocess();
 				$this->prepareSubprocess();
 				return null;
 			}
+			$line = rtrim($line, "\n");
+
+			if($line === ""){
+				//keepalive
+				return null;
+			}
+
+			$command = ConsoleReaderChildProcessUtils::parseMessage($line, $this->commandTokenSeed);
+			if($command === null){
+				//this is not a command - it may be some kind of error output from the subprocess
+				//write it directly to the console
+				$this->logger->warning("Unexpected output from child process: $line");
+				return null;
+			}
 
 			$command = preg_replace("#\\x1b\\x5b([^\\x1b]*\\x7e|[\\x40-\\x50])#", "", trim($command)) ?? throw new AssumptionFailedError("This regex is assumed to be valid");
 			$command = preg_replace('/[[:cntrl:]]/', '', $command) ?? throw new AssumptionFailedError("This regex is assumed to be valid");
diff --git a/src/console/ConsoleReaderChildProcessUtils.php b/src/console/ConsoleReaderChildProcessUtils.php
new file mode 100644
index 0000000000..661e9b0f7b
--- /dev/null
+++ b/src/console/ConsoleReaderChildProcessUtils.php
@@ -0,0 +1,71 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+ */
+
+declare(strict_types=1);
+
+namespace pocketmine\console;
+
+use function hash;
+use function strlen;
+use function strrpos;
+use function substr;
+
+final class ConsoleReaderChildProcessUtils{
+	public const TOKEN_DELIMITER = ":";
+	public const TOKEN_HASH_ALGO = "xxh3";
+
+	private function __construct(){
+
+	}
+
+	/**
+	 * Creates an IPC message to transmit a user's input command to the parent process.
+	 *
+	 * Unfortunately we can't currently provide IPC pipes other than stdout/stderr to subprocesses on Windows, so this
+	 * adds a hash of the user input (with a counter as salt) to prevent unintended process output (like error messages)
+	 * from being treated as user input.
+	 */
+	public static function createMessage(string $line, int &$counter) : string{
+		$token = hash(self::TOKEN_HASH_ALGO, $line, options: ['seed' => $counter]);
+		$counter++;
+		return $line . self::TOKEN_DELIMITER . $token;
+	}
+
+	/**
+	 * Extracts a command from an IPC message from the console reader subprocess.
+	 * Returns the user's input command, or null if this isn't a user input.
+	 */
+	public static function parseMessage(string $message, int &$counter) : ?string{
+		$delimiterPos = strrpos($message, self::TOKEN_DELIMITER);
+		if($delimiterPos !== false){
+			$left = substr($message, 0, $delimiterPos);
+			$right = substr($message, $delimiterPos + strlen(self::TOKEN_DELIMITER));
+			$expectedToken = hash(self::TOKEN_HASH_ALGO, $left, options: ['seed' => $counter]);
+
+			if($expectedToken === $right){
+				$counter++;
+				return $left;
+			}
+		}
+
+		return null;
+	}
+}
diff --git a/tests/phpunit/console/ConsoleReaderChildProcessUtilsTest.php b/tests/phpunit/console/ConsoleReaderChildProcessUtilsTest.php
new file mode 100644
index 0000000000..31ae2e27a4
--- /dev/null
+++ b/tests/phpunit/console/ConsoleReaderChildProcessUtilsTest.php
@@ -0,0 +1,92 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+ */
+
+declare(strict_types=1);
+
+namespace pocketmine\console;
+
+use PHPUnit\Framework\TestCase;
+use function mt_rand;
+use function str_repeat;
+
+final class ConsoleReaderChildProcessUtilsTest extends TestCase{
+
+	/**
+	 * @phpstan-return \Generator<int, array{string}, void, void>
+	 */
+	public static function commandStringProvider() : \Generator{
+		yield ["stop"];
+		yield ["pocketmine:status"];
+		yield [str_repeat("s", 1000)];
+		yield ["time set day"];
+		yield ["give \"Steve\" golden_apple"];
+	}
+
+	/**
+	 * @dataProvider commandStringProvider
+	 */
+	public function testCreateParseSymmetry(string $input) : void{
+		$counterCreate = $counterParse = mt_rand();
+		$message = ConsoleReaderChildProcessUtils::createMessage($input, $counterCreate);
+		$parsedInput = ConsoleReaderChildProcessUtils::parseMessage($message, $counterParse);
+
+		self::assertSame($input, $parsedInput);
+	}
+
+	public function testCreateMessage() : void{
+		$counter = 0;
+
+		ConsoleReaderChildProcessUtils::createMessage("", $counter);
+		self::assertSame(1, $counter, "createMessage should always bump the counter");
+	}
+
+	/**
+	 * @phpstan-return \Generator<int, array{string, bool}, void, void>
+	 */
+	public static function parseMessageProvider() : \Generator{
+		$counter = 0;
+		yield [ConsoleReaderChildProcessUtils::createMessage("", $counter), true];
+
+		yield ["", false]; //keepalive message, doesn't bump counter
+
+		$counter = 1;
+		yield [ConsoleReaderChildProcessUtils::createMessage("", $counter), false]; //mismatched counter
+
+		$counter = 0;
+		yield ["a" . ConsoleReaderChildProcessUtils::TOKEN_DELIMITER . "b", false]; //message with delimiter but not a valid IPC message
+	}
+
+	/**
+	 * @dataProvider parseMessageProvider
+	 */
+	public static function testParseMessage(string $message, bool $valid) : void{
+		$counter = $oldCounter = 0;
+
+		$input = ConsoleReaderChildProcessUtils::parseMessage($message, $counter);
+		if(!$valid){
+			self::assertNull($input, "Result should be null on invalid message");
+			self::assertSame($oldCounter, $counter, "Counter shouldn't be bumped on invalid message");
+		}else{
+			self::assertNotNull($input, "This was a valid message, expected a result");
+			self::assertSame($oldCounter + 1, $counter, "Counter should be bumped on valid message parse");
+		}
+	}
+}

From 49bdaee930f552b387c39992217519eff381447d Mon Sep 17 00:00:00 2001
From: "Dylan K. Taylor" <dktapps@pmmp.io>
Date: Wed, 20 Nov 2024 15:38:44 +0000
Subject: [PATCH 2/4] Move event handler inheritance test to PHPUnit using mock
 objects this is dodgy and we shouldn't rely on it long term.

relates to #6524
---
 tests/phpunit/event/EventTest.php             | 65 ++++++++++++++
 .../event/TestChildEvent.php}                 |  4 +-
 .../event/TestGrandchildEvent.php}            |  4 +-
 .../event/TestParentEvent.php}                |  4 +-
 .../src/EventHandlerInheritanceTest.php       | 88 -------------------
 tests/plugins/TesterPlugin/src/Main.php       |  2 +-
 6 files changed, 72 insertions(+), 95 deletions(-)
 create mode 100644 tests/phpunit/event/EventTest.php
 rename tests/{plugins/TesterPlugin/src/event/GrandchildEvent.php => phpunit/event/TestChildEvent.php} (90%)
 rename tests/{plugins/TesterPlugin/src/event/ParentEvent.php => phpunit/event/TestGrandchildEvent.php} (89%)
 rename tests/{plugins/TesterPlugin/src/event/ChildEvent.php => phpunit/event/TestParentEvent.php} (91%)
 delete mode 100644 tests/plugins/TesterPlugin/src/EventHandlerInheritanceTest.php

diff --git a/tests/phpunit/event/EventTest.php b/tests/phpunit/event/EventTest.php
new file mode 100644
index 0000000000..259daaf5d2
--- /dev/null
+++ b/tests/phpunit/event/EventTest.php
@@ -0,0 +1,65 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+ */
+
+declare(strict_types=1);
+
+namespace pocketmine\event;
+
+use PHPUnit\Framework\TestCase;
+use pocketmine\plugin\Plugin;
+use pocketmine\plugin\PluginManager;
+use pocketmine\Server;
+
+final class EventTest extends TestCase{
+
+	public function testHandlerInheritance() : void{
+		//TODO: this is a really bad hack and could break any time if PluginManager decides to access its Server field
+		//we really need to make it possible to register events without a Plugin or Server context
+		$mockServer = $this->createMock(Server::class);
+		$mockPlugin = self::createStub(Plugin::class);
+		$mockPlugin->method('isEnabled')->willReturn(true);
+
+		$pluginManager = new PluginManager($mockServer, null);
+
+		$expectedOrder = [
+			TestGrandchildEvent::class,
+			TestChildEvent::class,
+			TestParentEvent::class
+		];
+		$actualOrder = [];
+
+		foreach($expectedOrder as $class){
+			$pluginManager->registerEvent(
+				$class,
+				function(TestParentEvent $event) use (&$actualOrder, $class) : void{
+					$actualOrder[] = $class;
+				},
+				EventPriority::NORMAL,
+				$mockPlugin
+			);
+		}
+
+		$event = new TestGrandchildEvent();
+		$event->call();
+
+		self::assertSame($expectedOrder, $actualOrder, "Expected event handlers to be called from most specific to least specific");
+	}
+}
diff --git a/tests/plugins/TesterPlugin/src/event/GrandchildEvent.php b/tests/phpunit/event/TestChildEvent.php
similarity index 90%
rename from tests/plugins/TesterPlugin/src/event/GrandchildEvent.php
rename to tests/phpunit/event/TestChildEvent.php
index 40c37c5679..2b79f882fc 100644
--- a/tests/plugins/TesterPlugin/src/event/GrandchildEvent.php
+++ b/tests/phpunit/event/TestChildEvent.php
@@ -21,8 +21,8 @@
 
 declare(strict_types=1);
 
-namespace pmmp\TesterPlugin\event;
+namespace pocketmine\event;
 
-class GrandchildEvent extends ChildEvent{
+class TestChildEvent extends TestParentEvent{
 
 }
diff --git a/tests/plugins/TesterPlugin/src/event/ParentEvent.php b/tests/phpunit/event/TestGrandchildEvent.php
similarity index 89%
rename from tests/plugins/TesterPlugin/src/event/ParentEvent.php
rename to tests/phpunit/event/TestGrandchildEvent.php
index 68f7df6300..7685f3b0b5 100644
--- a/tests/plugins/TesterPlugin/src/event/ParentEvent.php
+++ b/tests/phpunit/event/TestGrandchildEvent.php
@@ -21,8 +21,8 @@
 
 declare(strict_types=1);
 
-namespace pmmp\TesterPlugin\event;
+namespace pocketmine\event;
 
-class ParentEvent extends \pocketmine\event\Event{
+class TestGrandchildEvent extends TestChildEvent{
 
 }
diff --git a/tests/plugins/TesterPlugin/src/event/ChildEvent.php b/tests/phpunit/event/TestParentEvent.php
similarity index 91%
rename from tests/plugins/TesterPlugin/src/event/ChildEvent.php
rename to tests/phpunit/event/TestParentEvent.php
index b71d2627fc..c770f6372e 100644
--- a/tests/plugins/TesterPlugin/src/event/ChildEvent.php
+++ b/tests/phpunit/event/TestParentEvent.php
@@ -21,8 +21,8 @@
 
 declare(strict_types=1);
 
-namespace pmmp\TesterPlugin\event;
+namespace pocketmine\event;
 
-class ChildEvent extends ParentEvent{
+class TestParentEvent extends Event{
 
 }
diff --git a/tests/plugins/TesterPlugin/src/EventHandlerInheritanceTest.php b/tests/plugins/TesterPlugin/src/EventHandlerInheritanceTest.php
deleted file mode 100644
index efe20f8d8f..0000000000
--- a/tests/plugins/TesterPlugin/src/EventHandlerInheritanceTest.php
+++ /dev/null
@@ -1,88 +0,0 @@
-<?php
-
-/*
- *
- *  ____            _        _   __  __ _                  __  __ ____
- * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
- * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
- * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
- * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Lesser General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * @author PocketMine Team
- * @link http://www.pocketmine.net/
- *
- *
- */
-
-declare(strict_types=1);
-
-namespace pmmp\TesterPlugin;
-
-use pmmp\TesterPlugin\event\ChildEvent;
-use pmmp\TesterPlugin\event\GrandchildEvent;
-use pmmp\TesterPlugin\event\ParentEvent;
-use pocketmine\event\EventPriority;
-use function implode;
-
-final class EventHandlerInheritanceTest extends Test{
-
-	private const EXPECTED_ORDER = [
-		GrandchildEvent::class,
-		ChildEvent::class,
-		ParentEvent::class
-	];
-
-	/** @var string[] */
-	private array $callOrder = [];
-
-	public function getName() : string{
-		return "Event Handler Inheritance Test";
-	}
-
-	public function getDescription() : string{
-		return "Tests that child events are correctly passed to parent event handlers";
-	}
-
-	public function run() : void{
-		$plugin = $this->getPlugin();
-		$plugin->getServer()->getPluginManager()->registerEvent(
-			ParentEvent::class,
-			function(ParentEvent $event) : void{
-				$this->callOrder[] = ParentEvent::class;
-			},
-			EventPriority::NORMAL,
-			$plugin
-		);
-		$plugin->getServer()->getPluginManager()->registerEvent(
-			ChildEvent::class,
-			function(ChildEvent $event) : void{
-				$this->callOrder[] = ChildEvent::class;
-			},
-			EventPriority::NORMAL,
-			$plugin
-		);
-		$plugin->getServer()->getPluginManager()->registerEvent(
-			GrandchildEvent::class,
-			function(GrandchildEvent $event) : void{
-				$this->callOrder[] = GrandchildEvent::class;
-			},
-			EventPriority::NORMAL,
-			$plugin
-		);
-
-		$event = new GrandchildEvent();
-		$event->call();
-
-		if($this->callOrder === self::EXPECTED_ORDER){
-			$this->setResult(Test::RESULT_OK);
-		}else{
-			$plugin->getLogger()->error("Expected order: " . implode(", ", self::EXPECTED_ORDER) . ", got: " . implode(", ", $this->callOrder));
-			$this->setResult(Test::RESULT_FAILED);
-		}
-	}
-}
diff --git a/tests/plugins/TesterPlugin/src/Main.php b/tests/plugins/TesterPlugin/src/Main.php
index 26d3441f4a..08b59dbacb 100644
--- a/tests/plugins/TesterPlugin/src/Main.php
+++ b/tests/plugins/TesterPlugin/src/Main.php
@@ -57,7 +57,7 @@ class Main extends PluginBase implements Listener{
 		}), 10);
 
 		$this->waitingTests = [
-			new EventHandlerInheritanceTest($this),
+			//Add test objects here
 		];
 	}
 

From 844ba0ff9fa69785ba8a583dd1cc7af5100f9871 Mon Sep 17 00:00:00 2001
From: "Dylan K. Taylor" <dktapps@pmmp.io>
Date: Wed, 20 Nov 2024 15:40:34 +0000
Subject: [PATCH 3/4] Move event test fixtures to a subdirectory

---
 tests/phpunit/event/EventTest.php                           | 3 +++
 tests/phpunit/event/HandlerListManagerTest.php              | 6 ++++++
 .../event/{ => fixtures}/TestAbstractAllowHandleEvent.php   | 4 +++-
 tests/phpunit/event/{ => fixtures}/TestAbstractEvent.php    | 4 +++-
 tests/phpunit/event/{ => fixtures}/TestChildEvent.php       | 2 +-
 tests/phpunit/event/{ => fixtures}/TestConcreteEvent.php    | 4 +++-
 .../{ => fixtures}/TestConcreteExtendsAbstractEvent.php     | 2 +-
 .../{ => fixtures}/TestConcreteExtendsAllowHandleEvent.php  | 2 +-
 .../{ => fixtures}/TestConcreteExtendsConcreteEvent.php     | 2 +-
 tests/phpunit/event/{ => fixtures}/TestGrandchildEvent.php  | 2 +-
 tests/phpunit/event/{ => fixtures}/TestParentEvent.php      | 4 +++-
 11 files changed, 26 insertions(+), 9 deletions(-)
 rename tests/phpunit/event/{ => fixtures}/TestAbstractAllowHandleEvent.php (92%)
 rename tests/phpunit/event/{ => fixtures}/TestAbstractEvent.php (92%)
 rename tests/phpunit/event/{ => fixtures}/TestChildEvent.php (95%)
 rename tests/phpunit/event/{ => fixtures}/TestConcreteEvent.php (92%)
 rename tests/phpunit/event/{ => fixtures}/TestConcreteExtendsAbstractEvent.php (95%)
 rename tests/phpunit/event/{ => fixtures}/TestConcreteExtendsAllowHandleEvent.php (95%)
 rename tests/phpunit/event/{ => fixtures}/TestConcreteExtendsConcreteEvent.php (95%)
 rename tests/phpunit/event/{ => fixtures}/TestGrandchildEvent.php (95%)
 rename tests/phpunit/event/{ => fixtures}/TestParentEvent.php (92%)

diff --git a/tests/phpunit/event/EventTest.php b/tests/phpunit/event/EventTest.php
index 259daaf5d2..5d5ace54d4 100644
--- a/tests/phpunit/event/EventTest.php
+++ b/tests/phpunit/event/EventTest.php
@@ -24,6 +24,9 @@ declare(strict_types=1);
 namespace pocketmine\event;
 
 use PHPUnit\Framework\TestCase;
+use pocketmine\event\fixtures\TestChildEvent;
+use pocketmine\event\fixtures\TestGrandchildEvent;
+use pocketmine\event\fixtures\TestParentEvent;
 use pocketmine\plugin\Plugin;
 use pocketmine\plugin\PluginManager;
 use pocketmine\Server;
diff --git a/tests/phpunit/event/HandlerListManagerTest.php b/tests/phpunit/event/HandlerListManagerTest.php
index edff36639d..c61043dab8 100644
--- a/tests/phpunit/event/HandlerListManagerTest.php
+++ b/tests/phpunit/event/HandlerListManagerTest.php
@@ -24,6 +24,12 @@ declare(strict_types=1);
 namespace pocketmine\event;
 
 use PHPUnit\Framework\TestCase;
+use pocketmine\event\fixtures\TestAbstractAllowHandleEvent;
+use pocketmine\event\fixtures\TestAbstractEvent;
+use pocketmine\event\fixtures\TestConcreteEvent;
+use pocketmine\event\fixtures\TestConcreteExtendsAbstractEvent;
+use pocketmine\event\fixtures\TestConcreteExtendsAllowHandleEvent;
+use pocketmine\event\fixtures\TestConcreteExtendsConcreteEvent;
 
 class HandlerListManagerTest extends TestCase{
 
diff --git a/tests/phpunit/event/TestAbstractAllowHandleEvent.php b/tests/phpunit/event/fixtures/TestAbstractAllowHandleEvent.php
similarity index 92%
rename from tests/phpunit/event/TestAbstractAllowHandleEvent.php
rename to tests/phpunit/event/fixtures/TestAbstractAllowHandleEvent.php
index 1bac06bbb0..3831698098 100644
--- a/tests/phpunit/event/TestAbstractAllowHandleEvent.php
+++ b/tests/phpunit/event/fixtures/TestAbstractAllowHandleEvent.php
@@ -21,7 +21,9 @@
 
 declare(strict_types=1);
 
-namespace pocketmine\event;
+namespace pocketmine\event\fixtures;
+
+use pocketmine\event\Event;
 
 /**
  * @allowHandle
diff --git a/tests/phpunit/event/TestAbstractEvent.php b/tests/phpunit/event/fixtures/TestAbstractEvent.php
similarity index 92%
rename from tests/phpunit/event/TestAbstractEvent.php
rename to tests/phpunit/event/fixtures/TestAbstractEvent.php
index 92a95363e3..b48d8f5269 100644
--- a/tests/phpunit/event/TestAbstractEvent.php
+++ b/tests/phpunit/event/fixtures/TestAbstractEvent.php
@@ -21,7 +21,9 @@
 
 declare(strict_types=1);
 
-namespace pocketmine\event;
+namespace pocketmine\event\fixtures;
+
+use pocketmine\event\Event;
 
 abstract class TestAbstractEvent extends Event{
 
diff --git a/tests/phpunit/event/TestChildEvent.php b/tests/phpunit/event/fixtures/TestChildEvent.php
similarity index 95%
rename from tests/phpunit/event/TestChildEvent.php
rename to tests/phpunit/event/fixtures/TestChildEvent.php
index 2b79f882fc..569a2c069a 100644
--- a/tests/phpunit/event/TestChildEvent.php
+++ b/tests/phpunit/event/fixtures/TestChildEvent.php
@@ -21,7 +21,7 @@
 
 declare(strict_types=1);
 
-namespace pocketmine\event;
+namespace pocketmine\event\fixtures;
 
 class TestChildEvent extends TestParentEvent{
 
diff --git a/tests/phpunit/event/TestConcreteEvent.php b/tests/phpunit/event/fixtures/TestConcreteEvent.php
similarity index 92%
rename from tests/phpunit/event/TestConcreteEvent.php
rename to tests/phpunit/event/fixtures/TestConcreteEvent.php
index 8b159df91a..cf744eb2ce 100644
--- a/tests/phpunit/event/TestConcreteEvent.php
+++ b/tests/phpunit/event/fixtures/TestConcreteEvent.php
@@ -21,7 +21,9 @@
 
 declare(strict_types=1);
 
-namespace pocketmine\event;
+namespace pocketmine\event\fixtures;
+
+use pocketmine\event\Event;
 
 class TestConcreteEvent extends Event{
 
diff --git a/tests/phpunit/event/TestConcreteExtendsAbstractEvent.php b/tests/phpunit/event/fixtures/TestConcreteExtendsAbstractEvent.php
similarity index 95%
rename from tests/phpunit/event/TestConcreteExtendsAbstractEvent.php
rename to tests/phpunit/event/fixtures/TestConcreteExtendsAbstractEvent.php
index 3f0fa572fe..54ec3dbb12 100644
--- a/tests/phpunit/event/TestConcreteExtendsAbstractEvent.php
+++ b/tests/phpunit/event/fixtures/TestConcreteExtendsAbstractEvent.php
@@ -21,7 +21,7 @@
 
 declare(strict_types=1);
 
-namespace pocketmine\event;
+namespace pocketmine\event\fixtures;
 
 class TestConcreteExtendsAbstractEvent extends TestAbstractEvent{
 
diff --git a/tests/phpunit/event/TestConcreteExtendsAllowHandleEvent.php b/tests/phpunit/event/fixtures/TestConcreteExtendsAllowHandleEvent.php
similarity index 95%
rename from tests/phpunit/event/TestConcreteExtendsAllowHandleEvent.php
rename to tests/phpunit/event/fixtures/TestConcreteExtendsAllowHandleEvent.php
index e414651211..362ce693b5 100644
--- a/tests/phpunit/event/TestConcreteExtendsAllowHandleEvent.php
+++ b/tests/phpunit/event/fixtures/TestConcreteExtendsAllowHandleEvent.php
@@ -21,7 +21,7 @@
 
 declare(strict_types=1);
 
-namespace pocketmine\event;
+namespace pocketmine\event\fixtures;
 
 class TestConcreteExtendsAllowHandleEvent extends TestAbstractAllowHandleEvent{
 
diff --git a/tests/phpunit/event/TestConcreteExtendsConcreteEvent.php b/tests/phpunit/event/fixtures/TestConcreteExtendsConcreteEvent.php
similarity index 95%
rename from tests/phpunit/event/TestConcreteExtendsConcreteEvent.php
rename to tests/phpunit/event/fixtures/TestConcreteExtendsConcreteEvent.php
index cc95589351..3fd3a6cf0e 100644
--- a/tests/phpunit/event/TestConcreteExtendsConcreteEvent.php
+++ b/tests/phpunit/event/fixtures/TestConcreteExtendsConcreteEvent.php
@@ -21,7 +21,7 @@
 
 declare(strict_types=1);
 
-namespace pocketmine\event;
+namespace pocketmine\event\fixtures;
 
 class TestConcreteExtendsConcreteEvent extends TestConcreteEvent{
 
diff --git a/tests/phpunit/event/TestGrandchildEvent.php b/tests/phpunit/event/fixtures/TestGrandchildEvent.php
similarity index 95%
rename from tests/phpunit/event/TestGrandchildEvent.php
rename to tests/phpunit/event/fixtures/TestGrandchildEvent.php
index 7685f3b0b5..bfe50f9f3d 100644
--- a/tests/phpunit/event/TestGrandchildEvent.php
+++ b/tests/phpunit/event/fixtures/TestGrandchildEvent.php
@@ -21,7 +21,7 @@
 
 declare(strict_types=1);
 
-namespace pocketmine\event;
+namespace pocketmine\event\fixtures;
 
 class TestGrandchildEvent extends TestChildEvent{
 
diff --git a/tests/phpunit/event/TestParentEvent.php b/tests/phpunit/event/fixtures/TestParentEvent.php
similarity index 92%
rename from tests/phpunit/event/TestParentEvent.php
rename to tests/phpunit/event/fixtures/TestParentEvent.php
index c770f6372e..c204422722 100644
--- a/tests/phpunit/event/TestParentEvent.php
+++ b/tests/phpunit/event/fixtures/TestParentEvent.php
@@ -21,7 +21,9 @@
 
 declare(strict_types=1);
 
-namespace pocketmine\event;
+namespace pocketmine\event\fixtures;
+
+use pocketmine\event\Event;
 
 class TestParentEvent extends Event{
 

From 1e3a858de6758c37e8a25c83258f439c348edde8 Mon Sep 17 00:00:00 2001
From: "Dylan K. Taylor" <dktapps@pmmp.io>
Date: Wed, 20 Nov 2024 16:42:50 +0000
Subject: [PATCH 4/4] Add setUp and tearDown for event unit tests

---
 tests/phpunit/event/EventTest.php | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/tests/phpunit/event/EventTest.php b/tests/phpunit/event/EventTest.php
index 5d5ace54d4..7410cc3bb0 100644
--- a/tests/phpunit/event/EventTest.php
+++ b/tests/phpunit/event/EventTest.php
@@ -33,15 +33,26 @@ use pocketmine\Server;
 
 final class EventTest extends TestCase{
 
-	public function testHandlerInheritance() : void{
+	private Plugin $mockPlugin;
+	private PluginManager $pluginManager;
+
+	protected function setUp() : void{
+		HandlerListManager::global()->unregisterAll();
+
 		//TODO: this is a really bad hack and could break any time if PluginManager decides to access its Server field
 		//we really need to make it possible to register events without a Plugin or Server context
 		$mockServer = $this->createMock(Server::class);
-		$mockPlugin = self::createStub(Plugin::class);
-		$mockPlugin->method('isEnabled')->willReturn(true);
+		$this->mockPlugin = self::createStub(Plugin::class);
+		$this->mockPlugin->method('isEnabled')->willReturn(true);
 
-		$pluginManager = new PluginManager($mockServer, null);
+		$this->pluginManager = new PluginManager($mockServer, null);
+	}
 
+	public static function tearDownAfterClass() : void{
+		HandlerListManager::global()->unregisterAll();
+	}
+
+	public function testHandlerInheritance() : void{
 		$expectedOrder = [
 			TestGrandchildEvent::class,
 			TestChildEvent::class,
@@ -50,13 +61,13 @@ final class EventTest extends TestCase{
 		$actualOrder = [];
 
 		foreach($expectedOrder as $class){
-			$pluginManager->registerEvent(
+			$this->pluginManager->registerEvent(
 				$class,
 				function(TestParentEvent $event) use (&$actualOrder, $class) : void{
 					$actualOrder[] = $class;
 				},
 				EventPriority::NORMAL,
-				$mockPlugin
+				$this->mockPlugin
 			);
 		}