Aude has uploaded a new change for review. https://gerrit.wikimedia.org/r/95783
Change subject: Move SpecialPage::getTitleFor exception to SpecialPageFactory ...................................................................... Move SpecialPage::getTitleFor exception to SpecialPageFactory The way the existing code was, it seems the exception condition was never hit, as one could enter a bogus special page title in SpecialPage::getTitleFor and it returns a Title object anyway. This change allows the exception to occur (which shouldn't!), it the SpecialPage name is invalid. Tests are also added for SpecialPage::getTitleFor() Change-Id: I6e427d047a2f8e58677eb65b50866f767078c255 --- M includes/SpecialPage.php M includes/SpecialPageFactory.php A tests/phpunit/includes/SpecialPageTest.php 3 files changed, 52 insertions(+), 6 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/83/95783/2 diff --git a/includes/SpecialPage.php b/includes/SpecialPage.php index a6195fc..20571d7 100644 --- a/includes/SpecialPage.php +++ b/includes/SpecialPage.php @@ -259,11 +259,7 @@ */ public static function getTitleFor( $name, $subpage = false, $fragment = '' ) { $name = SpecialPageFactory::getLocalNameFor( $name, $subpage ); - if ( $name ) { - return Title::makeTitle( NS_SPECIAL, $name, $fragment ); - } else { - throw new MWException( "Invalid special page name \"$name\"" ); - } + return Title::makeTitle( NS_SPECIAL, $name, $fragment ); } /** diff --git a/includes/SpecialPageFactory.php b/includes/SpecialPageFactory.php index 1ede0c1..01aae5d 100644 --- a/includes/SpecialPageFactory.php +++ b/includes/SpecialPageFactory.php @@ -542,6 +542,7 @@ * @param $name String * @param $subpage String|Bool * + * @throws MWException * @return String */ static function getLocalNameFor( $name, $subpage = false ) { @@ -563,7 +564,7 @@ } } if ( !$found ) { - wfWarn( "Did not find alias for special page '$name'. " . + throw new MWException( "Did not find alias for special page '$name'. " . "Perhaps no aliases are defined for it?" ); } } diff --git a/tests/phpunit/includes/SpecialPageTest.php b/tests/phpunit/includes/SpecialPageTest.php new file mode 100644 index 0000000..41764a3 --- /dev/null +++ b/tests/phpunit/includes/SpecialPageTest.php @@ -0,0 +1,49 @@ +<?php + +/** + * @covers SpecialPageTest + * + * @group Database + * + * @licence GNU GPL v2+ + * @author Katie Filbert < aude.w...@gmail.com > + */ +class SpecialPageTest extends MediaWikiTestCase { + + /** + * @dataProvider getTitleForProvider + */ + public function testGetTitleFor( $expected, $name ) { + $title = SpecialPage::getTitleFor( $name ); + $this->assertEquals( $expected, $title ); + } + + public function getTitleForProvider() { + return array( + array( Title::makeTitle( NS_SPECIAL, 'UserLogin' ), 'Userlogin' ) + ); + } + + /** + * @expectedException MWException + */ + public function testInvalidGetTitleFor() { + SpecialPage::getTitleFor( 'cat' ); + } + + /** + * @expectedException PHPUnit_Framework_Error_Notice + * @dataProvider getTitleForWithWarningProvider + */ + public function testGetTitleForWithWarning( $expected, $name ) { + $title = SpecialPage::getTitleFor( $name ); + $this->assertEquals( $expected, $title ); + } + + public function getTitleForWithWarningProvider() { + return array( + array( Title::makeTitle( NS_SPECIAL, 'UserLogin' ), 'UserLogin' ) + ); + } + +} -- To view, visit https://gerrit.wikimedia.org/r/95783 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6e427d047a2f8e58677eb65b50866f767078c255 Gerrit-PatchSet: 2 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: Aude <aude.w...@gmail.com> Gerrit-Reviewer: jenkins-bot _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits