Page MenuHomePhabricator

Refactor the /content-providers/MwApiContentProvider.php class for ease of writing PHPUnit tests
Closed, ResolvedPublic

Description

There are 2 issues in the MwApiContentProvider class in /content-providers/ which 1 will affect testing the class and the other is just an improvement.

  1. Wrapping the call to file_get_contents() so it can be stubbed and using MediaWikiServices HttpRequestFactory class to make an HTTP request. The benefit of this is; the class will be easily testable and data can easily be injected (DI) during tests. Also, will avoid the use of suppressing warnings and all that messy stuff :)
  1. The use of MediaWiki's JSON Formatter class, FormatJson::decode() instead of json_decode() which is generally advised to be used; https://doc.wikimedia.org/mediawiki-core/master/php/classFormatJson.html.

This minor refactoring will improve on the over all functionality and efficiency of the code and provides an easy way of testing this class.

Event Timeline

D3r1ck01 created this task.Dec 20 2018, 9:58 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 20 2018, 9:58 AM

Change 478488 had a related patch set uploaded (by D3r1ck01; owner: Alangi Derick):
[mediawiki/extensions/MobileFrontend@master] MediaWikiServices & Service Wirings in MwApiContentProvider.php

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

D3r1ck01 removed D3r1ck01 as the assignee of this task.Dec 20 2018, 5:05 PM

Freeing for whosoever will sign this off :)

Change 478488 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] MediaWikiServices & Service Wirings in MwApiContentProvider.php

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

phuedx closed this task as Resolved.EditedDec 21 2018, 3:41 PM
phuedx claimed this task.
phuedx added a subscriber: phuedx.

Points 1 and 2 were addressed in https://gerrit.wikimedia.org/r/478488. It'd be good to see a follow-on change to add tests for the class.

Being bold.

Points 1 and 2 were addressed in https://gerrit.wikimedia.org/r/478488. It'd be good to see a follow-on change to add tests for the class.

Sure, it's on it's way soon! And T212666 tracks it :)