test: refactor/speed up release note file test
authorAntoine Musso <hashar@free.fr>
Tue, 2 Jul 2019 10:07:50 +0000 (12:07 +0200)
committerAntoine Musso <hashar@free.fr>
Tue, 2 Jul 2019 10:12:19 +0000 (12:12 +0200)
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

tests/phpunit/documentation/ReleaseNotesTest.php

index d20fcff..3ac69ae 100644 (file)
@@ -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"
+               );
        }
 }