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

[Elvis Angelaccio]

elvisangelaccio requested changes to this revision.
elvisangelaccio added a comment.
This revision now requires changes to proceed.


  In https://phabricator.kde.org/D10119#203350, @michaelh wrote:
  
  > Treated all test equally:  ` QVERIFY(spy.wait(xxx));` -> `spy.wait(xxx)`
  >  `shouldSignalOnceWithoutFile()` and `shouldSignalOnceWithEmptyFile()` still fail, the others pass.
  >
  > Just to see what would happen I changed  `filemetadataprovider.cpp:465` to
  >
  >   KFileItemList FileMetaDataProvider::items() const
  >   {
  >       return m_fileItems.isEmpty() 
  >           ? KFileItemList() << QUrl(QStringLiteral("file:///"))
  >           : m_fileItems;
  >   }
  >
  >
  > Now all tests pass.
  >
  > The crux must be in `build/kde/applications/baloo-widgets/src/KF5BalooWidgets_autogen/include/moc_filemetadatawidget.cpp`:
  >
  >   // SIGNAL 1
  >   void Baloo::FileMetaDataWidget::metaDataRequestFinished(const KFileItemList & _t1)
  >   {
  >       void *_a[] = { nullptr, const_cast<void*>(reinterpret_cast<const void*>(&_t1)) };
  >       QMetaObject::activate(this, &staticMetaObject, 1, _a);
  >   }
  >
  >
  > I can't read this, but hopefully you can tell why there is no signal when `_t1` is an empty list.
  
  
  Sorry but I'm not following you. The `metaDataRequestFinished()` signal is emitted, as the `QCOMPARE(spy.count(), 1)` test proves.
  The first two tests are currently failing for another reason:
  
    FAIL!  : FileMetadataWidgetTest::shouldSignalOnceWithoutFile() Compared values are not the same
       Actual   (m_widget->items().count()): 0
       Expected (1)                        : 1
       Loc: [../autotests/filemetadatawidgettest.cpp(64)]
  
  and:
  
    FAIL!  : FileMetadataWidgetTest::shouldSignalOnceWithEmptyFile() Compared values are not the same
       Actual   (m_widget->items().count()): 0
       Expected (1) 
       Loc: [../autotests/filemetadatawidgettest.cpp(74)]
  
  (see inline comments below).

INLINE COMMENTS

> filemetadatawidgettest.cpp:62
> +    m_widget->setItems(KFileItemList() << QUrl());
> +    spy.wait(500);
> +    QCOMPARE(spy.count(), 1);

I meant that this line should be removed. The test passes even without this nested event loop.

> filemetadatawidgettest.cpp:64
> +    QCOMPARE(spy.count(), 1);
> +    QCOMPARE(m_widget->items().count(), 1);
> +    

Why are you expecting one element in `items()`? You are passing a list with no //valid local// urls to `setItems()`, so we don't append anything to `localItemsList`. Just replace the 1 with a 0 and the test will pass.

> filemetadatawidgettest.cpp:72
> +    m_widget->setItems(KFileItemList());
> +    spy.wait(500);
> +    QCOMPARE(spy.count(), 1);

remove

> filemetadatawidgettest.cpp:74
> +    QCOMPARE(spy.count(), 1);
> +    QCOMPARE(m_widget->items().count(), 1);
> +    

same as above, 0 rather than 1

> filemetadatawidgettest.cpp:84
> +    );
> +    spy.wait();
> +    QCOMPARE(spy.count(), 1);

Here we do need the nested event loop (because `metaDataRequestFinished` is emitted insied a slot).

So please wrap it inside a `QVERIFY()`.

> filemetadatawidgettest.cpp:98
> +    );
> +    spy.wait();
> +    QCOMPARE(spy.count(), 1);

`QVERIFY()` also here

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/20180209/164479b5/attachment.htm>