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

Michael Heidelbach noreply at phabricator.kde.org
Sun Feb 18 22:55:20 GMT 2018


michaelh added a comment.


  WTR tests failing on your machine. 
  16 Items, that is the number of synchronously extracted items + labels. Apparently there are no data pulled out of the testfiles.
  Probably more tweaking of baloo config has to be done.
  
  I don't quite understand this, though. On my machine all tests pass regardless of initial baloo config and with file indexing enabled or disabled.
  Could you post `~/.config/baloofilerc` and `~/.config/baloofileinforc` of your test environment somewhere?
  Also, if possible, could you please visually inspect the testfiles with infopanel or tooltips for properties like channels or bitrate?
  
  I also re-checked my test environment running the dolphin binary in the debugger. I can set break points in filemetadatawidget so it is used.

INLINE COMMENTS

> elvisangelaccio wrote in filemetadatawidgettest.cpp:126
> Weird, it should be possible (see for example kio/autotests/favicontest.cpp line 86).
> 
> How did you try to do it?

Nearly like in `kio/autotests/favicontest.cpp`. I tried

- `qputenv("LC_ALL", "C");`  `QCOMPARE(qgetenv("LC_ALL", "C");`
- `qputenv("LC_ALL", QByteArray("C"));`  `QCOMPARE(qgetenv("LC_ALL", "C");`
- `qputenv("LC_ALL", QByteArray();`
- `qputenv("LANG", "C");`  `QCOMPARE(qgetenv("LANG", "C");`

in `initTestCase()` and some other places.

en_US.UTF-8 does not work either. 
???

> elvisangelaccio wrote in filemetadatawidget.cpp:129-139
> In this case because we have the private class where we can put things. In general free functions are fine, but they should be within the anonymous namespace for clarity.

Thanks

> filemetadatawidget.cpp:72
>      QStringList sortedKeys(const QVariantMap& data) const;
> +    QLabel* createLabel(const QString &key,  QString itemLabel, FileMetaDataWidget* parent);
>  

We can't, we have to append a colon to the string.

> elvisangelaccio wrote in filemetadatawidget.cpp:120
> Isn't this already called in `slotDataAvailable()` ?

The answer is no. But it made me think. 
In conclusion: the `dataavailable` signal is not needed at all. Seems to be just a crutch for myself to understand the data flow in `filemetadataprovider`.  With a commented out `connect` statement in `filemetadatawidget` Tests pass(here) and the tooltips look like expected. 
Leaving the code commented for now to (hopefully) prevent the inline comments from jumping around.

> elvisangelaccio wrote in filemetadatawidget.cpp:152
> This information should probably go in the documentation of `filter()` in metadatafilter.h

Marked it with a FIXME.
I intend to do a beautification diff after this. e.g. with qfindtestdata we can get rid of #include "config.h"

REPOSITORY
  R824 Baloo Widgets

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

To: michaelh, #dolphin, elvisangelaccio
Cc: ngraham
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20180218/508d5910/attachment.htm>


More information about the kfm-devel mailing list