D10119: baloo-widgets: Create test to assert metaDataRequestFinished is emitted once only

[Elvis Angelaccio]

elvisangelaccio added inline comments.

INLINE COMMENTS

> michaelh wrote in CMakeLists.txt:17
> This certainly needs some cleanup. I just added more and more libs until it compiled. If there is a better strategy than trial-and-error. Let me know please.

This is fine, but please move `Qt5::Widgets` to its own line

> michaelh wrote in filemetadatawidgettest.cpp:69
> Not sure if this really needed. The original code already fails at line 64.

No this is not needed. `QEXPECT_FAIL` should only be used when we know some test fails and we don't know how to fix it.

> filemetadatawidgettest.cpp:26
> +#include <kfileitem.h>
> +#include <QtTest>
> +#include <QMetaType>

Please use `<QTest>` instead, it includes less stuff.

> filemetadatawidgettest.cpp:85-86
> +    // To be safe: wait for a 2nd signal
> +    QEXPECT_FAIL("", "Wait for consecutive signals timed out", Continue);
> +    QVERIFY(spy.wait(1000));
> +    

Same here

> filemetadatawidgettest.h:28
> +
> +#include <kfileitem.h>
> +#include <filemetadatawidget.h>

`#include <KFileItem>`

> filemetadatawidgettest.h:31-32
> +
> +//#include <KFileItem>
> +// Q_DECLARE_METATYPE(KFileItemList)
> +    

Please remove all the commented code

> filemetadatawidgettest.h:34
> +    
> +class filemetadatawidgettest : public QObject
> +{

This should be camel case

> filemetadatawidgettest.h:44
> +
> +    void ShouldSignalOnceFile();
> +    void ShouldSignalOnceFiles();

method names usually start with a lowercase letter

REPOSITORY
  R824 Baloo Widgets

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

To: michaelh, elvisangelaccio, smithjd, vhanda, ngraham, #dolphin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20180128/592220c6/attachment.htm>