r92962 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r92961‎ | r92962 | r92963 >
Date:20:23, 23 July 2011
Author:platonides
Status:ok (Comments)
Tags:
Comment:
It is stupid to test that a void method returns null. Add a comment explaining that
what we are testing is actually the function existence (see r91123 summary)
Change the onArticle* to is_callable()s, so that a db is not needed.
Modified paths:
  • /trunk/phase3/tests/phpunit/includes/ArticleTest.php (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/phpunit/includes/ArticleTest.php
@@ -50,14 +50,17 @@
5151 "Article get/set magic on update to new field" );
5252 }
5353
 54+ /**
 55+ * Checks for the existence of the backwards compatibility static functions (forwarders to WikiPage class)
 56+ */
5457 function testStaticFunctions() {
5558 $this->assertEquals( WikiPage::selectFields(), Article::selectFields(),
5659 "Article static functions" );
57 - $this->assertEquals( null, Article::onArticleCreate( $this->title ),
 60+ $this->assertEquals( true, is_callable( "Article::onArticleCreate" ),
5861 "Article static functions" );
59 - $this->assertEquals( null, Article::onArticleDelete( $this->title ),
 62+ $this->assertEquals( true, is_callable( "Article::onArticleDelete" ),
6063 "Article static functions" );
61 - $this->assertEquals( null, ImagePage::onArticleEdit( $this->title ),
 64+ $this->assertEquals( true, is_callable( "ImagePage::onArticleEdit" ),
6265 "Article static functions" );
6366 $this->assertTrue( is_string( CategoryPage::getAutosummary( '', '', 0 ) ),
6467 "Article static functions" );

Follow-up revisions

RevisionCommit summaryAuthorDate
r964701.17wmf1: MFT r92962, r93062, r93093, r93385, r93468, r93473, r94350, r94502,...catrope19:07, 7 September 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r91123* Split off WikiPage class from Article, WikiFilePage class from ImagePage, a...aaron22:09, 29 June 2011

Comments

#Comment by Catrope (talk | contribs)   15:01, 6 September 2011

Shouldn't assertEquals( true, $foo ) be assertTrue( $foo )?

#Comment by Platonides (talk | contribs)   21:19, 8 September 2011

I think that assertTrue() is newer than assertEquals(). Not sure which version I was using in July or if I tried to keep BC compatibility.

Status & tagging log