D10149: baloo-widgets: Add autotest for asychronously extracted data

Elvis Angelaccio noreply at phabricator.kde.org
Sun Feb 25 15:46:58 GMT 2018


elvisangelaccio added inline comments.

INLINE COMMENTS

> filemetadataitemcounttest.cpp:77-87
> +    const auto propCheck = [this, expectedItems, widgetsPerItem](){
> +        QList<QWidget*> items = m_widget->findChildren<QWidget*>(QString(), Qt::FindDirectChildrenOnly);
> +        QCOMPARE(items.count(), expectedItems * widgetsPerItem);
> +    };
> +    
> +    connect(m_widget, &Baloo::FileMetaDataWidget::metaDataRequestFinished, propCheck);
> +    QSignalSpy spy(m_widget, &Baloo::FileMetaDataWidget::metaDataRequestFinished);

Actually I don't understand why there is a lambda here.

Imho we should test how many signals we emit (should be only 1, right?), and then check the number of items:

  QVERIFY(spy.wait());
  QCOMPARE(spy.count(), 1);
  QList<QWidget*> items = m_widget->findChildren<QWidget*>(QString(), Qt::FindDirectChildrenOnly);
  QCOMPARE(items.count(), expectedItems * widgetsPerItem);

> filemetadatawidgettest.cpp:70
> +    const QStringList args = {QStringLiteral("--name=user.baloo.rating")
> +            , QStringLiteral("--value=5") 
> +            , QFINDTESTDATA("testtagged.mp3")

Why commas at the beginning of lines? This is usually done only in constructor initializer lists

> filemetadatawidgettest.cpp:167
> +    );
> +    QVERIFY(spy.wait());
> +}

Same here. How many signals do we expect? Do we really need the lambda?

REPOSITORY
  R824 Baloo Widgets

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

To: michaelh, #dolphin, elvisangelaccio
Cc: ngraham, ashaposhnikov, spoorun, nicolasfella, alexeymin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20180225/5c52c8d0/attachment.htm>


More information about the kfm-devel mailing list