Make PHPUnit tests more robust

- Enable UnusedSuppressionPlugin as an additional guarantee in case future tests will rely on suppression instead of output matching. - Expand $backupStaticAttributesExcludeList with more properties backed up in phan's BaseTest. - Add plugin_instances_cache to the exclude list as well, so that phan won't attepmt to load the same plugin multiple times. This allows us to actually enable UnusedSuppressionPlugin, and also remove the class_exists hack from both plugin files. - Clear type memoizations before each test, in case they cause any conflict, like phan's AbstractPhanFileTest. Change-Id: Ib7ca397c78c41e805ffaa611e00b002ec7e5c88a
wikimedia · Oct 22, 2023 · 34590f1 · 34590f1
1 parent c80924d
commit 34590f1
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 34 deletions.
diff --git a/src/Plugin/NoBaseExceptionPlugin.php b/src/Plugin/NoBaseExceptionPlugin.php
@@ -7,20 +7,14 @@
 use Phan\PluginV3;
 use Phan\PluginV3\PostAnalyzeNodeCapability;
 
-// HACK: Avoid redeclaring the class if phan `require`s this file multiple times (e.g., in tests, where
-// we reset the plugin list)
-if ( !class_exists( NoBaseExceptionPlugin::class ) ) {
-	class NoBaseExceptionPlugin extends PluginV3 implements PostAnalyzeNodeCapability {
-
-		public const ISSUE_TYPE = 'MediaWikiNoBaseException';
-
-		/**
-		 * @inheritDoc
-		 */
-		public static function getPostAnalyzeNodeVisitorClassName(): string {
-			return NoBaseExceptionVisitor::class;
-		}
-
+class NoBaseExceptionPlugin extends PluginV3 implements PostAnalyzeNodeCapability {
+	public const ISSUE_TYPE = 'MediaWikiNoBaseException';
+
+	/**
+	 * @inheritDoc
+	 */
+	public static function getPostAnalyzeNodeVisitorClassName(): string {
+		return NoBaseExceptionVisitor::class;
 	}
 }
 

diff --git a/src/Plugin/NoEmptyIfDefinedPlugin.php b/src/Plugin/NoEmptyIfDefinedPlugin.php
@@ -7,20 +7,14 @@
 use Phan\PluginV3;
 use Phan\PluginV3\PostAnalyzeNodeCapability;
 
-// HACK: Avoid redeclaring the class if phan `require`s this file multiple times (e.g., in tests, where
-// we reset the plugin list)
-if ( !class_exists( NoEmptyIfDefinedPlugin::class ) ) {
-	class NoEmptyIfDefinedPlugin extends PluginV3 implements PostAnalyzeNodeCapability {
-
-		public const ISSUE_TYPE = 'MediaWikiNoEmptyIfDefined';
-
-		/**
-		 * @inheritDoc
-		 */
-		public static function getPostAnalyzeNodeVisitorClassName(): string {
-			return NoEmptyIfDefinedVisitor::class;
-		}
-
+class NoEmptyIfDefinedPlugin extends PluginV3 implements PostAnalyzeNodeCapability {
+	public const ISSUE_TYPE = 'MediaWikiNoEmptyIfDefined';
+
+	/**
+	 * @inheritDoc
+	 */
+	public static function getPostAnalyzeNodeVisitorClassName(): string {
+		return NoEmptyIfDefinedVisitor::class;
 	}
 }
 

diff --git a/tests/PluginTest.php b/tests/PluginTest.php
@@ -2,6 +2,7 @@
 
 use Phan\CLIBuilder;
 use Phan\Config;
+use Phan\Language\Type;
 use Phan\Output\Printer\PlainTextPrinter;
 use Phan\Phan;
 use Phan\Plugin\ConfigPluginSet;
@@ -18,6 +19,13 @@ class PluginTest extends TestCase {
 	 * @inheritDoc
 	 */
 	protected $backupStaticAttributesExcludeList = [
+		'Phan\AST\PhanAnnotationAdder' => [
+			'closures_for_kind',
+		],
+		'Phan\AST\ASTReverter' => [
+			'closure_map',
+			'noop',
+		],
 		'Phan\Language\Type' => [
 			'canonical_object_map',
 			'internal_fn_cache',
@@ -40,6 +48,11 @@ class PluginTest extends TestCase {
 		'SecurityCheckPlugin' => [
 			'pluginInstance'
 		],
+		// Back this up to avoid loading plugins multiple times (which is slow, and most importantly fails because
+		// the class would be re-declared every time).
+		'Phan\Plugin\ConfigPluginSet' => [
+			'plugin_instances_cache'
+		]
 	];
 
 	/**
@@ -54,6 +67,7 @@ private function runPhan( string $plugin, string $cfgFile, bool $usePolyfill ):
 		}
 
 		Config::reset();
+		Type::clearAllMemoizations();
 		$codeBase = require __DIR__ . '/../vendor/phan/phan/src/codebase.php';
 		$cliBuilder = new CLIBuilder();
 		$cliBuilder->setOption( 'project-root-directory', __DIR__ );

diff --git a/tests/plugins/NoBaseExceptionPlugin/config.php b/tests/plugins/NoBaseExceptionPlugin/config.php
@@ -4,9 +4,7 @@
 
 $baseCfg = require __DIR__ . '/../base-plugin-test-config.php';
 
-$baseCfg['plugins'] = [
-	Config::projectPath( __DIR__ . '/../../../src/Plugin/NoBaseExceptionPlugin.php' )
-];
+$baseCfg['plugins'][] = Config::projectPath( __DIR__ . '/../../../src/Plugin/NoBaseExceptionPlugin.php' );
 
 $baseCfg['whitelist_issue_types'] = [
 	// Fun fact: we can't use NoBaseExceptionPlugin::ISSUE_TYPE as that would trigger the composer autoloader

diff --git a/tests/plugins/NoEmptyIfDefinedPlugin/config.php b/tests/plugins/NoEmptyIfDefinedPlugin/config.php
@@ -4,9 +4,7 @@
 
 $baseCfg = require __DIR__ . '/../base-plugin-test-config.php';
 
-$baseCfg['plugins'] = [
-	Config::projectPath( __DIR__ . '/../../../src/Plugin/NoEmptyIfDefinedPlugin.php' )
-];
+$baseCfg['plugins'][] = Config::projectPath( __DIR__ . '/../../../src/Plugin/NoEmptyIfDefinedPlugin.php' );
 
 // Note: we're not setting whitelist_issue_types because the plugin is a bit hacky and we want to make sure that
 // no builtin issue types are emitted when they shouldn't be.

diff --git a/tests/plugins/base-plugin-test-config.php b/tests/plugins/base-plugin-test-config.php
@@ -5,4 +5,7 @@
 	'quick_mode' => false,
 	'dead_code_detection' => false,
 	'redundant_condition_detection' => false,
+	'plugins' => [
+		'UnusedSuppressionPlugin',
+	],
 ];