D15704: increase test coverage of taglibwriter

[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 IMHO it is nicer to have some rule for ordering, instead of arbitrary order.

> taglibwriter.cpp:23
>      QStringList types = {
>          QStringLiteral("audio/mpeg"),
>          QStringLiteral("audio/mpeg3"),

Can you also order these alphabetically based on the main mime type (aliases immediate after)?

> taglibwriter.cpp:33
> +        QStringLiteral("audio/x-opus+ogg"),
> +        QStringLiteral("audio/wav"),
> +        QStringLiteral("audio/x-aiff"),

audio/wav and the next three are not backed by tests AFAICS, can you add these in a separate review, accompanied by tests files?

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D15704

To: astippich, mgallien, bruns
Cc: svuorela, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180927/fff89620/attachment.html>