Page MenuHomePhabricator

Improve code coverage of FileChunkSaver
Closed, ResolvedPublic2 Story Points

Description

FileChunkSaver is technically part of HttpRequestExecutor::executeAndSave() and could be tested via HttpRequestExecutorTest, but current isn't.

Event Timeline

Restricted Application added a project: TCB-Team. · View Herald TranscriptMar 27 2018, 3:20 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 431598 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/FileImporter@master] Add test for FileChunkSaver

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

WMDE-Fisch moved this task from Sprint Backlog to Review on the WMDE-QWERTY-Sprint-2018-05-02 board.
thiemowmde added a subscriber: thiemowmde.

The first patch raised the coverage of the file to about 80%. I would like to watch https://doc.wikimedia.org/cover-extensions/FileImporter/Services/Http/FileChunkSaver.php.html, see what's missing, and possibly fill relevant gaps with a second patch. The goal is not 100%, but relevant code covered. Maybe there is none left. :-)

Change 431598 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Add tests for FileChunkSaver

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

Change 432047 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/FileImporter@master] Add test for getHandle in FileChunkSaver

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

I added another test for the uncovered path in getHandle by creating a file with insufficient permission. The throwExceptionIfOnShortWrite part will be harder to hit though since we have to limit the size of the file system. ... It could work with the mikey179/vfsStream lib though.

I added another test for the uncovered path in getHandle by creating a file with insufficient permission. The throwExceptionIfOnShortWrite part will be harder to hit though since we have to limit the size of the file system. ... It could work with the mikey179/vfsStream lib though.

I did a rough draft in https://gerrit.wikimedia.org/r/#/c/432049/ - this does not work since the lib needs to be present in core I guess. I pinged @Krinkle on IRC in the hope he can help figuring this out.

Change 432047 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Add test for getHandle in FileChunkSaver

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

WMDE-Fisch moved this task from Review to Done on the WMDE-QWERTY-Sprint-2018-05-02 board.
Krinkle removed a subscriber: Krinkle.May 9 2018, 7:25 PM
Lea_WMDE closed this task as Resolved.May 16 2018, 10:29 AM
Lea_WMDE triaged this task as Normal priority.