From f3690d48bc666825c6452911d901549a9c14a37c Mon Sep 17 00:00:00 2001
From: Tim Starling <tstarling@wikimedia.org>
Date: Mon, 27 Feb 2017 16:08:36 +1100
Subject: [PATCH] Fix @covers for FileBackend

It's up to the developer to choose the level of granularity they are
aspiring for in their test coverage. But that granularity level should be
reflected in @covers. When you have hundreds of lines of code testing a
single-line function (FileBackend::doOperation), that's a good hint that
something went wrong.

FileBackendTest is 2700 lines of code and appears to be aiming to test
the whole filebackend module. So I adjusted @covers to reflect that.

Change-Id: Iacd8cf475d1761c3b32c739983343619a9509d6b
---
 .../includes/filebackend/FileBackendTest.php  | 90 ++++++++-----------
 .../filebackend/SwiftFileBackendTest.php      |  9 +-
 2 files changed, 43 insertions(+), 56 deletions(-)

diff --git a/tests/phpunit/includes/filebackend/FileBackendTest.php b/tests/phpunit/includes/filebackend/FileBackendTest.php
index c3d31d1222..f77720653f 100644
--- a/tests/phpunit/includes/filebackend/FileBackendTest.php
+++ b/tests/phpunit/includes/filebackend/FileBackendTest.php
@@ -4,6 +4,44 @@
  * @group FileRepo
  * @group FileBackend
  * @group medium
+ *
+ * @covers FileBackend
+ *
+ * @covers CopyFileOp
+ * @covers CreateFileOp
+ * @covers DeleteFileOp
+ * @covers DescribeFileOp
+ * @covers FSFile
+ * @covers FSFileBackend
+ * @covers FSFileBackendDirList
+ * @covers FSFileBackendFileList
+ * @covers FSFileBackendList
+ * @covers FSFileOpHandle
+ * @covers FileBackendDBRepoWrapper
+ * @covers FileBackendError
+ * @covers FileBackendGroup
+ * @covers FileBackendMultiWrite
+ * @covers FileBackendStore
+ * @covers FileBackendStoreOpHandle
+ * @covers FileBackendStoreShardDirIterator
+ * @covers FileBackendStoreShardFileIterator
+ * @covers FileBackendStoreShardListIterator
+ * @covers FileJournal
+ * @covers FileOp
+ * @covers FileOpBatch
+ * @covers HTTPFileStreamer
+ * @covers LockManagerGroup
+ * @covers MemoryFileBackend
+ * @covers MoveFileOp
+ * @covers MySqlLockManager
+ * @covers NullFileJournal
+ * @covers NullFileOp
+ * @covers StoreFileOp
+ * @covers TempFSFile
+ *
+ * @covers FSLockManager
+ * @covers LockManager
+ * @covers NullLockManager
  */
 class FileBackendTest extends MediaWikiTestCase {
 
@@ -89,7 +127,6 @@ class FileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_testIsStoragePath
-	 * @covers FileBackend::isStoragePath
 	 */
 	public function testIsStoragePath( $path, $isStorePath ) {
 		$this->assertEquals( $isStorePath, FileBackend::isStoragePath( $path ),
@@ -114,7 +151,6 @@ class FileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_testSplitStoragePath
-	 * @covers FileBackend::splitStoragePath
 	 */
 	public function testSplitStoragePath( $path, $res ) {
 		$this->assertEquals( $res, FileBackend::splitStoragePath( $path ),
@@ -139,7 +175,6 @@ class FileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_normalizeStoragePath
-	 * @covers FileBackend::normalizeStoragePath
 	 */
 	public function testNormalizeStoragePath( $path, $res ) {
 		$this->assertEquals( $res, FileBackend::normalizeStoragePath( $path ),
@@ -169,7 +204,6 @@ class FileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_testParentStoragePath
-	 * @covers FileBackend::parentStoragePath
 	 */
 	public function testParentStoragePath( $path, $res ) {
 		$this->assertEquals( $res, FileBackend::parentStoragePath( $path ),
@@ -191,7 +225,6 @@ class FileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_testExtensionFromPath
-	 * @covers FileBackend::extensionFromPath
 	 */
 	public function testExtensionFromPath( $path, $res ) {
 		$this->assertEquals( $res, FileBackend::extensionFromPath( $path ),
@@ -224,9 +257,6 @@ class FileBackendTest extends MediaWikiTestCase {
 		$this->tearDownFiles();
 	}
 
-	/**
-	 * @covers FileBackend::doOperation
-	 */
 	private function doTestStore( $op ) {
 		$backendName = $this->backendClass();
 
@@ -286,7 +316,6 @@ class FileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_testCopy
-	 * @covers FileBackend::doOperation
 	 */
 	public function testCopy( $op ) {
 		$this->backend = $this->singleBackend;
@@ -407,7 +436,6 @@ class FileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_testMove
-	 * @covers FileBackend::doOperation
 	 */
 	public function testMove( $op ) {
 		$this->backend = $this->singleBackend;
@@ -529,7 +557,6 @@ class FileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_testDelete
-	 * @covers FileBackend::doOperation
 	 */
 	public function testDelete( $op, $withSource, $okStatus ) {
 		$this->backend = $this->singleBackend;
@@ -621,7 +648,6 @@ class FileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_testDescribe
-	 * @covers FileBackend::doOperation
 	 */
 	public function testDescribe( $op, $withSource, $okStatus ) {
 		$this->backend = $this->singleBackend;
@@ -722,7 +748,6 @@ class FileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_testCreate
-	 * @covers FileBackend::doOperation
 	 */
 	public function testCreate( $op, $alreadyExists, $okStatus, $newSize ) {
 		$this->backend = $this->singleBackend;
@@ -843,9 +868,6 @@ class FileBackendTest extends MediaWikiTestCase {
 		return $cases;
 	}
 
-	/**
-	 * @covers FileBackend::doQuickOperations
-	 */
 	public function testDoQuickOperations() {
 		$this->backend = $this->singleBackend;
 		$this->doTestDoQuickOperations();
@@ -1056,7 +1078,6 @@ class FileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_testGetFileStat
-	 * @covers FileBackend::getFileStat
 	 */
 	public function testGetFileStat( $path, $content, $alreadyExists ) {
 		$this->backend = $this->singleBackend;
@@ -1132,7 +1153,6 @@ class FileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_testGetFileStat
-	 * @covers FileBackend::streamFile
 	 */
 	public function testStreamFile( $path, $content, $alreadyExists ) {
 		$this->backend = $this->singleBackend;
@@ -1231,8 +1251,6 @@ class FileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_testGetFileContents
-	 * @covers FileBackend::getFileContents
-	 * @covers FileBackend::getFileContentsMulti
 	 */
 	public function testGetFileContents( $source, $content ) {
 		$this->backend = $this->singleBackend;
@@ -1304,7 +1322,6 @@ class FileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_testGetLocalCopy
-	 * @covers FileBackend::getLocalCopy
 	 */
 	public function testGetLocalCopy( $source, $content ) {
 		$this->backend = $this->singleBackend;
@@ -1390,7 +1407,6 @@ class FileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_testGetLocalReference
-	 * @covers FileBackend::getLocalReference
 	 */
 	public function testGetLocalReference( $source, $content ) {
 		$this->backend = $this->singleBackend;
@@ -1467,10 +1483,6 @@ class FileBackendTest extends MediaWikiTestCase {
 		return $cases;
 	}
 
-	/**
-	 * @covers FileBackend::getLocalCopy
-	 * @covers FileBackend::getLocalReference
-	 */
 	public function testGetLocalCopyAndReference404() {
 		$this->backend = $this->singleBackend;
 		$this->tearDownFiles();
@@ -1499,7 +1511,6 @@ class FileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_testGetFileHttpUrl
-	 * @covers FileBackend::getFileHttpUrl
 	 */
 	public function testGetFileHttpUrl( $source, $content ) {
 		$this->backend = $this->singleBackend;
@@ -1544,8 +1555,6 @@ class FileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_testPrepareAndClean
-	 * @covers FileBackend::prepare
-	 * @covers FileBackend::clean
 	 */
 	public function testPrepareAndClean( $path, $isOK ) {
 		$this->backend = $this->singleBackend;
@@ -1626,9 +1635,6 @@ class FileBackendTest extends MediaWikiTestCase {
 		$this->tearDownFiles();
 	}
 
-	/**
-	 * @covers FileBackend::clean
-	 */
 	private function doTestRecursiveClean() {
 		$backendName = $this->backendClass();
 
@@ -1673,9 +1679,6 @@ class FileBackendTest extends MediaWikiTestCase {
 		}
 	}
 
-	/**
-	 * @covers FileBackend::doOperations
-	 */
 	public function testDoOperations() {
 		$this->backend = $this->singleBackend;
 		$this->tearDownFiles();
@@ -1763,9 +1766,6 @@ class FileBackendTest extends MediaWikiTestCase {
 			"Correct file SHA-1 of $fileC" );
 	}
 
-	/**
-	 * @covers FileBackend::doOperations
-	 */
 	public function testDoOperationsPipeline() {
 		$this->backend = $this->singleBackend;
 		$this->tearDownFiles();
@@ -1862,9 +1862,6 @@ class FileBackendTest extends MediaWikiTestCase {
 			"Correct file SHA-1 of $fileC" );
 	}
 
-	/**
-	 * @covers FileBackend::doOperations
-	 */
 	public function testDoOperationsFailing() {
 		$this->backend = $this->singleBackend;
 		$this->tearDownFiles();
@@ -1939,9 +1936,6 @@ class FileBackendTest extends MediaWikiTestCase {
 			"Correct file SHA-1 of $fileA" );
 	}
 
-	/**
-	 * @covers FileBackend::getFileList
-	 */
 	public function testGetFileList() {
 		$this->backend = $this->singleBackend;
 		$this->tearDownFiles();
@@ -2117,10 +2111,6 @@ class FileBackendTest extends MediaWikiTestCase {
 		}
 	}
 
-	/**
-	 * @covers FileBackend::getTopDirectoryList
-	 * @covers FileBackend::getDirectoryList
-	 */
 	public function testGetDirectoryList() {
 		$this->backend = $this->singleBackend;
 		$this->tearDownFiles();
@@ -2334,10 +2324,6 @@ class FileBackendTest extends MediaWikiTestCase {
 		$this->assertEquals( [], $items, "Directory listing is empty." );
 	}
 
-	/**
-	 * @covers FileBackend::lockFiles
-	 * @covers FileBackend::unlockFiles
-	 */
 	public function testLockCalls() {
 		$this->backend = $this->singleBackend;
 		$this->doTestLockCalls();
diff --git a/tests/phpunit/includes/filebackend/SwiftFileBackendTest.php b/tests/phpunit/includes/filebackend/SwiftFileBackendTest.php
index 95ffb7057b..6acc94388a 100644
--- a/tests/phpunit/includes/filebackend/SwiftFileBackendTest.php
+++ b/tests/phpunit/includes/filebackend/SwiftFileBackendTest.php
@@ -4,6 +4,11 @@
  * @group FileRepo
  * @group FileBackend
  * @group medium
+ *
+ * @covers SwiftFileBackend
+ * @covers SwiftFileBackendDirList
+ * @covers SwiftFileBackendFileList
+ * @covers SwiftFileBackendList
  */
 class SwiftFileBackendTest extends MediaWikiTestCase {
 	/** @var TestingAccessWrapper Proxy to SwiftFileBackend */
@@ -28,8 +33,6 @@ class SwiftFileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_testSanitizeHdrs
-	 * @covers SwiftFileBackend::sanitizeHdrs
-	 * @covers SwiftFileBackend::getCustomHeaders
 	 */
 	public function testSanitizeHdrs( $raw, $sanitized ) {
 		$hdrs = $this->backend->sanitizeHdrs( [ 'headers' => $raw ] );
@@ -92,7 +95,6 @@ class SwiftFileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_testGetMetadataHeaders
-	 * @covers SwiftFileBackend::getMetadataHeaders
 	 */
 	public function testGetMetadataHeaders( $raw, $sanitized ) {
 		$hdrs = $this->backend->getMetadataHeaders( $raw );
@@ -120,7 +122,6 @@ class SwiftFileBackendTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider provider_testGetMetadata
-	 * @covers SwiftFileBackend::getMetadata
 	 */
 	public function testGetMetadata( $raw, $sanitized ) {
 		$hdrs = $this->backend->getMetadata( $raw );
-- 
2.20.1