From ffc23ee333fffb6fbf26605facea6d1134be7d39 Mon Sep 17 00:00:00 2001
From: Antoine Musso <hashar@free.fr>
Date: Tue, 2 Jul 2019 12:07:50 +0200
Subject: [PATCH] test: refactor/speed up release note file test

The test ReleaseNotesTest:testReleaseNotesFilesExistAndAreNotMalformed
takes 4 seconds on my machine. That is due to assertLessThanOrEqual
being invoked for each lines of various files at the root of the
repository. HISTORY has more than 20000 lines and assert functions are
rather slow.

Refactor the test:
* extract the logic to verify the file length of various files to a
  standalone test and with a dataprovider.
* flip the boolean logic ensuring whether the file exists. When it does
  not, PHP emit a warning which breaks the test anyway.
* Check the line lenght and collect errors in an array, then for each
  file run a single assertion to ensure the array of errors is empty.
  That effectively get rid of assertLessThanOrEqual overhead.
* Use assertSame() to show the full faulty line. assertEqual() shorten
  the arrays content.

Running tests went from 4 seconds to 700 ms on my machine.

Bug: T227067
Change-Id: I9bbbc6647ba9732b462e331e4b6d4acffe35e7cd
---
 .../documentation/ReleaseNotesTest.php        | 63 +++++++++++++------
 1 file changed, 43 insertions(+), 20 deletions(-)

diff --git a/tests/phpunit/documentation/ReleaseNotesTest.php b/tests/phpunit/documentation/ReleaseNotesTest.php
index d20fcff7b8..3ac69ae270 100644
--- a/tests/phpunit/documentation/ReleaseNotesTest.php
+++ b/tests/phpunit/documentation/ReleaseNotesTest.php
@@ -32,38 +32,61 @@ class ReleaseNotesTest extends MediaWikiTestCase {
 		foreach ( $notesFiles as $index => $fileName ) {
 			$this->assertFileLength( "Release Notes", $fileName );
 		}
+	}
+
+	public static function provideFilesAtRoot() {
+		global $IP;
 
-		// Also test the README and similar files
-		$otherFiles = [
-			"$IP/COPYING",
-			"$IP/FAQ",
-			"$IP/HISTORY",
-			"$IP/INSTALL",
-			"$IP/README",
-			"$IP/SECURITY"
+		$rootFiles = [
+			"COPYING",
+			"FAQ",
+			"HISTORY",
+			"INSTALL",
+			"README",
+			"SECURITY",
 		];
 
-		foreach ( $otherFiles as $index => $fileName ) {
-			$this->assertFileLength( "Help", $fileName );
+		$testCases = [];
+		foreach ( $rootFiles as $rootFile ) {
+			$testCases["$rootFile file"] = [ "$IP/$rootFile" ];
 		}
+		return $testCases;
+	}
+
+	/**
+	 * @dataProvider provideFilesAtRoot
+	 * @coversNothing
+	 */
+	public function testRootFilesHaveProperLineLength( $fileName ) {
+		$this->assertFileLength( "Help", $fileName );
 	}
 
 	private function assertFileLength( $type, $fileName ) {
-		$file = file( $fileName, FILE_IGNORE_NEW_LINES );
+		$lines = file( $fileName, FILE_IGNORE_NEW_LINES );
 
-		$this->assertFalse(
-			!$file,
+		$this->assertNotFalse(
+			$lines,
 			"$type file '$fileName' is inaccessible."
 		);
 
-		foreach ( $file as $i => $line ) {
+		$errors = [];
+		foreach ( $lines as $i => $line ) {
 			$num = $i + 1;
-			$this->assertLessThanOrEqual(
-				// FILE_IGNORE_NEW_LINES drops the \n at the EOL, so max length is 80 not 81.
-				80,
-				mb_strlen( $line ),
-				"$type file '$fileName' line $num, is longer than 80 chars:\n\t'$line'"
-			);
+
+			// FILE_IGNORE_NEW_LINES drops the \n at the EOL, so max length is 80 not 81.
+			$max_length = 80;
+
+			$length = mb_strlen( $line );
+			if ( $length <= $max_length ) {
+				continue;
+			}
+			$errors[] = "line $num: length $length > $max_length:\n$line";
 		}
+		# Using assertSame() to show the full line
+		$this->assertSame(
+			[], $errors,
+			"$type file '$fileName' lines " .
+			"have at most $max_length characters"
+		);
 	}
 }
-- 
2.20.1