I'm getting these two strict PHP warnings when I clear caches and go to the Testing page (which is reading all of the .test files to build the list of tests):
* Strict warning: Declaration of FieldUIManageFieldsTestCase::setUp() should be compatible with that of FieldUITestCase::setUp() in _registry_check_code() (line 2811 of /opt/www/eclipsework/d7dev/includes/bootstrap.inc).
* Strict warning: Declaration of FieldUIManageDisplayTestCase::setUp() should be compatible with that of FieldUITestCase::setUp() in _registry_check_code() (line 2811 of /opt/www/eclipsework/d7dev/includes/bootstrap.inc).
It looks like this is legit. FieldUiTestCase:
class FieldUITestCase extends DrupalWebTestCase {
function setUp($modules = array()) {
...
class FieldUIManageDisplayTestCase extends FieldUITestCase {
function setUp() {
...
Comment | File | Size | Author |
---|---|---|---|
#16 | drupal.fieldui-test-strict.16.patch | 811 bytes | lyricnz |
#13 | drupal.fieldui-test-strict.12.patch | 779 bytes | lyricnz |
#6 | drupal.fieldui-test-strict.6.patch | 575 bytes | sun |
Comments
Comment #1
izus CreditAttribution: izus commentedI have this error too when running tests for the first time following the steps on Documentation http://drupal.org/node/1128366
Comment #2
jhodgdonActually this now needs to be addressed in d8 and backported to d7.
Comment #3
lyricnz CreditAttribution: lyricnz commentedCannot reproduce, either with PHP 5.2 or 5.3. Can you describe more about how you got this error?
Comment #4
jhodgdonYou have to turn on all the strict warnings in PHP, in your php.ini, in order to see these warnings, and then restart your Apache to make the settings take effect.
Comment #5
lyricnz CreditAttribution: lyricnz commentedOK, I didn't know about E_STRICT. I can reproduce now.
Technically, it's warning because we're breaking polymorphism. We actually do the same thing on just about every test-case, but due to some wierdness in PHP about compile-time vs run-time, most don't show warnings.
Comment #6
sunComment #7
jhodgdonI think/hope you meant to set this to "needs review" until someone has had a chance to review your patch?
Comment #8
sunSure, if you want to hold off a braindeadsimple fix like this. We have better things to do.
Comment #9
jhodgdonI was under the impression that *all* patches need a review. I don't go around marking my doc patches that fix simple typos RTBC... maybe I should? I don't think so.
Comment #10
izus CreditAttribution: izus commented#6: drupal.fieldui-test-strict.6.patch queued for re-testing.
Comment #11
lyricnz CreditAttribution: lyricnz commentedCode is fine.
Comment #12
webchickUh. Sorry, but I totally can't follow what's going on there. I mean, I can of course read the code. But we need a comment there to the effect of "Like FieldTestCase::setUp(), allow for modules passed in as either an array or strings."
Additionally, both DrupalWebTestCase::setUp() and FieldTestCase::setUp() write this line as:
...rather than consolidating. We should do the same here, for legibility.
So, yes. Even "braindeadsimple" fixes need reviews. :)
Comment #13
lyricnz CreditAttribution: lyricnz commentedCut and paste from FieldTestCase
Comment #15
sunYou missed this line from the last patch.
Powered by Dreditor.
Comment #16
lyricnz CreditAttribution: lyricnz commentedLame.
Comment #17
sunComment #18
webchickAwesome, thanks!
Committed to 8.x and 7.x.