Merge pull request #817 from WordPress/feature/upgrade-phpunit-polyfi…

…lls-phpunit-10
WordPress · Jul 24, 2023 · cb9da04 · cb9da04
2 parents efb240e + 148c25e
commit cb9da04
Show file tree

Hide file tree

Showing 61 changed files with 332 additions and 171 deletions.
diff --git a/.gitattributes b/.gitattributes
@@ -9,3 +9,4 @@ tests/ export-ignore
 .phpcs.xml.dist export-ignore
 phpdoc.dist.xml export-ignore
 phpunit.xml.dist export-ignore
+phpunit10.xml.dist export-ignore
diff --git a/.github/workflows/quicktest.yml b/.github/workflows/quicktest.yml
@@ -90,12 +90,18 @@ jobs:
       - name: Access localhost on port 9002
         run: curl -i http://localhost:9002
 
-      - name: Show PHPUnit version
-        run: vendor/bin/phpunit --version
+      - name: Grab PHPUnit version
+        id: phpunit_version
+        run: echo "VERSION=$(vendor/bin/phpunit --version | grep --only-matching --max-count=1 --extended-regexp '\b[0-9]+\.[0-9]+')" >> $GITHUB_OUTPUT
 
-      - name: Run the unit tests
+      - name: Run the unit tests (PHPUnit < 10)
+        if: ${{ ! startsWith( steps.phpunit_version.outputs.VERSION, '10.' ) }}
         run: composer test
 
+      - name: Run the unit tests (PHPUnit 10+)
+        if: ${{ startsWith( steps.phpunit_version.outputs.VERSION, '10.' ) }}
+        run: composer test10
+
       - name: Stop proxy server
         continue-on-error: true
         run: |

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -114,16 +114,25 @@ jobs:
       - name: Access localhost on port 9002
         run: curl -i http://localhost:9002
 
-      - name: Show PHPUnit version
-        run: vendor/bin/phpunit --version
+      - name: Grab PHPUnit version
+        id: phpunit_version
+        run: echo "VERSION=$(vendor/bin/phpunit --version | grep --only-matching --max-count=1 --extended-regexp '\b[0-9]+\.[0-9]+')" >> $GITHUB_OUTPUT
 
-      - name: Run the unit tests, no code coverage
-        if: ${{ matrix.coverage == false }}
+      - name: Run the unit tests, no code coverage (PHPUnit < 10)
+        if: ${{  matrix.coverage == false && ! startsWith( steps.phpunit_version.outputs.VERSION, '10.' ) }}
         run: composer test
 
-      - name: Run the unit tests with code coverage
-        if: ${{ matrix.coverage == true }}
-        run: vendor/bin/phpunit --coverage-clover clover.xml
+      - name: Run the unit tests, no code coverage (PHPUnit 10+)
+        if: ${{  matrix.coverage == false && startsWith( steps.phpunit_version.outputs.VERSION, '10.' ) }}
+        run: composer test10
+
+      - name: Run the unit tests with code coverage (PHPUnit < 10)
+        if: ${{  matrix.coverage == true && ! startsWith( steps.phpunit_version.outputs.VERSION, '10.' ) }}
+        run: composer coverage -- --coverage-clover clover.xml
+
+      - name: Run the unit tests with code coverage (PHPUnit 10+)
+        if: ${{  matrix.coverage == true && startsWith( steps.phpunit_version.outputs.VERSION, '10.' ) }}
+        run: composer coverage10 -- --coverage-clover clover.xml
 
       - name: Stop proxy server
         continue-on-error: true

diff --git a/.phpcs.xml.dist b/.phpcs.xml.dist
@@ -247,4 +247,9 @@
 		<exclude-pattern>/tests/*\.php$</exclude-pattern>
 	</rule>
 
+	<!-- Using set_error_handler() in test code is fine. -->
+	<rule ref="WordPress.PHP.DevelopmentFunctions.error_log_set_error_handler">
+		<exclude-pattern>/tests/*\.php$</exclude-pattern>
+	</rule>
+
 </ruleset>
diff --git a/composer.json b/composer.json
@@ -48,7 +48,7 @@
     "dealerdirect/phpcodesniffer-composer-installer": "^0.7",
     "php-parallel-lint/php-parallel-lint": "^1.3.2",
     "php-parallel-lint/php-console-highlighter": "^1.0.0",
-    "yoast/phpunit-polyfills": "^1.0.0",
+    "yoast/phpunit-polyfills": "^2.0.0",
     "roave/security-advisories": "dev-latest"
   },
   "suggest": {
@@ -84,8 +84,23 @@
     "test": [
       "@php ./vendor/phpunit/phpunit/phpunit --no-coverage"
     ],
+    "test10": [
+      "@php ./vendor/phpunit/phpunit/phpunit -c phpunit10.xml.dist --no-coverage"
+    ],
     "coverage": [
       "@php ./vendor/phpunit/phpunit/phpunit"
+    ],
+    "coverage10": [
+      "@php ./vendor/phpunit/phpunit/phpunit -c phpunit10.xml.dist"
     ]
+  },
+  "scripts-descriptions": {
+    "lint": "Lint PHP files to find parse errors.",
+    "checkcs": "Check the entire codebase for code-style issues.",
+    "fixcs": "Fix all auto-fixable code-style issues in the entire codebase.",
+    "test": "Run the unit tests on PHPUnit 5.x - 9.x without code coverage.",
+    "test10": "Run the unit tests on PHPUnit 10.x without code coverage.",
+    "coverage": "Run the unit tests on PHPUnit 5.x - 9.x with code coverage.",
+    "coverage10": "Run the unit tests on PHPUnit 10.x with code coverage."
   }
 }
diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -4,6 +4,7 @@
 	xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/7.5/phpunit.xsd"
 	backupGlobals="true"
 	bootstrap="tests/bootstrap.php"
+	beStrictAboutTestsThatDoNotTestAnything="true"
 	convertDeprecationsToExceptions="true"
 	colors="true"
 	verbose="true"

diff --git a/phpunit10.xml.dist b/phpunit10.xml.dist
@@ -0,0 +1,36 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<phpunit
+	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+	xsi:noNamespaceSchemaLocation="http://schema.phpunit.de/10.1/phpunit.xsd"
+	backupGlobals="true"
+	bootstrap="tests/bootstrap.php"
+	beStrictAboutTestsThatDoNotTestAnything="true"
+	colors="true"
+	displayDetailsOnTestsThatTriggerErrors="true"
+	displayDetailsOnTestsThatTriggerWarnings="true"
+	displayDetailsOnTestsThatTriggerNotices="true"
+	displayDetailsOnTestsThatTriggerDeprecations="true"
+	displayDetailsOnIncompleteTests="true"
+	displayDetailsOnSkippedTests="true"
+	failOnWarning="true"
+	failOnNotice="true"
+	failOnDeprecation="true"
+	>
+	<testsuites>
+		<testsuite name="RequestsTests">
+			<directory suffix="Test.php">tests</directory>
+		</testsuite>
+	</testsuites>
+
+	<source>
+		<include>
+			<directory suffix=".php">./src/</directory>
+		</include>
+	</source>
+
+	<coverage includeUncoveredFiles="true" ignoreDeprecatedCodeUnits="true">
+		<report>
+			<html outputDirectory="tests/coverage" lowUpperBound="35" highLowerBound="90"/>
+		</report>
+	</coverage>
+</phpunit>
diff --git a/src/Transport/Curl.php b/src/Transport/Curl.php
@@ -177,6 +177,11 @@ public function request($url, $headers = [], $data = [], $options = []) {
 			$this->stream_handle = @fopen($options['filename'], 'wb');
 			if ($this->stream_handle === false) {
 				$error = error_get_last();
+				if (!is_array($error)) {
+					// Shouldn't be possible, but can happen in test situations.
+					$error = ['message' => 'Failed to open stream'];
+				}
+
 				throw new Exception($error['message'], 'fopen');
 			}
 		}

diff --git a/src/Transport/Fsockopen.php b/src/Transport/Fsockopen.php
@@ -282,6 +282,11 @@ public function request($url, $headers = [], $data = [], $options = []) {
 			$download = @fopen($options['filename'], 'wb');
 			if ($download === false) {
 				$error = error_get_last();
+				if (!is_array($error)) {
+					// Shouldn't be possible, but can happen in test situations.
+					$error = ['message' => 'Failed to open stream'];
+				}
+
 				throw new Exception($error['message'], 'fopen');
 			}
 		}

diff --git a/tests/Auth/Basic/BasicTest.php b/tests/Auth/Basic/BasicTest.php
@@ -44,14 +44,14 @@ public function testUsingArray($transport) {
 		$result = json_decode($request->body);
 		$this->assertIsObject($result, 'Decoded response body is not an object');
 
-		$this->assertObjectHasAttribute(
+		$this->assertObjectHasProperty(
 			'authenticated',
 			$result,
 			'Property "authenticated" not available in decoded response'
 		);
 		$this->assertTrue($result->authenticated, 'Authentication failed');
 
-		$this->assertObjectHasAttribute(
+		$this->assertObjectHasProperty(
 			'user',
 			$result,
 			'Property "user" not available in decoded response'
@@ -91,14 +91,14 @@ public function testUsingInstantiation($transport) {
 		$result = json_decode($request->body);
 		$this->assertIsObject($result, 'Decoded response body is not an object');
 
-		$this->assertObjectHasAttribute(
+		$this->assertObjectHasProperty(
 			'authenticated',
 			$result,
 			'Property "authenticated" not available in decoded response'
 		);
 		$this->assertTrue($result->authenticated, 'Authentication failed');
 
-		$this->assertObjectHasAttribute(
+		$this->assertObjectHasProperty(
 			'user',
 			$result,
 			'Property "user" not available in decoded response'
@@ -141,14 +141,14 @@ public function testUsingInstantiationWithDelayedSettingOfCredentials($transport
 		$result = json_decode($request->body);
 		$this->assertIsObject($result, 'Decoded response body is not an object');
 
-		$this->assertObjectHasAttribute(
+		$this->assertObjectHasProperty(
 			'authenticated',
 			$result,
 			'Property "authenticated" not available in decoded response'
 		);
 		$this->assertTrue($result->authenticated, 'Authentication failed');
 
-		$this->assertObjectHasAttribute(
+		$this->assertObjectHasProperty(
 			'user',
 			$result,
 			'Property "user" not available in decoded response'
@@ -189,12 +189,12 @@ public function testPOSTUsingInstantiation($transport) {
 		$result = json_decode($request->body);
 
 		$this->assertIsObject($result, 'Decoded response body is not an object');
-		$this->assertObjectHasAttribute(
+		$this->assertObjectHasProperty(
 			'headers',
 			$result,
 			'Property "headers" not available in decoded response'
 		);
-		$this->assertObjectHasAttribute(
+		$this->assertObjectHasProperty(
 			'Authorization',
 			$result->headers,
 			'Property "headers->Authorization" not available in decoded response'
@@ -205,7 +205,7 @@ public function testPOSTUsingInstantiation($transport) {
 		$this->assertArrayHasKey(1, $auth, 'Authorization header failed to be split into two parts');
 		$this->assertSame(base64_encode('user:passwd'), $auth[1], 'Unexpected authorization string in headers');
 
-		$this->assertObjectHasAttribute(
+		$this->assertObjectHasProperty(
 			'data',
 			$result,
 			'Property "data" not available in decoded response'

diff --git a/tests/Auth/Basic/ConstructorTest.php b/tests/Auth/Basic/ConstructorTest.php
@@ -34,7 +34,7 @@ public function testInvalidInputType($input) {
 	 *
 	 * @return array
 	 */
-	public function dataInvalidInputType() {
+	public static function dataInvalidInputType() {
 		return TypeProviderHelper::getAllExcept(TypeProviderHelper::GROUP_NULL, TypeProviderHelper::GROUP_ARRAY);
 	}
 
@@ -59,7 +59,7 @@ public function testInvalidArgumentCount($input) {
 	 *
 	 * @return array
 	 */
-	public function dataInvalidArgumentCount() {
+	public static function dataInvalidArgumentCount() {
 		return [
 			'empty array'                 => [[]],
 			'array with only one element' => [['user']],

diff --git a/tests/Autoload/AutoloadTest.php b/tests/Autoload/AutoloadTest.php
@@ -2,6 +2,7 @@
 
 namespace WpOrg\Requests\Tests\Autoload;
 
+use Exception;
 use Requests;
 use Requests_Exception_Transport_cURL;
 use Requests_Utility_FilteredIterator;
@@ -21,8 +22,17 @@ final class AutoloadTest extends TestCase {
 	 * Verify that a deprecation notice is thrown when the "old" Requests class is loaded via a require/include.
 	 */
 	public function testDeprecationNoticeThrownForOldRequestsClass() {
-		$this->expectDeprecation();
-		$this->expectDeprecationMessage(self::MSG);
+		// PHPUnit 10 compatible way to test the deprecation notice.
+		set_error_handler(
+			static function ($errno, $errstr) {
+				restore_error_handler();
+				throw new Exception($errstr, $errno);
+			},
+			E_USER_DEPRECATED
+		);
+
+		$this->expectException(Exception::class);
+		$this->expectExceptionMessage(self::MSG);
 
 		require_once dirname(dirname(__DIR__)) . '/library/Requests.php';
 	}
@@ -31,8 +41,17 @@ public function testDeprecationNoticeThrownForOldRequestsClass() {
 	 * Verify that a deprecation notice is thrown when one of the other "old" Requests classes is autoloaded.
 	 */
 	public function testDeprecationNoticeThrownForOtherOldRequestsClass() {
-		$this->expectDeprecation();
-		$this->expectDeprecationMessage(self::MSG);
+		// PHPUnit 10 compatible way to test the deprecation notice.
+		set_error_handler(
+			static function ($errno, $errstr) {
+				restore_error_handler();
+				throw new Exception($errstr, $errno);
+			},
+			E_USER_DEPRECATED
+		);
+
+		$this->expectException(Exception::class);
+		$this->expectExceptionMessage(self::MSG);
 
 		$this->assertNotEmpty(Requests_Exception_Transport_cURL::EASY);
 	}
@@ -91,7 +110,7 @@ public function testLoad($class_name, $expected) {
 	 *
 	 * @return array
 	 */
-	public function dataLoad() {
+	public static function dataLoad() {
 		return [
 			'Request for class not in this package should be rejected' => [
 				'class_name' => 'Unrelated\Package\ClassName',

diff --git a/tests/Cookie/ConstructorTest.php b/tests/Cookie/ConstructorTest.php
@@ -50,7 +50,7 @@ public function testInvalidValue($input) {
 	 *
 	 * @return array
 	 */
-	public function dataInvalidStringInput() {
+	public static function dataInvalidStringInput() {
 		return TypeProviderHelper::getAllExcept(TypeProviderHelper::GROUP_STRING);
 	}
 
@@ -75,7 +75,7 @@ public function testInvalidAttributes($input) {
 	 *
 	 * @return array
 	 */
-	public function dataInvalidAttributes() {
+	public static function dataInvalidAttributes() {
 		$except = array_intersect(TypeProviderHelper::GROUP_ITERABLE, TypeProviderHelper::GROUP_ARRAY_ACCESSIBLE);
 		return TypeProviderHelper::getAllExcept($except);
 	}
@@ -101,7 +101,7 @@ public function testInvalidFlags($input) {
 	 *
 	 * @return array
 	 */
-	public function dataInvalidFlags() {
+	public static function dataInvalidFlags() {
 		return TypeProviderHelper::getAllExcept(TypeProviderHelper::GROUP_ARRAY);
 	}
 
@@ -126,7 +126,7 @@ public function testInvalidReferenceTime($input) {
 	 *
 	 * @return array
 	 */
-	public function dataInvalidReferenceTime() {
+	public static function dataInvalidReferenceTime() {
 		return TypeProviderHelper::getAllExcept(TypeProviderHelper::GROUP_NULL, TypeProviderHelper::GROUP_INT);
 	}
 
@@ -152,7 +152,7 @@ public function testMinimalArguments($name, $value) {
 	 *
 	 * @return array
 	 */
-	public function dataMinimalArguments() {
+	public static function dataMinimalArguments() {
 		return [
 			'empty name and value' => [
 				'name'  => '',
@@ -199,7 +199,7 @@ public function testFlagMerging($flags, $expected) {
 	 *
 	 * @return array
 	 */
-	public function dataFlagMerging() {
+	public static function dataFlagMerging() {
 		return [
 			'empty array' => [
 				'flags'    => [],
@@ -269,7 +269,7 @@ public function testSetReferenceTime($time, $expected = null) {
 	 *
 	 * @return array
 	 */
-	public function dataSetReferenceTime() {
+	public static function dataSetReferenceTime() {
 		return [
 			'null' => [
 				'time' => null,
@@ -302,7 +302,7 @@ public function testAttributesAreNormalized($attributes, $expected) {
 	 *
 	 * @return array
 	 */
-	public function dataAttributesAreNormalized() {
+	public static function dataAttributesAreNormalized() {
 		return [
 			'empty array' => [
 				'attributes' => [],