Upgrade to PHPUnit 10 (#1551)

The major functional changes from 9.5 to 10.0 are: * Annotations are converted to attributes. * Data providers must now be static. Many data provider functions are converted to `TestWith` attributes. These are safer now as attributes, because they can contain real PHP objects (previously they had to be a JSON representation). When the data sets are trivial, this is much simpler and more compact. PHPUnit no longer returns non-zero exit code on notices/deprecations, though they will still be printed in the results due to the use of the `displayDetailsOnTestsThatTrigger*` settings. For more info, see: sebastianbergmann/phpunit#5196 All tests are upgraded using the PHPUNIT_100 rector ruleset. It didn't work perfectly, so some manual fixes were necessary. Add custom CSS for phpunit HTML code coverage. This uses dark-mode and colorblind-friendly colors.
smrealms · Feb 10, 2023 · cd79787 · cd79787
1 parent c08418f
commit cd79787
Show file tree

Hide file tree

Showing 66 changed files with 359 additions and 522 deletions.
diff --git a/composer.json b/composer.json
@@ -81,8 +81,8 @@
 		"fig-r/psr2r-sniffer": "1.4.0",
 		"overtrue/phplint": "6.1.0",
 		"phpstan/phpstan": "1.9.17",
-		"phpunit/phpunit": "9.5.28",
-		"phpunit/php-code-coverage": "9.2.20",
+		"phpunit/phpunit": "10.0.7",
+		"phpunit/php-code-coverage": "10.0.0",
 		"rector/rector": "0.15.13",
 		"squizlabs/php_codesniffer": "3.7.1"
 	}

diff --git a/phpunit.xml b/phpunit.xml
@@ -1,17 +1,19 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <phpunit
 	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-	xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.4/phpunit.xsd"
-	beStrictAboutTodoAnnotatedTests="true"
+	xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.0/phpunit.xsd"
+	beStrictAboutChangesToGlobalState="true"
 	bootstrap="src/bootstrap.php"
-	cacheResultFile=".phpunit.cache/test-results"
+	cacheDirectory=".phpunit.cache"
 	colors="true"
-	convertDeprecationsToExceptions="true"
+	displayDetailsOnTestsThatTriggerDeprecations="true"
+	displayDetailsOnTestsThatTriggerErrors="true"
+	displayDetailsOnTestsThatTriggerNotices="true"
+	displayDetailsOnTestsThatTriggerWarnings="true"
 	executionOrder="depends,defects"
 	failOnRisky="true"
 	failOnWarning="true"
-	forceCoversAnnotation="true"
-	verbose="true">
+	requireCoverageMetadata="true">
 	<testsuites>
 		<testsuite name="default">
 			<directory suffix="Test.php">test</directory>
@@ -20,16 +22,17 @@
 	<php>
 		<env name="DISABLE_PHPDI_COMPILATION" value="true" force="true" />
 	</php>
-	<coverage
-		cacheDirectory=".phpunit.cache/code-coverage"
-		includeUncoveredFiles="true">
+	<coverage includeUncoveredFiles="true">
 		<include>
 			<directory suffix=".php">src/lib</directory>
 		</include>
 		<report>
 			<clover outputFile="test/coverage/clover.xml" />
 			<text outputFile="php://stdout" showOnlySummary="true" />
-			<html outputDirectory="test/coverage/html" />
+			<html
+				outputDirectory="test/coverage/html"
+				customCssFile="test/coverage-report.css"
+			/>
 		</report>
 	</coverage>
 </phpunit>
diff --git a/rector.php b/rector.php
@@ -6,16 +6,22 @@
 use Rector\Php74\Rector\Assign\NullCoalescingOperatorRector;
 use Rector\Php80\Rector\FuncCall\ClassOnObjectRector;
 use Rector\Php81\Rector\Array_\FirstClassCallableRector;
+use Rector\PHPUnit\Set\PHPUnitSetList;
 
 return static function (RectorConfig $rectorConfig): void {
 	$rectorConfig->paths([
 		__DIR__ . '/test',
 		__DIR__ . '/src',
 	]);
 	$rectorConfig->importNames(true, false);
+
 	$rectorConfig->rule(DirNameFileConstantToDirConstantRector::class);
 	$rectorConfig->rule(JsonThrowOnErrorRector::class);
 	$rectorConfig->rule(NullCoalescingOperatorRector::class);
 	$rectorConfig->rule(ClassOnObjectRector::class);
 	$rectorConfig->rule(FirstClassCallableRector::class);
+
+	$rectorConfig->sets([
+		PHPUnitSetList::PHPUNIT_100,
+	]);
 };
diff --git a/test/SmrTest/BaseIntegrationSpec.php b/test/SmrTest/BaseIntegrationSpec.php
@@ -5,6 +5,9 @@
 use Exception;
 use mysqli;
 use PHPUnit\Framework\AssertionFailedError;
+use PHPUnit\Framework\Attributes\After;
+use PHPUnit\Framework\Attributes\AfterClass;
+use PHPUnit\Framework\Attributes\BeforeClass;
 use PHPUnit\Framework\TestCase;
 use Smr\Container\DiContainer;
 
@@ -24,9 +27,8 @@ abstract protected function tablesToTruncate(): array;
 
 	/**
 	 * Get checksums for the initial state of the database tables.
-	 *
-	 * @beforeClass
 	 */
+	#[BeforeClass]
 	final public static function initializeTableRowCounts(): void {
 		if (!isset(self::$conn)) {
 			self::$conn = DiContainer::make(mysqli::class);
@@ -38,9 +40,8 @@ final public static function initializeTableRowCounts(): void {
 	 * Any table that is modified during a test class should be declared in the
 	 * `tablesToTruncate()` method, and those tables will be reset after each
 	 * test method.
-	 *
-	 * @after
 	 */
+	#[After]
 	final protected function truncateTables(): void {
 		foreach ($this->tablesToTruncate() as $name) {
 			// Include hard-coded test database name as a safety precaution
@@ -52,9 +53,8 @@ final protected function truncateTables(): void {
 	 * All modified tables should be reset after each test, but here we perform
 	 * a final sanity check to make sure that no tables have changed checksums.
 	 * This is only done once per class because it is expensive!
-	 *
-	 * @afterClass
 	 */
+	#[AfterClass]
 	final public static function checkTables(): void {
 		$checksums = self::getChecksums();
 		$errors = [];

diff --git a/test/SmrTest/Container/DiContainerTest.php b/test/SmrTest/Container/DiContainerTest.php
@@ -2,13 +2,13 @@
 
 namespace SmrTest\Container;
 
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\RunInSeparateProcess;
 use PHPUnit\Framework\TestCase;
 use Smr\Container\DiContainer;
 use Smr\DatabaseProperties;
 
-/**
- * @covers \Smr\Container\DiContainer
- */
+#[CoversClass(DiContainer::class)]
 class DiContainerTest extends TestCase {
 
 	private const PHPDI_COMPILED_CONTAINER_FILE = '/tmp/CompiledContainer.php';
@@ -60,9 +60,7 @@ public function test_factory_DatabaseName(): void {
 		self::assertSame($dbName, 'smr_live_test');
 	}
 
-	/**
-	 * @runInSeparateProcess
-	 */
+	#[RunInSeparateProcess]
 	public function test_initialized(): void {
 		// Note that we need to run in a separate process since this is the
 		// only way to ensure that the DiContainer is not yet initialized

diff --git a/test/SmrTest/Container/ResettableContainerTraitTest.php b/test/SmrTest/Container/ResettableContainerTraitTest.php
@@ -2,12 +2,12 @@
 
 namespace SmrTest\Container;
 
+use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\TestCase;
 use Smr\Container\ResettableContainer;
+use Smr\Container\ResettableContainerTrait;
 
-/**
- * @covers \Smr\Container\ResettableContainerTrait
- */
+#[CoversClass(ResettableContainerTrait::class)]
 class ResettableContainerTraitTest extends TestCase {
 
 	public function test_not_initialized_by_definition(): void {

diff --git a/test/SmrTest/PhpFileInspectionTest.php b/test/SmrTest/PhpFileInspectionTest.php
@@ -3,11 +3,10 @@
 namespace SmrTest;
 
 use Overtrue\PHPLint\Linter;
+use PHPUnit\Framework\Attributes\CoversNothing;
 use PHPUnit\Framework\TestCase;
 
-/**
- * @coversNothing
- */
+#[CoversNothing]
 class PhpFileInspectionTest extends TestCase {
 
 	public function test_all_files_pass_phplint(): void {

diff --git a/test/SmrTest/lib/AbstractPlayerIntegrationTest.php b/test/SmrTest/lib/AbstractPlayerIntegrationTest.php
@@ -2,15 +2,15 @@
 
 namespace SmrTest\lib;
 
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\DataProvider;
 use Smr\AbstractPlayer;
 use Smr\Exceptions\PlayerNotFound;
 use Smr\Exceptions\UserError;
 use Smr\Game;
 use SmrTest\BaseIntegrationSpec;
 
-/**
- * @covers Smr\AbstractPlayer
- */
+#[CoversClass(AbstractPlayer::class)]
 class AbstractPlayerIntegrationTest extends BaseIntegrationSpec {
 
 	private static int $gameID = 42;
@@ -134,9 +134,7 @@ public function test_changePlayerNameByPlayer(): void {
 		self::assertTrue($player->isNameChanged());
 	}
 
-	/**
-	 * @dataProvider dataProvider_alignment
-	 */
+	#[DataProvider('dataProvider_alignment')]
 	public function test_alignment(int $alignment, bool $isGood, bool $isEvil, bool $isNeutral): void {
 		// Create a player with a specific alignment
 		$player = AbstractPlayer::createPlayer(1, self::$gameID, 'test', RACE_HUMAN, false);
@@ -152,7 +150,7 @@ public function test_alignment(int $alignment, bool $isGood, bool $isEvil, bool
 	/**
 	 * @return array<array{int, bool, bool, bool}>
 	 */
-	public function dataProvider_alignment(): array {
+	public static function dataProvider_alignment(): array {
 		// Test at, above, and below alignment thresholds
 		return [
 			[0, false, false, true],

diff --git a/test/SmrTest/lib/AbstractShipTest.php b/test/SmrTest/lib/AbstractShipTest.php
@@ -3,6 +3,8 @@
 namespace SmrTest\lib;
 
 use Exception;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Smr\AbstractPlayer;
@@ -12,9 +14,8 @@
 
 /**
  * This test is expected to not make any changes to the database.
- *
- * @covers Smr\AbstractShip
  */
+#[CoversClass(AbstractShip::class)]
 class AbstractShipTest extends TestCase {
 
 	private AbstractPlayer&MockObject $player; // will be mocked
@@ -185,11 +186,10 @@ public function test_hardware(): void {
 	}
 
 	/**
-	 * @dataProvider dataProvider_takeDamage
-	 *
 	 * @param WeaponDamageData $damage
 	 * @param TakenDamageData $expected
 	 */
+	#[DataProvider('dataProvider_takeDamage')]
 	public function test_takeDamage(string $case, array $damage, array $expected, int $shields, int $cds, int $armour): void {
 		// Set up a ship with a fixed amount of defenses
 		$this->player
@@ -207,7 +207,7 @@ public function test_takeDamage(string $case, array $damage, array $expected, in
 	/**
 	 * @return array<array{0: string, 1: WeaponDamageData, 2: TakenDamageData, 3: int, 4: int, 5: int}>
 	 */
-	public function dataProvider_takeDamage(): array {
+	public static function dataProvider_takeDamage(): array {
 		return [
 			[
 				'Do overkill damage (e.g. 1000 drone damage)',
@@ -365,10 +365,9 @@ public function dataProvider_takeDamage(): array {
 	}
 
 	/**
-	 * @dataProvider dataProvider_takeDamageFromMines
-	 *
 	 * @param TakenDamageData $expected
 	 */
+	#[DataProvider('dataProvider_takeDamageFromMines')]
 	public function test_takeDamageFromMines(string $case, int $damage, array $expected, int $shields, int $cds, int $armour): void {
 		// Set up a ship with a fixed amount of defenses
 		$this->player
@@ -391,7 +390,7 @@ public function test_takeDamageFromMines(string $case, int $damage, array $expec
 	/**
 	 * @return array<array{0: string, 1: int, 2: TakenDamageData, 3: int, 4: int, 5: int}>
 	 */
-	public function dataProvider_takeDamageFromMines(): array {
+	public static function dataProvider_takeDamageFromMines(): array {
 		return [
 			[
 				'Do overkill damage (e.g. 1000 mine damage)',

diff --git a/test/SmrTest/lib/AccountTest.php b/test/SmrTest/lib/AccountTest.php
@@ -2,14 +2,13 @@
 
 namespace SmrTest\lib;
 
+use PHPUnit\Framework\Attributes\CoversClass;
 use Smr\Account;
 use Smr\Exceptions\AccountNotFound;
 use Smr\SocialLogin\Facebook;
 use SmrTest\BaseIntegrationSpec;
 
-/**
- * @covers Smr\Account
- */
+#[CoversClass(Account::class)]
 class AccountTest extends BaseIntegrationSpec {
 
 	protected function tablesToTruncate(): array {

diff --git a/test/SmrTest/lib/AdminPermissionsTest.php b/test/SmrTest/lib/AdminPermissionsTest.php
@@ -2,13 +2,12 @@
 
 namespace SmrTest\lib;
 
+use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\TestCase;
 use Smr\AdminPermissions;
 use Smr\Pages\Admin\EnableGame;
 
-/**
- * @covers Smr\AdminPermissions
- */
+#[CoversClass(AdminPermissions::class)]
 class AdminPermissionsTest extends TestCase {
 
 	public function test_getPermissionInfo(): void {

diff --git a/test/SmrTest/lib/AllianceIntegrationTest.php b/test/SmrTest/lib/AllianceIntegrationTest.php
@@ -2,14 +2,13 @@
 
 namespace SmrTest\lib;
 
+use PHPUnit\Framework\Attributes\CoversClass;
 use Smr\Alliance;
 use Smr\Exceptions\AllianceNotFound;
 use Smr\Exceptions\UserError;
 use SmrTest\BaseIntegrationSpec;
 
-/**
- * @covers Smr\Alliance
- */
+#[CoversClass(Alliance::class)]
 class AllianceIntegrationTest extends BaseIntegrationSpec {
 
 	protected function tablesToTruncate(): array {

diff --git a/test/SmrTest/lib/BarDrinkTest.php b/test/SmrTest/lib/BarDrinkTest.php
@@ -2,12 +2,11 @@
 
 namespace SmrTest\lib;
 
+use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\TestCase;
 use Smr\BarDrink;
 
-/**
- * @covers Smr\BarDrink
- */
+#[CoversClass(BarDrink::class)]
 class BarDrinkTest extends TestCase {
 
 	public function test_getAll(): void {

diff --git a/test/SmrTest/lib/Blackjack/CardTest.php b/test/SmrTest/lib/Blackjack/CardTest.php
@@ -2,12 +2,12 @@
 
 namespace SmrTest\lib\Blackjack;
 
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 use Smr\Blackjack\Card;
 
-/**
- * @covers Smr\Blackjack\Card
- */
+#[CoversClass(Card::class)]
 class CardTest extends TestCase {
 
 	public function test_getCardID(): void {
@@ -34,9 +34,7 @@ public function test_isAce(): void {
 		}
 	}
 
-	/**
-	 * @dataProvider card_details_provider
-	 */
+	#[DataProvider('card_details_provider')]
 	public function test_card_details(int $cardID, string $rankName, string $suitName, int $value): void {
 		// check various details of a card
 		$card = new Card($cardID);
@@ -48,7 +46,7 @@ public function test_card_details(int $cardID, string $rankName, string $suitNam
 	/**
 	 * @return array<array{int, string, string, int}>
 	 */
-	public function card_details_provider(): array {
+	public static function card_details_provider(): array {
 		// spot check a handful of cards
 		return [
 			[0, 'A', 'hearts', 11],