autotests: Test for multiple values
Needs RevisionPublic
Actions

Authored by michaelh on Apr 14 2018, 4:01 PM.

Details

Reviewers

mgallien
bruns

Group Reviewers

Baloo
Frameworks

Summary

Prepare sample files for tags which may have multiple values

epub: 'dc:subject'
office docs: keyword
audio; artist + genre

Create partially failing tests for subject, keywords, artist

Test Plan

make test

Diff Detail

Repository

R286 KFileMetaData

Branch

multi-value-test (branched from master)

Lint

No Linters Available

Unit

No Unit Test Coverage

michaelh created this revision.Apr 14 2018, 4:01 PM

Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptApr 14 2018, 4:01 PM

michaelh requested review of this revision.Apr 14 2018, 4:01 PM

michaelh added inline comments.Apr 14 2018, 4:07 PM

autotests/multivaluetest.cpp
87	The following behaviours need to be discussed. They are just suggestions.

michaelh mentioned this in D10694: epubextractor: Handle multiple subjects better.Apr 14 2018, 4:15 PM

michaelh added a dependent revision: D10694: epubextractor: Handle multiple subjects better.

michaelh mentioned this in D12017: taglibextractortest: Test for multiple values.Apr 14 2018, 4:18 PM

"Prepare sample files for tags which may have multiple values"

michaelh edited the summary of this revision. (Show Details)Apr 15 2018, 12:15 PM

michaelh mentioned this in D11820: Handle properties with multiple values.Apr 15 2018, 12:22 PM

Thanks

This revision is now accepted and ready to land.Apr 18 2018, 7:51 PM

The test for mp3 is failing to complete. I am investigating.

This revision now requires changes to proceed.Apr 18 2018, 8:00 PM

Sorry, that was a local problem.

This revision is now accepted and ready to land.Apr 18 2018, 8:05 PM

bruns added inline comments.Apr 19 2018, 1:05 AM

autotests/multivaluetest.cpp
91	Here you use the property value directly, while in other places you use the `property` variable - I think using the value directly is much easier to read.
92	Again, I prefer this notation versus `expectedresult` You can make this somewhat shorter by using `QVariantList{QStringLiteral{"Baloo KFileMetaData"}}`, i.e. omit the extra `QVariant`

Rebase
Apply some suggested changes

michaelh requested review of this revision.Apr 19 2018, 12:46 PM

michaelh added a reviewer: bruns.

michaelh marked an inline comment as done.

michaelh added inline comments.

autotests/multivaluetest.cpp
92	I want defend `expectedresult` a little. The background: `testXXX_data` functions tend to be very long and it is easy (for me) to get confused about what is tested against which value. `expectedresult` tries to make clear that it is always the same value. If you can't/don't agree with that. I'll follow your lead :-)

mgallien accepted this revision.May 5 2018, 9:24 AM

This revision is now accepted and ready to land.May 5 2018, 9:24 AM

@michaelh Ping! Can we land this?

Restricted Application added a subscriber: kde-frameworks-devel. · View Herald TranscriptMay 9 2018, 6:50 PM

This depends on the pending discussion how "Subject" and "Keyword" should be handled, see D10694

This revision now requires changes to proceed.May 9 2018, 9:52 PM

Revision Contents
Changeset List

			Path
M			autotests/CMakeLists.txt (4 lines)
A	M		autotests/multivaluetest.h (36 lines)
A	M		autotests/multivaluetest.cpp (229 lines)
A	M	(bin)	autotests/samplefiles/multi/test.epub
A	M	(bin)	autotests/samplefiles/multi/test.flac
A	M	(bin)	autotests/samplefiles/multi/test.m4a
A	M	(bin)	autotests/samplefiles/multi/test.mp3
A	M	(bin)	autotests/samplefiles/multi/test.mpc
A	M	(bin)	autotests/samplefiles/multi/test.odp
A	M	(bin)	autotests/samplefiles/multi/test.odt
A	M	(bin)	autotests/samplefiles/multi/test.ogg
A	M	(bin)	autotests/samplefiles/multi/test.opus
A	M	(bin)	autotests/samplefiles/multi/test.pdf
A	M	(bin)	autotests/samplefiles/multi/test_libreoffice.docx

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	32122	8b8857c		Apr 14 2018, 3:49 PM	★	★
Diff 2	32559	c30204f	- Rebase	Apr 19 2018, 12:41 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
a8f29acb7600	8735c65c519a	2cde24f9d84f	Michael Heidelbach	Apply some suggested changes	Apr 19 2018, 12:25 PM
2cde24f9d84f	7ae60bd701f8	8c79e09f8dba	Michael Heidelbach	Verify extractors	Apr 14 2018, 3:52 PM
8c79e09f8dba	9c05583d5555	5fef1f1a2913	Michael Heidelbach	Tweak test.mp3	Apr 14 2018, 2:59 PM
5fef1f1a2913	ba246062f002	1e44f6d9ca3b	Michael Heidelbach	Add audio sample files	Apr 14 2018, 2:07 PM
1e44f6d9ca3b	0db4347b003a	a77dab23116d	Michael Heidelbach	Repair docx	Apr 14 2018, 2:01 PM
a77dab23116d	f3d79e32e228	7ac25059eb0d	Michael Heidelbach	Revert epubextractortest.cpp	Apr 14 2018, 1:17 PM
7ac25059eb0d	7b8022758c4e	0dc9e27efe29	Michael Heidelbach	Revert test.epub	Apr 14 2018, 1:15 PM
0dc9e27efe29	05d1fd7c9fe7	1e36c73c0abb	Michael Heidelbach	Revert test.odt	Apr 14 2018, 1:14 PM
1e36c73c0abb	d5575123a63e	fc5ca50bed68	Michael Heidelbach	Expect epub to fail	Apr 14 2018, 12:55 PM
fc5ca50bed68	8deba902542c	3d3631defaa8	Michael Heidelbach	Add test.pdf	Apr 14 2018, 12:43 PM
3d3631defaa8	6b02099812d5	c30204f7b9c0	Michael Heidelbach	autotests: Add test for multi-value properties (Show More…)	Feb 20 2018, 1:06 PM

Status	Author	Revision
Abandoned	astippich	D11820 Handle properties with multiple values
Accepted	bruns	D10694 epubextractor: Handle multiple subjects better
Needs Revision	michaelh	D12197 autotests: Test for multiple values