Require ClassMatchesFilename sniff to pass for most of tests/

phpunit-patch-coverage assumes that the filename matches the classname
as a performance optimization. And for most test cases, this is true. We
should enforce this with PHPCS, mostly to help developers not make
mistakes.

Test cases that have mock classes will need to ensure that the test case
class that matches the filename comes first, since that's the only class
the sniff will look at.

Tests in GlobalFunctions/ and maintenance/ are still exempted for now,
since they don't match yet.

Change-Id: Iede341504290f5ba2da1c81908069ba9d465600f

diff --git a/.phpcs.xml b/.phpcs.xml
index 88a6f8c..30f9201 100644 (file)
--- a/.phpcs.xml
+++ b/.phpcs.xml
@@ -175,8 +175,12 @@
                <exclude-pattern>*/profileinfo\.php</exclude-pattern>
                <!-- Language converters use the pattern of 2 classes in one file -->
                <exclude-pattern>*/languages/*\.php</exclude-pattern>
-               <!-- We don't care that much about violations in tests -->
-               <exclude-pattern>*/tests/*\.php</exclude-pattern>
+               <!-- Skip violations in some tests for now -->
+               <exclude-pattern>*/tests/parser/*\.php</exclude-pattern>
+               <exclude-pattern>*/tests/phan/*\.php</exclude-pattern>
+               <exclude-pattern>*/tests/phpunit/maintenance/*\.php</exclude-pattern>
+               <exclude-pattern>*/tests/phpunit/bootstrap\.php</exclude-pattern>
+               <exclude-pattern>*/tests/phpunit/phpunit\.php</exclude-pattern>
        </rule>
        <rule ref="MediaWiki.Files.ClassMatchesFilename.WrongCase">
                <!--
@@ -207,8 +211,9 @@
                <exclude-pattern>*/maintenance/storage/checkStorage\.php</exclude-pattern>
                <exclude-pattern>*/maintenance/storage/recompressTracked\.php</exclude-pattern>
                <exclude-pattern>*/maintenance/storage/trackBlobs\.php</exclude-pattern>
-               <!-- We don't care that much about violations in tests -->
-               <exclude-pattern>*/tests/*\.php</exclude-pattern>
+               <!-- Skip violations in some tests for now -->
+               <exclude-pattern>*/tests/phpunit/includes/GlobalFunctions/*\.php</exclude-pattern>
+               <exclude-pattern>*/tests/phpunit/maintenance/*\.php</exclude-pattern>
        </rule>
 
        <rule ref="Generic.Files.OneObjectStructurePerFile.MultipleFound">

diff --git a/tests/phpunit/includes/db/DatabaseSqliteTest.php b/tests/phpunit/includes/db/DatabaseSqliteTest.php
index 78af11d..eebb045 100644 (file)
--- a/tests/phpunit/includes/db/DatabaseSqliteTest.php
+++ b/tests/phpunit/includes/db/DatabaseSqliteTest.php
@@ -5,26 +5,6 @@ use Wikimedia\Rdbms\Database;
 use Wikimedia\Rdbms\DatabaseSqlite;
 use Wikimedia\Rdbms\ResultWrapper;
 
-class DatabaseSqliteMock extends DatabaseSqlite {
-       public static function newInstance( array $p = [] ) {
-               $p['dbFilePath'] = ':memory:';
-               $p['schema'] = false;
-
-               return Database::factory( 'SqliteMock', $p );
-       }
-
-       function query( $sql, $fname = '', $tempIgnore = false ) {
-               return true;
-       }
-
-       /**
-        * Override parent visibility to public
-        */
-       public function replaceVars( $s ) {
-               return parent::replaceVars( $s );
-       }
-}
-
 /**
  * @group sqlite
  * @group Database
@@ -539,3 +519,23 @@ class DatabaseSqliteTest extends MediaWikiTestCase {
                $this->assertTrue( $attributes[Database::ATTR_DB_LEVEL_LOCKING] );
        }
 }
+
+class DatabaseSqliteMock extends DatabaseSqlite {
+       public static function newInstance( array $p = [] ) {
+               $p['dbFilePath'] = ':memory:';
+               $p['schema'] = false;
+
+               return Database::factory( 'SqliteMock', $p );
+       }
+
+       function query( $sql, $fname = '', $tempIgnore = false ) {
+               return true;
+       }
+
+       /**
+        * Override parent visibility to public
+        */
+       public function replaceVars( $s ) {
+               return parent::replaceVars( $s );
+       }
+}

diff --git a/tests/phpunit/includes/deferred/SearchUpdateTest.php b/tests/phpunit/includes/deferred/SearchUpdateTest.php
index 9e4dbea..74a5e3c 100644 (file)
--- a/tests/phpunit/includes/deferred/SearchUpdateTest.php
+++ b/tests/phpunit/includes/deferred/SearchUpdateTest.php
@@ -1,20 +1,5 @@
 <?php
 
-class MockSearch extends SearchEngine {
-       public static $id;
-       public static $title;
-       public static $text;
-
-       public function __construct( $db ) {
-       }
-
-       public function update( $id, $title, $text ) {
-               self::$id = $id;
-               self::$title = $title;
-               self::$text = $text;
-       }
-}
-
 /**
  * @group Search
  */
@@ -85,3 +70,18 @@ EOT
                );
        }
 }
+
+class MockSearch extends SearchEngine {
+       public static $id;
+       public static $title;
+       public static $text;
+
+       public function __construct( $db ) {
+       }
+
+       public function update( $id, $title, $text ) {
+               self::$id = $id;
+               self::$title = $title;
+               self::$text = $text;
+       }
+}

diff --git a/tests/phpunit/includes/libs/MemoizedCallableTest.php b/tests/phpunit/includes/libs/MemoizedCallableTest.php
index 9127a30..628cca0 100644 (file)
--- a/tests/phpunit/includes/libs/MemoizedCallableTest.php
+++ b/tests/phpunit/includes/libs/MemoizedCallableTest.php
@@ -1,27 +1,6 @@
 <?php
 /**
- * A MemoizedCallable subclass that stores function return values
- * in an instance property rather than APC or APCu.
- */
-class ArrayBackedMemoizedCallable extends MemoizedCallable {
-       private $cache = [];
-
-       protected function fetchResult( $key, &$success ) {
-               if ( array_key_exists( $key, $this->cache ) ) {
-                       $success = true;
-                       return $this->cache[$key];
-               }
-               $success = false;
-               return false;
-       }
-
-       protected function storeResult( $key, $result ) {
-               $this->cache[$key] = $result;
-       }
-}
-
-/**
- * PHP Unit tests for MemoizedCallable class.
+ * PHPUnit tests for MemoizedCallable class.
  * @covers MemoizedCallable
  */
 class MemoizedCallableTest extends PHPUnit\Framework\TestCase {
@@ -140,3 +119,24 @@ class MemoizedCallableTest extends PHPUnit\Framework\TestCase {
                $memoized = new MemoizedCallable( 14 );
        }
 }
+
+/**
+ * A MemoizedCallable subclass that stores function return values
+ * in an instance property rather than APC or APCu.
+ */
+class ArrayBackedMemoizedCallable extends MemoizedCallable {
+       private $cache = [];
+
+       protected function fetchResult( $key, &$success ) {
+               if ( array_key_exists( $key, $this->cache ) ) {
+                       $success = true;
+                       return $this->cache[$key];
+               }
+               $success = false;
+               return false;
+       }
+
+       protected function storeResult( $key, $result ) {
+               $this->cache[$key] = $result;
+       }
+}

diff --git a/tests/phpunit/includes/poolcounter/PoolCounterTest.php b/tests/phpunit/includes/poolcounter/PoolCounterTest.php
index f7f2013..19baf5a 100644 (file)
--- a/tests/phpunit/includes/poolcounter/PoolCounterTest.php
+++ b/tests/phpunit/includes/poolcounter/PoolCounterTest.php
@@ -1,14 +1,5 @@
 <?php
 
-// We will use this class with getMockForAbstractClass to create a concrete mock class.
-// That call will die if the contructor is not public, unless we use disableOriginalConstructor(),
-// in which case we could not test the constructor.
-abstract class PoolCounterAbstractMock extends PoolCounter {
-       public function __construct() {
-               call_user_func_array( 'parent::__construct', func_get_args() );
-       }
-}
-
 /**
  * @covers PoolCounter
  */
@@ -82,3 +73,12 @@ class PoolCounterTest extends MediaWikiTestCase {
                );
        }
 }
+
+// We will use this class with getMockForAbstractClass to create a concrete mock class.
+// That call will die if the contructor is not public, unless we use disableOriginalConstructor(),
+// in which case we could not test the constructor.
+abstract class PoolCounterAbstractMock extends PoolCounter {
+       public function __construct() {
+               call_user_func_array( 'parent::__construct', func_get_args() );
+       }
+}