From a72b082811ac1f5b3a61371501828a53f6f0b145 Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Wed, 19 Jun 2019 23:44:39 +0100
Subject: [PATCH] resourceloader: Document which FileModule methods use a DB

Also, for the unit test, disable the two methods we use there
that can get called. The unintended side-effects of these two
methods was the only reason it used `@group Database`.
Removing that makes the test a bit faster as well.

Enforce this via MediaWikiServices for this suite to avoid an
untracked dependency slipping back in in the future.

Bug: T225730
Change-Id: I6c54466e9517d9899bc39f8f9bb946369c0a526d
---
 .../resourceloader/ResourceLoaderFileModule.php  |  6 ++++++
 tests/phpunit/ResourceLoaderTestCase.php         | 16 ++++++++++++++++
 .../ResourceLoaderFileModuleTest.php             | 12 +++++++-----
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/includes/resourceloader/ResourceLoaderFileModule.php b/includes/resourceloader/ResourceLoaderFileModule.php
index 47c8987a00..7093ab1dc0 100644
--- a/includes/resourceloader/ResourceLoaderFileModule.php
+++ b/includes/resourceloader/ResourceLoaderFileModule.php
@@ -24,6 +24,12 @@
 
 /**
  * ResourceLoader module based on local JavaScript/CSS files.
+ *
+ * The following public methods can query the database:
+ *
+ * - getDefinitionSummary / … / ResourceLoaderModule::getFileDependencies.
+ * - getVersionHash / getDefinitionSummary / … / ResourceLoaderModule::getFileDependencies.
+ * - getStyles / ResourceLoaderModule::saveFileDependencies.
  */
 class ResourceLoaderFileModule extends ResourceLoaderModule {
 
diff --git a/tests/phpunit/ResourceLoaderTestCase.php b/tests/phpunit/ResourceLoaderTestCase.php
index e5c0cbc41d..64693b0dc5 100644
--- a/tests/phpunit/ResourceLoaderTestCase.php
+++ b/tests/phpunit/ResourceLoaderTestCase.php
@@ -157,6 +157,12 @@ class ResourceLoaderTestModule extends ResourceLoaderModule {
 	}
 }
 
+/**
+ * A more constrained and testable variant of ResourceLoaderFileModule.
+ *
+ * - Implements getLessVars() support.
+ * - Disables database persistance of discovered file dependencies.
+ */
 class ResourceLoaderFileTestModule extends ResourceLoaderFileModule {
 	protected $lessVars = [];
 
@@ -172,6 +178,16 @@ class ResourceLoaderFileTestModule extends ResourceLoaderFileModule {
 	public function getLessVars( ResourceLoaderContext $context ) {
 		return $this->lessVars;
 	}
+
+	/** @return array */
+	protected function getFileDependencies( ResourceLoaderContext $context ) {
+		// No-op
+		return [];
+	}
+
+	protected function saveFileDependencies( ResourceLoaderContext $context, $refs ) {
+		// No-op
+	}
 }
 
 class ResourceLoaderFileModuleTestingSubclass extends ResourceLoaderFileModule {
diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderFileModuleTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderFileModuleTest.php
index 1585cbcef0..5be0f9b79f 100644
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderFileModuleTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderFileModuleTest.php
@@ -1,7 +1,6 @@
 <?php
 
 /**
- * @group Database
  * @group ResourceLoader
  */
 class ResourceLoaderFileModuleTest extends ResourceLoaderTestCase {
@@ -19,11 +18,14 @@ class ResourceLoaderFileModuleTest extends ResourceLoaderTestCase {
 			}
 		);
 		$this->setService( 'SkinFactory', $skinFactory );
+
+		// This test is not expected to query any database
+		MediaWiki\MediaWikiServices::disableStorageBackend();
 	}
 
 	private static function getModules() {
 		$base = [
-			'localBasePath' => realpath( __DIR__ ),
+			'localBasePath' => __DIR__,
 		];
 
 		return [
@@ -229,12 +231,12 @@ class ResourceLoaderFileModuleTest extends ResourceLoaderTestCase {
 	 */
 	public function testMixedCssAnnotations() {
 		$basePath = __DIR__ . '/../../data/css';
-		$testModule = new ResourceLoaderFileModule( [
+		$testModule = new ResourceLoaderFileTestModule( [
 			'localBasePath' => $basePath,
 			'styles' => [ 'test.css' ],
 		] );
 		$testModule->setName( 'testing' );
-		$expectedModule = new ResourceLoaderFileModule( [
+		$expectedModule = new ResourceLoaderFileTestModule( [
 			'localBasePath' => $basePath,
 			'styles' => [ 'expected.css' ],
 		] );
@@ -319,7 +321,7 @@ class ResourceLoaderFileModuleTest extends ResourceLoaderTestCase {
 	 */
 	public function testBomConcatenation() {
 		$basePath = __DIR__ . '/../../data/css';
-		$testModule = new ResourceLoaderFileModule( [
+		$testModule = new ResourceLoaderFileTestModule( [
 			'localBasePath' => $basePath,
 			'styles' => [ 'bom.css' ],
 		] );
-- 
2.20.1