Page MenuHomePhabricator

Reach a stable test coverage for the FileImporter PHP code
Closed, ResolvedPublic

Description

The current coverage for the FileImporter extensions PHP code is about 40%. This is better than many other codebases, but not ideal. A good coverage of at least 60%, better 80% increases confidence and allows the team to apply changes to a codebase much faster, with much less fear.

Acceptance criteria for this task is a coverage of at least 80%, with a sensible mixture of unit and integration tests (this means a few integration tests that cover everything are not sufficient).

Currently highest ranked "project risks" according to https://doc.wikimedia.org/cover-extensions/FileImporter/dashboard.html, most critical first:

  • T190821 ApiDetailRetriever is already tested with a line coverage of about 67%. The code is quite complex (long and deeply nested), that's why this is ranked higher than anything else.
  • T190825 FileChunkSaver is technically part of HttpRequestExecutor::executeAndSave() and could be tested via HttpRequestExecutorTest, but current isn't.
  • T190826 Importer is technically part of SpecialImportFile::doImport() and could be tested via SpecialImportFileIntegrationTest, but currently isn't.
  • T190828 SpecialImportFile::doImport() is uncovered. See above.
  • T190829 WikiTextEditor appears trivial, but interacts with the super-complex EditPage. Some kind of integration test should make sure this interaction can't break.

"Low hanging fruits" that don't need any integration testing, but a trivial unit test only. In order of significance, more critical first:

  • HttpApiLookup should definitely have it's separate unit test.
  • ImportDetails is quite small, but does have many methods. A separate unit test should be very easy to create.
  • ImportOperations
  • TextRevisions
  • FileRevision
  • FileRevisionFromRemoteUrl
  • TextRevision
  • DuplicateFileRevisionChecker
  • ImportPlan
  • See https://doc.wikimedia.org/cover-extensions/FileImporter/dashboard.html for more.

Related Objects

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 422173 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/FileImporter@master] Add WikiTextEditor to ChangeFileInfoFormTest coverage

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

Change 422173 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Add WikiTextEditor to ChangeFileInfoFormTest coverage

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

Change 422392 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/FileImporter@master] Add unit tests for FileImporterHooks and two small Services

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

Change 422392 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Add unit tests for FileImporterHooks and two small Services

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

Change 425834 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/FileImporter@master] Add tests for all non-empty Exception classes

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

Change 425834 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Add tests for all non-empty Exception classes

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

Change 432063 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/FileImporter@master] Add test for FileImporter\Data\ImportDetails

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

Change 432067 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/FileImporter@master] Add test for FileImporter\Interfaces\ImportOperation

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

Change 432067 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Add test for FileImporter\Interfaces\ImportOperation

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

Change 432063 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Add test for FileImporter\Data\ImportDetails

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

Change 435785 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/FileImporter@master] Add test for DuplicateFileRevisionChecker

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

thiemowmde triaged this task as Medium priority.May 28 2018, 1:48 PM
thiemowmde updated the task description. (Show Details)

Change 435785 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Add test for DuplicateFileRevisionChecker

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

Change 439633 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/FileImporter@master] Add first test case for HttpApiLookup

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

Change 439633 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Add first test case for HttpApiLookup

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

Change 439840 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/FileImporter@master] Add happy-path test case to HttpApiLookupTest

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

Change 439840 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Add happy-path test case to HttpApiLookupTest

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

thiemowmde claimed this task.
thiemowmde removed a project: Patch-For-Review.

As of today we are at ~89%.