Page MenuHomePhabricator

Improve code coverage of FileChunkSaver
Closed, ResolvedPublic2 Estimated 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 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

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

Lea_WMDE triaged this task as Medium priority.