Consistently use LogicException for test-only methods

"BadMethodCallException" sounds like it would fit, but it does
have a very different meaning, described as "exception thrown if
a callback refers to an undefined method or if some arguments are
missing". This is not what's going on here. These are methods that
should only be called from unit tests.

This appears to be a common mistake, often copy-pasted.

Change-Id: Ib39e28f596a883481d5f526460a5c871c75f5313
This commit is contained in:
thiemowmde 2023-07-20 15:31:47 +02:00 committed by Thiemo Kreuz (WMDE)
parent 175563ded4
commit ef84619cd3
15 changed files with 24 additions and 26 deletions

View file

@ -178,12 +178,11 @@ class HookContainer implements SalvageableService {
* @param string $hook Name of hook to clear
*
* @internal For use by Hooks.php
* @throws MWException If not in testing mode.
* @codeCoverageIgnore
*/
public function clear( string $hook ): void {
if ( !defined( 'MW_PHPUNIT_TEST' ) && !defined( 'MW_PARSER_TEST' ) ) {
throw new MWException( 'Cannot reset hooks in operation.' );
throw new LogicException( 'Cannot reset hooks in operation.' );
}
$this->handlers[$hook] = [];

View file

@ -67,7 +67,7 @@ class Hooks {
public static function clear( $name ) {
wfDeprecated( __METHOD__, '1.35' );
if ( !defined( 'MW_PHPUNIT_TEST' ) && !defined( 'MW_PARSER_TEST' ) ) {
throw new BadMethodCallException( 'Cannot reset hooks in operation.' );
throw new LogicException( 'Cannot reset hooks in operation.' );
}
$hookContainer = MediaWikiServices::getInstance()->getHookContainer();
$hookContainer->clear( $name );

View file

@ -20,7 +20,6 @@
namespace MediaWiki;
use BadMethodCallException;
use BagOStuff;
use CentralIdLookup;
use Config;
@ -40,6 +39,7 @@ use Language;
use LinkCache;
use Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface;
use LocalisationCache;
use LogicException;
use MediaHandlerFactory;
use MediaWiki\Actions\ActionFactory;
use MediaWiki\Auth\AuthManager;
@ -299,7 +299,7 @@ class MediaWikiServices extends ServiceContainer {
if ( !self::$globalInstanceAllowed ) {
if ( defined( 'MW_PHPUNIT_TEST' ) ) {
// Fail hard in PHPUnit tests only
throw new BadMethodCallException( 'Premature access to service container' );
throw new LogicException( 'Premature access to service container' );
}
// TODO: getInstance() should always fail if $globalInstanceAllowed is false! (T153256)
wfDeprecatedMsg( 'Premature access to service container', '1.36' );
@ -343,7 +343,7 @@ class MediaWikiServices extends ServiceContainer {
*/
public static function forceGlobalInstance( MediaWikiServices $services ) {
if ( !defined( 'MW_PHPUNIT_TEST' ) && !defined( 'MW_PARSER_TEST' ) ) {
throw new BadMethodCallException( __METHOD__ . ' must not be used outside unit tests.' );
throw new LogicException( __METHOD__ . ' must not be used outside unit tests.' );
}
$old = self::getInstance();
@ -607,7 +607,7 @@ class MediaWikiServices extends ServiceContainer {
*/
public function resetServiceForTesting( $name, $destroy = true ) {
if ( !defined( 'MW_PHPUNIT_TEST' ) && !defined( 'MW_PARSER_TEST' ) ) {
throw new BadMethodCallException( 'resetServiceForTesting() must not be used outside unit tests.' );
throw new LogicException( 'resetServiceForTesting() must not be used outside unit tests.' );
}
$this->resetService( $name, $destroy );
@ -645,7 +645,7 @@ class MediaWikiServices extends ServiceContainer {
&& !defined( 'RUN_MAINTENANCE_IF_MAIN' )
&& defined( 'MW_SERVICE_BOOTSTRAP_COMPLETE' )
) {
throw new BadMethodCallException( $method . ' may only be called during bootstrapping and unit tests!' );
throw new LogicException( $method . ' may only be called during bootstrapping and unit tests!' );
}
}

View file

@ -20,8 +20,8 @@
namespace MediaWiki\Permissions;
use Article;
use BadMethodCallException;
use InvalidArgumentException;
use LogicException;
use MediaWiki\Actions\ActionFactory;
use MediaWiki\Block\BlockErrorFormatter;
use MediaWiki\Block\DatabaseBlock;
@ -1627,7 +1627,6 @@ class PermissionManager {
// Remove any rights that aren't allowed to the global-session user,
// unless there are no sessions for this endpoint.
if ( !defined( 'MW_NO_SESSION' ) ) {
// XXX: think what could be done with the below
$allowedRights = SessionManager::getGlobalSession()->getAllowedUserRights();
if ( $allowedRights !== null && !in_array( $right, $allowedRights, true ) ) {
@ -1810,7 +1809,7 @@ class PermissionManager {
*/
public function overrideUserRightsForTesting( $user, $rights = [] ) {
if ( !defined( 'MW_PHPUNIT_TEST' ) ) {
throw new BadMethodCallException( __METHOD__ . ' can not be called outside of tests' );
throw new LogicException( __METHOD__ . ' can not be called outside of tests' );
}
$this->usersRights[ $this->getRightsCacheKey( $user ) ] =
is_array( $rights ) ? $rights : [ $rights ];

View file

@ -1323,7 +1323,7 @@ abstract class ApiBase extends ContextSource {
*/
public static function clearCacheForTest(): void {
if ( !defined( 'MW_PHPUNIT_TEST' ) ) {
throw new RuntimeException( 'Not allowed outside tests' );
throw new LogicException( 'Not allowed outside tests' );
}
self::$filterIDsCache = [];
}

View file

@ -93,7 +93,7 @@ class CategoryMembershipChange {
*/
public function overrideNewForCategorizationCallback( callable $callback ) {
if ( !defined( 'MW_PHPUNIT_TEST' ) ) {
throw new BadMethodCallException( 'Cannot override newForCategorization callback in operation.' );
throw new LogicException( 'Cannot override newForCategorization callback in operation.' );
}
$this->newForCategorizationCallback = $callback;
}

View file

@ -568,8 +568,8 @@ class RequestContext implements IContextSource, MutableContext {
* Resets singleton returned by getMain(). Should be called only from unit tests.
*/
public static function resetMain() {
if ( !( defined( 'MW_PHPUNIT_TEST' ) || defined( 'MW_PARSER_TEST' ) ) ) {
throw new BadMethodCallException( __METHOD__ . '() should be called only from unit tests!' );
if ( !defined( 'MW_PHPUNIT_TEST' ) && !defined( 'MW_PARSER_TEST' ) ) {
throw new LogicException( __METHOD__ . '() should be called only from unit tests!' );
}
self::$instance = null;
}

View file

@ -407,7 +407,7 @@ class MWDebug {
string $regex, ?callable $callback = null
): void {
if ( !defined( 'MW_PHPUNIT_TEST' ) && !defined( 'MW_PARSER_TEST' ) ) {
throw new RuntimeException( __METHOD__ . ' can only be used in tests' );
throw new LogicException( __METHOD__ . ' can only be used in tests' );
}
self::$deprecationFilters[$regex] = $callback;
}

View file

@ -22,12 +22,12 @@ namespace MediaWiki\Logger;
use DateTimeZone;
use Error;
use LogicException;
use MediaWiki\WikiMap\WikiMap;
use MWDebug;
use MWExceptionHandler;
use Psr\Log\AbstractLogger;
use Psr\Log\LogLevel;
use RuntimeException;
use Throwable;
use UDPTransport;
use Wikimedia\AtEase\AtEase;
@ -140,7 +140,7 @@ class LegacyLogger extends AbstractLogger {
*/
public function setMinimumForTest( ?int $level ) {
if ( !defined( 'MW_PHPUNIT_TEST' ) ) {
throw new RuntimeException( 'Not allowed outside tests' );
throw new LogicException( 'Not allowed outside tests' );
}
// Set LEVEL_INFINITY if given null, or restore the original level.
$original = $this->minimumLevel;

View file

@ -307,7 +307,7 @@ class DeletePage {
*/
public function setIsDeletePageUnitTest( bool $test ): void {
if ( !defined( 'MW_PHPUNIT_TEST' ) ) {
throw new BadMethodCallException( __METHOD__ . ' can only be used in tests!' );
throw new LogicException( __METHOD__ . ' can only be used in tests!' );
}
$this->isDeletePageUnitTest = $test;
}

View file

@ -1147,7 +1147,7 @@ class ParserOptions {
*/
public static function clearStaticCache() {
if ( !defined( 'MW_PHPUNIT_TEST' ) && !defined( 'MW_PARSER_TEST' ) ) {
throw new RuntimeException( __METHOD__ . ' is just for testing' );
throw new LogicException( __METHOD__ . ' is just for testing' );
}
self::$defaults = null;
self::$lazyOptions = null;

View file

@ -632,7 +632,7 @@ class ExtensionRegistry {
public function setAttributeForTest( $name, array $value ) {
// @codeCoverageIgnoreStart
if ( !defined( 'MW_PHPUNIT_TEST' ) ) {
throw new RuntimeException( __METHOD__ . ' can only be used in tests' );
throw new LogicException( __METHOD__ . ' can only be used in tests' );
}
// @codeCoverageIgnoreEnd
if ( isset( $this->testAttributes[$name] ) ) {

View file

@ -23,10 +23,10 @@
namespace MediaWiki\Session;
use BadMethodCallException;
use BagOStuff;
use CachedBagOStuff;
use Config;
use LogicException;
use MediaWiki\HookContainer\HookContainer;
use MediaWiki\HookContainer\HookRunner;
use MediaWiki\MainConfigNames;
@ -1010,7 +1010,7 @@ class SessionManager implements SessionManagerInterface {
public static function resetCache() {
if ( !defined( 'MW_PHPUNIT_TEST' ) && !defined( 'MW_PARSER_TEST' ) ) {
// @codeCoverageIgnoreStart
throw new BadMethodCallException( __METHOD__ . ' may only be called from unit tests!' );
throw new LogicException( __METHOD__ . ' may only be called from unit tests!' );
// @codeCoverageIgnoreEnd
}

View file

@ -20,6 +20,7 @@
* @file
* @author Daniel Kinzler
*/
use MediaWiki\Interwiki\InterwikiLookup;
use MediaWiki\Linker\LinkTarget;
use MediaWiki\Page\PageReference;
@ -101,7 +102,7 @@ class MediaWikiTitleCodec implements TitleFormatter, TitleParser {
public function overrideCreateMalformedTitleExceptionCallback( callable $callback ) {
// @codeCoverageIgnoreStart
if ( !defined( 'MW_PHPUNIT_TEST' ) ) {
throw new RuntimeException( __METHOD__ . ' can only be used in tests' );
throw new LogicException( __METHOD__ . ' can only be used in tests' );
}
// @codeCoverageIgnoreEnd
$this->createMalformedTitleException = $callback;

View file

@ -178,11 +178,10 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
* @see DeferredUpdates::addCallableUpdate for callback signiture
*
* @return ScopedCallback to reset the overridden value
* @throws MWException
*/
public function overrideDeferredUpdatesAddCallableUpdateCallback( callable $callback ) {
if ( !defined( 'MW_PHPUNIT_TEST' ) ) {
throw new MWException(
throw new LogicException(
'Cannot override DeferredUpdates::addCallableUpdate callback in operation.'
);
}