Comments

tim.plunkett’s picture

Assigned: Unassigned » tim.plunkett
Status: Active » Needs review
StatusFileSize
new6.31 KB

Had to do some trickery in SimpleTestTest.php, please review it carefully.

kid_icarus’s picture

Status: Needs review » Needs work
+++ b/core/modules/simpletest/lib/Drupal/simpletest/Tests/SimpleTestTest.php
@@ -32,14 +40,14 @@ class SimpleTestTest extends WebTestBase {
-      parent::setUp('non_existent_module');
+      $this::$modules = array('non_existent_module');

This is the only change I'm uncertain of. Instead of $this::$modules, would self::$modules be a better choice? I'm kind of a OO noob so I wouldn't know better, but my reasoning lies in example #2 at http://www.php.net/manual/en/language.oop5.paamayim-nekudotayim.php

class OtherClass extends MyClass
{
    public static $my_static = 'static var';

    public static function doubleColon() {
        echo parent::CONST_VALUE . "\n";
        echo self::$my_static . "\n";
    }
}

and from http://stackoverflow.com/a/151976/1097146

Use $this to refer to the current object. Use self to refer to the current class. In other words, use $this->member for non-static members, use self::$member for static members.

Since in this case we're altering the class definition instead of an instantiation, I think self:: may be better?

kid_icarus’s picture

Status: Needs work » Needs review

Sorry, I'm not sure if this needs work or not, it definitely needs more review than my two noob cents :)

tim.plunkett’s picture

Without any patch:
SimpleTest functionality 114 passes, 0 fails, 0 exceptions, and 28 debug messages

With the patch from #1:
SimpleTest functionality 114 passes, 0 fails, 0 exceptions, and 28 debug messages

With the patch from #1, but with $self::$modules instead of $this::modules:
SimpleTest functionality 87 passes, 23 fails, 42 exceptions, and 26 debug messages

Basically, $self::$modules is read-only. $this::$modules lets you change the value.

kid_icarus’s picture

StatusFileSize
new530 bytes
new6.31 KB

@tim.plunket

I think you had a typo there. Instead of $self::$modules, it should be self::$modules.

I created a patch that uses self::$modules instead and it passed for me locally.

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community

Oh, hah!

That does make sense.

tim.plunkett’s picture

Status: Reviewed & tested by the community » Closed (duplicate)