fail writing test if mime type is not supported by the extractor
ClosedPublic

Authored by astippich on Nov 6 2018, 7:39 PM.

Details

Summary

Do not crash if no suitable extractor is found.
Skip the test instead.

Diff Detail

Repository
R286 KFileMetaData
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
astippich created this revision.Nov 6 2018, 7:39 PM
Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptNov 6 2018, 7:39 PM
Restricted Application added subscribers: Baloo, kde-frameworks-devel. · View Herald Transcript
astippich requested review of this revision.Nov 6 2018, 7:39 PM
bruns added a comment.Nov 6 2018, 7:56 PM

AFAICS the only chance this can happen if the test is using a different library version, e.g. when accidentally using the lib from the system. This would be a test setup error and should be reported with a fail ...

I think that's my fault when I introduced the extractor for test. Apparently it is using the system libraries, and I'm looking for a solution.
I think a skip is still the right thing to do here, since we cannot determine if the tags are correctly written.

bruns added a comment.Dec 5 2018, 2:49 PM

I think that's my fault when I introduced the extractor for test. Apparently it is using the system libraries, and I'm looking for a solution.
I think a skip is still the right thing to do here, since we cannot determine if the tags are correctly written.

I think skip should only be used iff the test does not apply (e.g. running a Windows specific test on Linux) or when some of the required fixtures can not be provided (e.g. running a test requiring network access in an isolated environment).

In this case, it is "easy" to fulfill the requirements, just correct the test setup.

Imagine what may have happened if this were a QSKIP from the beginning - it would have skipped the tests, and you might have committed untested code. I am not quite sure, but I think skips are not even reported when you run the complete test suite via ctest.

astippich updated this revision to Diff 46990.Dec 6 2018, 6:27 PM
  • use QFAIL

Well, if it is not even signaled via ctest, that's pretty bad, I agree

astippich retitled this revision from skip writing test if mime type is not supported by the extractor to fail writing test if mime type is not supported by the extractor.Dec 6 2018, 7:09 PM
bruns accepted this revision.Dec 6 2018, 8:33 PM
This revision is now accepted and ready to land.Dec 6 2018, 8:33 PM
This revision was automatically updated to reflect the committed changes.