Differential D11156

add autotest for zips within zips
AbandonedPublic
Actions

Authored by sandsmark on Mar 8 2018, 3:07 PM.

Details

Reviewers

dfaure

Group Reviewers

Frameworks

Summary

an epub in a zip file compressed by e. g. the golang standard zip library, or infozip in streaming mode.

Test Plan

fails before my patch, passes afterwards

Diff Detail

Repository

R243 KArchive

Lint

Lint Skipped

Unit

Unit Tests Skipped

sandsmark created this revision.Mar 8 2018, 3:07 PM

Restricted Application added a project: Frameworks. · View Herald TranscriptMar 8 2018, 3:07 PM

sandsmark requested review of this revision.Mar 8 2018, 3:07 PM

dfaure added inline comments.Mar 8 2018, 9:22 PM

karchivetest.cpp
1156	A check for a file listing would avoid that just "return true" in open() is enough for the test to pass :-)

@sandsmark can you add an expected list of files like other tests do?

Restricted Application added a subscriber: kde-frameworks-devel. · View Herald TranscriptJul 24 2019, 9:40 PM

In D11156#501817, @dfaure wrote:

@sandsmark can you add an expected list of files like other tests do?

Yeah, I just need to find out which machine I wrote this on because it is hard to download patches with binaries from phabricator for fixing. :-P

And I just got a file that I think got broken by the fix this test is for, so I think the zip stuff is overdue for a tiny rewrite (the seeking for two magic bytes and failing everything is a bit brittle).

The command you're looking for is arc patch D11156 (tested, works)

I landed it manually, I hope that's okay: https://cgit.kde.org/karchive.git/commit/?id=17d409236a792fb0db93ab6109fd30b2d51ccc1a

I also noticed that a couple of the other tests failed here, seems like there's some assumptions on the global logging config. I'm not sure if just adding QLoggingCategory::setFilterRules(QStringLiteral("kf5.karchive.warning=true")); at the top of those tests is the best, but I'll open a new PR.