Merge pull request #97 from Yoast/feature/improve-error-messages

Polyfills: improve handling of custom error message
Yoast · Mar 12, 2023 · 981b266 · 981b266
2 parents 07ad708 + e873683
commit 981b266
Show file tree

Hide file tree

Showing 6 changed files with 234 additions and 36 deletions.
diff --git a/src/Polyfills/AssertClosedResource.php b/src/Polyfills/AssertClosedResource.php
@@ -25,12 +25,14 @@ trait AssertClosedResource {
 	 * @return void
 	 */
 	public static function assertIsClosedResource( $actual, $message = '' ) {
-		if ( $message === '' ) {
-			$exporter = \class_exists( 'SebastianBergmann\Exporter\Exporter' ) ? new Exporter() : new Exporter_In_Phar();
-			$message  = \sprintf( 'Failed asserting that %s is of type "resource (closed)"', $exporter->export( $actual ) );
+		$exporter = \class_exists( 'SebastianBergmann\Exporter\Exporter' ) ? new Exporter() : new Exporter_In_Phar();
+		$msg      = \sprintf( 'Failed asserting that %s is of type "resource (closed)"', $exporter->export( $actual ) );
+
+		if ( $message !== '' ) {
+			$msg = $message . \PHP_EOL . $msg;
 		}
 
-		static::assertTrue( ResourceHelper::isClosedResource( $actual ), $message );
+		static::assertTrue( ResourceHelper::isClosedResource( $actual ), $msg );
 	}
 
 	/**
@@ -42,16 +44,18 @@ public static function assertIsClosedResource( $actual, $message = '' ) {
 	 * @return void
 	 */
 	public static function assertIsNotClosedResource( $actual, $message = '' ) {
-		if ( $message === '' ) {
-			$exporter = \class_exists( 'SebastianBergmann\Exporter\Exporter' ) ? new Exporter() : new Exporter_In_Phar();
-			$type     = $exporter->export( $actual );
-			if ( $type === 'NULL' ) {
-				$type = 'resource (closed)';
-			}
-			$message = \sprintf( 'Failed asserting that %s is not of type "resource (closed)"', $type );
+		$exporter = \class_exists( 'SebastianBergmann\Exporter\Exporter' ) ? new Exporter() : new Exporter_In_Phar();
+		$type     = $exporter->export( $actual );
+		if ( $type === 'NULL' ) {
+			$type = 'resource (closed)';
+		}
+		$msg = \sprintf( 'Failed asserting that %s is not of type "resource (closed)"', $type );
+
+		if ( $message !== '' ) {
+			$msg = $message . \PHP_EOL . $msg;
 		}
 
-		static::assertFalse( ResourceHelper::isClosedResource( $actual ), $message );
+		static::assertFalse( ResourceHelper::isClosedResource( $actual ), $msg );
 	}
 
 	/**

diff --git a/src/Polyfills/AssertFileDirectory.php b/src/Polyfills/AssertFileDirectory.php
@@ -32,11 +32,12 @@ public static function assertIsReadable( $filename, $message = '' ) {
 			throw PHPUnit_Util_InvalidArgumentHelper::factory( 1, 'string' );
 		}
 
-		if ( $message === '' ) {
-			$message = \sprintf( 'Failed asserting that "%s" is readable', $filename );
+		$msg = \sprintf( 'Failed asserting that "%s" is readable', $filename );
+		if ( $message !== '' ) {
+			$msg = $message . \PHP_EOL . $msg;
 		}
 
-		static::assertTrue( \is_readable( $filename ), $message );
+		static::assertTrue( \is_readable( $filename ), $msg );
 	}
 
 	/**
@@ -54,11 +55,12 @@ public static function assertNotIsReadable( $filename, $message = '' ) {
 			throw PHPUnit_Util_InvalidArgumentHelper::factory( 1, 'string' );
 		}
 
-		if ( $message === '' ) {
-			$message = \sprintf( 'Failed asserting that "%s" is not readable', $filename );
+		$msg = \sprintf( 'Failed asserting that "%s" is not readable', $filename );
+		if ( $message !== '' ) {
+			$msg = $message . \PHP_EOL . $msg;
 		}
 
-		static::assertFalse( \is_readable( $filename ), $message );
+		static::assertFalse( \is_readable( $filename ), $msg );
 	}
 
 	/**
@@ -76,11 +78,12 @@ public static function assertIsWritable( $filename, $message = '' ) {
 			throw PHPUnit_Util_InvalidArgumentHelper::factory( 1, 'string' );
 		}
 
-		if ( $message === '' ) {
-			$message = \sprintf( 'Failed asserting that "%s" is writable', $filename );
+		$msg = \sprintf( 'Failed asserting that "%s" is writable', $filename );
+		if ( $message !== '' ) {
+			$msg = $message . \PHP_EOL . $msg;
 		}
 
-		static::assertTrue( \is_writable( $filename ), $message );
+		static::assertTrue( \is_writable( $filename ), $msg );
 	}
 
 	/**
@@ -98,11 +101,12 @@ public static function assertNotIsWritable( $filename, $message = '' ) {
 			throw PHPUnit_Util_InvalidArgumentHelper::factory( 1, 'string' );
 		}
 
-		if ( $message === '' ) {
-			$message = \sprintf( 'Failed asserting that "%s" is not writable', $filename );
+		$msg = \sprintf( 'Failed asserting that "%s" is not writable', $filename );
+		if ( $message !== '' ) {
+			$msg = $message . \PHP_EOL . $msg;
 		}
 
-		static::assertFalse( \is_writable( $filename ), $message );
+		static::assertFalse( \is_writable( $filename ), $msg );
 	}
 
 	/**
@@ -120,11 +124,12 @@ public static function assertDirectoryExists( $directory, $message = '' ) {
 			throw PHPUnit_Util_InvalidArgumentHelper::factory( 1, 'string' );
 		}
 
-		if ( $message === '' ) {
-			$message = \sprintf( 'Failed asserting that directory "%s" exists', $directory );
+		$msg = \sprintf( 'Failed asserting that directory "%s" exists', $directory );
+		if ( $message !== '' ) {
+			$msg = $message . \PHP_EOL . $msg;
 		}
 
-		static::assertTrue( \is_dir( $directory ), $message );
+		static::assertTrue( \is_dir( $directory ), $msg );
 	}
 
 	/**
@@ -142,11 +147,12 @@ public static function assertDirectoryNotExists( $directory, $message = '' ) {
 			throw PHPUnit_Util_InvalidArgumentHelper::factory( 1, 'string' );
 		}
 
-		if ( $message === '' ) {
-			$message = \sprintf( 'Failed asserting that directory "%s" does not exist', $directory );
+		$msg = \sprintf( 'Failed asserting that directory "%s" does not exist', $directory );
+		if ( $message !== '' ) {
+			$msg = $message . \PHP_EOL . $msg;
 		}
 
-		static::assertFalse( \is_dir( $directory ), $message );
+		static::assertFalse( \is_dir( $directory ), $msg );
 	}
 
 	/**

diff --git a/src/Polyfills/AssertStringContains.php b/src/Polyfills/AssertStringContains.php
@@ -69,11 +69,12 @@ public static function assertStringContainsStringIgnoringCase( $needle, $haystac
 	 */
 	public static function assertStringNotContainsString( $needle, $haystack, $message = '' ) {
 		if ( $needle === '' ) {
-			if ( $message === '' ) {
-				$message = "Failed asserting that '{$haystack}' does not contain \"{$needle}\".";
+			$msg = "Failed asserting that '{$haystack}' does not contain \"\".";
+			if ( $message !== '' ) {
+				$msg = $message . \PHP_EOL . $msg;
 			}
 
-			static::fail( $message );
+			static::fail( $msg );
 		}
 
 		static::assertNotContains( $needle, $haystack, $message );
@@ -90,11 +91,12 @@ public static function assertStringNotContainsString( $needle, $haystack, $messa
 	 */
 	public static function assertStringNotContainsStringIgnoringCase( $needle, $haystack, $message = '' ) {
 		if ( $needle === '' ) {
-			if ( $message === '' ) {
-				$message = "Failed asserting that '{$haystack}' does not contain \"{$needle}\".";
+			$msg = "Failed asserting that '{$haystack}' does not contain \"\".";
+			if ( $message !== '' ) {
+				$msg = $message . \PHP_EOL . $msg;
 			}
 
-			static::fail( $message );
+			static::fail( $msg );
 		}
 
 		static::assertNotContains( $needle, $haystack, $message, true );

diff --git a/tests/Polyfills/AssertClosedResourceNotResourceTest.php b/tests/Polyfills/AssertClosedResourceNotResourceTest.php
@@ -39,6 +39,21 @@ public function testAssertIsClosedResource( $value ) {
 		$this->assertIsClosedResource( $value );
 	}
 
+	/**
+	 * Verify that the assertIsClosedResource() method fails a test with the correct custom failure message,
+	 * when the custom $message parameter has been passed.
+	 *
+	 * @return void
+	 */
+	public function testAssertIsClosedResourceFailsWithCustomMessage() {
+		$pattern = '`^This assertion failed for reason XYZ\s+Failed asserting that .+? is of type ["]?resource \(closed\)["]?`s';
+
+		$this->expectException( $this->getAssertionFailedExceptionName() );
+		$this->expectExceptionMessageMatches( $pattern );
+
+		$this->assertIsClosedResource( 'text string', 'This assertion failed for reason XYZ' );
+	}
+
 	/**
 	 * Verify that the assertIsNotClosedResource() method passes the test when the variable
 	 * passed is not a resource.
@@ -53,6 +68,24 @@ public function testAssertIsNotClosedResource( $value ) {
 		self::assertIsNotClosedResource( $value );
 	}
 
+	/**
+	 * Verify that the assertIsNotClosedResource() method fails a test with the correct custom failure message,
+	 * when the custom $message parameter has been passed.
+	 *
+	 * @return void
+	 */
+	public function testAssertIsNotClosedResourceFailsWithCustomMessage() {
+		$pattern = '`^This assertion failed for reason XYZ\s+Failed asserting that .+? not of type ["]?resource \(closed\)["]?`s';
+
+		$this->expectException( $this->getAssertionFailedExceptionName() );
+		$this->expectExceptionMessageMatches( $pattern );
+
+		$resource = \opendir( __DIR__ . '/Fixtures/' );
+		\closedir( $resource );
+
+		$this->assertIsNotClosedResource( $resource, 'This assertion failed for reason XYZ' );
+	}
+
 	/**
 	 * Verify that the shouldClosedResourceAssertionBeSkipped() method returns true for non-resources.
 	 *