Page MenuHomePhabricator

MediaWikiTestCaseTrait::creatNoOpMock should permit calling destructors
Closed, ResolvedPublic

Description

rMW709773ab5747: createNoOpMock() method for PHPUnit tests is awesome, but causes issues when mocking a class that implement __destruct. According to the documentation about destructors these will be called as soon as there are no other references to a particular object. I.E. unconditionally.

MovePageTest did include this; it had

$mock->expects( $this->never() )->method( $this->anythingBut( '__destruct' ) );

vs

$mock->expects( $this->never() )->method( $this->anything() );

found in MediaWikiTestCaseTrait::creatNoOpMock.

Event Timeline

Change 533543 had a related patch set uploaded (by Mainframe98; owner: Mainframe98):
[mediawiki/core@master] Permit destructors in MediaWikiTestCaseTrait::createNoOpMock

https://gerrit.wikimedia.org/r/533543

Is this actually causing problems? MovePage did have it because destruct would actually get called, but now it seems it doesn't. Why would destruct actually get called on the mock? It seems it doesn't really get called every time an object is destroyed.

I tried mocking the Language class, and all the tests that used creatNoOpMock() to mock Language failed until I added the exception for the destructor.

To give an example:

Using PHP 7.2.13-1+0~20181207100540.13+stretch~1.gbpf57305
PHPUnit 6.5.14 by Sebastian Bergmann and contributors.

.............................................................   61 / 1302 (  4%)
.............................................................  122 / 1302 (  9%)
.............................................................  183 / 1302 ( 14%)
........................................................IIIII  244 / 1302 ( 18%)
IIIIIIIIIII..................................................  305 / 1302 ( 23%)
..WW.........................................................  366 / 1302 ( 28%)
.............................................................  427 / 1302 ( 32%)
.............................................................  488 / 1302 ( 37%)
.............................................................  549 / 1302 ( 42%)
.............................................................  610 / 1302 ( 46%)
........................................F....................  671 / 1302 ( 51%)
.............................................................  732 / 1302 ( 56%)
.............................................................  793 / 1302 ( 60%)
.............................................................  854 / 1302 ( 65%)
......................................FFFFFF.................  915 / 1302 ( 70%)
...................................................F.........  976 / 1302 ( 74%)
....Language::__destruct() was not expected to be called.
vagrant@vagrant:~$

is the output I get. Makes it rather difficult to find out where the error is.

Change 533543 merged by jenkins-bot:
[mediawiki/core@master] Permit destructors in MediaWikiTestCaseTrait::createNoOpMock

https://gerrit.wikimedia.org/r/533543

Mainframe98 triaged this task as Medium priority.
Mainframe98 removed a project: Patch-For-Review.