D15704: increase test coverage of taglibwriter

2018-09-28 Thread Alexander Stippich
This revision was automatically updated to reflect the committed changes. Closed by commit R286:ae3c38293421: increase test coverage of taglibwriter (authored by astippich). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D15704?vs=42519=42521#toc REPOSITORY R286 KFileMetaData CHANGES

D15704: increase test coverage of taglibwriter

2018-09-28 Thread Stefan Brüns
bruns accepted this revision. This revision is now accepted and ready to land. REPOSITORY R286 KFileMetaData BRANCH taglib_write_mimetypes REVISION DETAIL https://phabricator.kde.org/D15704 To: astippich, mgallien, bruns Cc: svuorela, kde-frameworks-devel, #baloo, ashaposhnikov,

D15704: increase test coverage of taglibwriter

2018-09-28 Thread Alexander Stippich
astippich updated this revision to Diff 42519. astippich added a comment. - remove untested mime types and sort REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15704?vs=42196=42519 BRANCH taglib_write_mimetypes REVISION DETAIL

D15704: increase test coverage of taglibwriter

2018-09-27 Thread Stefan Brüns
bruns requested changes to this revision. bruns added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > taglibwritertest.cpp:74 > + > +QTest::addRow("mp3") > +<< QStringLiteral("mp3") Can you sort the tests alphabetically? It does not matter much, but

D15704: increase test coverage of taglibwriter

2018-09-24 Thread Alexander Stippich
astippich added inline comments. INLINE COMMENTS > svuorela wrote in taglibwriter.cpp:22 > Unrelated. but consider making this static ? Never really though about that, but should be static, yes. None of the writers and extractors I looked at currently do this, so if I find time I will do that

D15704: increase test coverage of taglibwriter

2018-09-23 Thread Sune Vuorela
svuorela added a comment. Looks good to me. A nice and simple way to drastically increase test coverage. INLINE COMMENTS > taglibwriter.cpp:22 > { > QStringList types = { > QStringLiteral("audio/mpeg"), Unrelated. but consider making this static ? REPOSITORY R286

D15704: increase test coverage of taglibwriter

2018-09-23 Thread Alexander Stippich
astippich added a dependent revision: D15714: add a string suffix to test data and use for unicode testing of taglibwriter. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D15704 To: astippich, mgallien, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov,

D15704: increase test coverage of taglibwriter

2018-09-23 Thread Alexander Stippich
astippich updated this revision to Diff 42196. astippich added a comment. - don't change test strings for now REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15704?vs=42174=42196 BRANCH taglib_write_mimetypes REVISION DETAIL

D15704: increase test coverage of taglibwriter

2018-09-23 Thread Stefan Brüns
bruns added a comment. In general this looks good, but I would like two changes: 1. Do the conversion to QTest first, and leave out the change for unicode testing (e.g. `Title1` -> `Title €`) 2. Add a third column like "stringsuffix", and then add another test (row) for each format.

D15704: increase test coverage of taglibwriter

2018-09-23 Thread Alexander Stippich
astippich added a dependency: D15220: implement more basic tags for taglibwriter. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D15704 To: astippich, mgallien, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns,

D15704: increase test coverage of taglibwriter

2018-09-23 Thread Alexander Stippich
astippich created this revision. astippich added reviewers: mgallien, bruns. Herald added projects: Frameworks, Baloo. Herald added subscribers: Baloo, kde-frameworks-devel. astippich requested review of this revision. REVISION SUMMARY increase the test coverage by testing more mimetypes, and