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