r104563 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r104562‎ | r104563 | r104564 >
Date:14:23, 29 November 2011
Author:hashar
Status:ok (Comments)
Tags:
Comment:
Fix fatal error when running newParserTest

The delayed parser test feature was expecting a ParserTest object which is
almost never the case when using the NewParserTest for PHPUnit. The class
name is forged after the file name! They at least all extends NewParserTest

no polymorphism? no multiple hinting. Be creative the PHP way by writing
your own code :-(
Modified paths:
  • /trunk/phase3/tests/testHelpers.inc (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/testHelpers.inc
——@@ -543,7 +543,12 @@
544544 * Called whenever we actually want to run the hook.
545545 * Should be the case if we found the parserTest is not disabled
546546 */
547 - public function unleash( ParserTest &$parserTest ) {
 547+ public function unleash( &$parserTest ) {
 548+ if( !($parserTest instanceof ParserTest || $parserTest instanceof NewParserTest
 549+ ) ) {
 550+ throw new MWException( __METHOD__ . " must be passed an instance of ParserTest or NewParserTest classes\n" );
 551+ }
 552+
548553 # Trigger delayed hooks. Any failure will make us abort
549554 foreach( $this->hooks as $hook ) {
550555 $ret = $parserTest->requireHook( $hook );

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r104561parserTest: delay hooks execution to the point we really need them...hashar13:54, 29 November 2011

Comments

#Comment by Hashar (talk | contribs)   14:24, 29 November 2011

Follow up r104561

#Comment by Platonides (talk | contribs)   17:41, 29 November 2011

Forging it from the name was easy and much more convenient than eg. md5(mt_rand()) :)

In an ideal world, ParserTest and NewParserTest wouldn't duplicate code.

#Comment by Hashar (talk | contribs)   08:40, 1 December 2011

I was ranting about PHP not supporting multiple types/class hinting nor polymorphism. Forging the class name using the .txt filename is fine :p

I will eventually one day merge the ParserTest and NewParserTest class. Not sure why we had to duplicate them. NewParserTests was probably written as a replacement for ParserTests.

#Comment by Platonides (talk | contribs)   16:04, 30 November 2011

Hehe, it triggers a warning 'Use of unknown class NewParserTest in line 547'

Status & tagging log