Page MenuHomePhabricator
Create Task
Maniphest T136348

Cannot write to the test database in @dataProvider methods
Closed, DeclinedPublic
Actions

Description

[[ https://github.com/sebastianbergmann/phpunit/issues/836 | PHPUnit executes @dataProvider methods before PHPUnit_Framework_TestCase::setupBeforeClass() ]]. We set up the test database much later in the test suite setup sequence, in MediaWikiTestCase::run(). This means that @dataProvider methods cannot call methods that use the test database, like MediaWikiTestCase::getMutableTestUser().

It is possible to work around this by having the data provider pass a closure that returns the desired user object, instead of passing the user object directly. It is also possible to rely on the fact that we hard-code the primary immutable test sysop user to be UTSysop. Neither option is palatable.

It'd be better to ensure that the test database is ready before data provider methods are invoked. Alternately, we could make TestUser objects lazy, so that the users they represent are only inserted into the database on property / method access.

Related Objects

Event Timeline

ori created this task.May 26 2016, 6:36 PM
Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptMay 26 2016, 6:36 PM
Legoktm removed a project: Continuous-Integration-Infrastructure.May 26 2016, 6:37 PM
Anomie added a comment.May 26 2016, 7:23 PM

Alternately, we could make TestUser objects lazy, so that the users they represent are only inserted into the database on property / method access.

A case like https://gerrit.wikimedia.org/r/#/c/289369/20/tests/phpunit/includes/logging/NewUsersLogFormatterTest.php would need to access the user's properties (specifically, the user_id), though, or would have to use the callback method or otherwise construct the row inside the test instead of the data provider somehow.

Krinkle closed this task as Declined.Jan 12 2017, 8:20 PM
Krinkle subscribed.

Data providers are meant to be stateless and must be declared as static per PHPUnit design principles. As such, making the database ready in time wouldn't help as one still can't call instance methods from a static method.

Using a lazy object would work. MediaWikiTestCase::getMutableUser and TestUserRegistry seem to already be set up to work this way (it returns a TestUser object on which the developer must call getUser to get the actual User object). However it is not actually lazily loaded/written at the moment.

Alternatively, the specification could be declared declaratively. So that the method is called by the test function instead of the data provider. For example by having the data provider only indicate the requirements (as a constant and/or array) which are then passed to a TestUserRegistry method inside the test function. This is the kind of design that PHPUnit generally tries to encourage.

Anomie added a comment.Jan 12 2017, 8:59 PM
In T136348#2936760, @Krinkle wrote:

Alternatively, the specification could be declared declaratively. So that the method is called by the test function instead of the data provider. For example by having the data provider only indicate the requirements (as a constant and/or array) which are then passed to a TestUserRegistry method inside the test function. This is the kind of design that PHPUnit generally tries to encourage.

This description does not tell me what the preferred way would be to make a test such as https://gerrit.wikimedia.org/r/#/c/289369/20/tests/phpunit/includes/logging/NewUsersLogFormatterTest.php that tests that the correct user data is interpolated into the result. Consider also examples where the user ID rather than the name needs to be interpolated somehow.

Krinkle added a comment.Jan 13 2017, 2:12 AM
In T136348#2936813, @Anomie wrote:
In T136348#2936760, @Krinkle wrote:

Alternatively, the specification could be declared declaratively. So that the method is called by the test function instead of the data provider. For example by having the data provider only indicate the requirements (as a constant and/or array) which are then passed to a TestUserRegistry method inside the test function. This is the kind of design that PHPUnit generally tries to encourage.

This description does not tell me what the preferred way would be to make a test such as https://gerrit.wikimedia.org/r/#/c/289369/20/tests/phpunit/includes/logging/NewUsersLogFormatterTest.php that tests that the correct user data is interpolated into the result. Consider also examples where the user ID rather than the name needs to be interpolated somehow.

In such case, the object should be created in the test method and the ID retrieved there to form the said string. Could perhaps be generalised by having a placeholder in the declared string from the data provider and substitute it at run time.

Krinkle mentioned this in T70231: wfWikiID() returns different values from test and data provider.Jan 13 2017, 2:14 AM
Anomie added a comment.Jan 13 2017, 4:20 PM

In other words don't use a data provider at all, or write a postprocessor for the data provider that can handle all the possible "expect" data structures. That's... inconvenient.

Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:28 PM
Daimona mentioned this in T341731: CentralAuth tests broken: The data provider specified for ApiStructureTest::testParameters is invalid.Jul 12 2023, 8:54 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL