D17302: add simple test for string merging

[Alexander Stippich]

astippich updated this revision to Diff 48688.
astippich added a comment.


  - extend the test

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17302?vs=47329&id=48688

BRANCH
  test_result

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

AFFECTED FILES
  autotests/unit/file/CMakeLists.txt
  autotests/unit/file/resulttest.cpp
  src/file/extractor/result.h

To: astippich, #baloo, bruns
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams

D17302: add simple test for string merging

[Alexander Stippich]

astippich added a comment.


  I was recently wondering if this is actually desired behavior. Right now it 
is definitely required since KFileMetaData wrongly outputs QStrings instead of 
a QStringList, but when this is fixed, this behavior should be removed imho. 
Otherwise, querying metadata via KFileMetaData and via Baloo differs in output, 
which it shouldn't.

REPOSITORY
  R293 Baloo

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

To: astippich, #baloo, bruns
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams

D17302: add simple test for string merging

[Alexander Stippich]

astippich updated this revision to Diff 47329.
astippich added a comment.


  - updates for master

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17302?vs=46710&id=47329

BRANCH
  test_result

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

AFFECTED FILES
  autotests/unit/file/CMakeLists.txt
  autotests/unit/file/resulttest.cpp
  src/file/extractor/result.h

To: astippich, #baloo, bruns
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams

D17302: add simple test for string merging

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> astippich wrote in resulttest.cpp:40
> Actually it is. The TermGenerators are otherwise only initialized with 
> nullptr, and consequently will crash. That is something that should be fixed 
> (separately).

D17315 

REPOSITORY
  R293 Baloo

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

To: astippich, #baloo, bruns
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams

D17302: add simple test for string merging

[Alexander Stippich]

astippich updated this revision to Diff 46710.
astippich marked 2 inline comments as done.
astippich added a comment.


  - small fixes

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17302?vs=46686&id=46710

BRANCH
  test_result

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

AFFECTED FILES
  autotests/unit/file/CMakeLists.txt
  autotests/unit/file/resulttest.cpp

To: astippich, #baloo, bruns
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams

D17302: add simple test for string merging

[Alexander Stippich]

astippich added inline comments.

INLINE COMMENTS

> bruns wrote in resulttest.cpp:40
> No need to reset the default constructed `Baloo::Document` with a default 
> constructed one.

Actually it is. The TermGenerators are otherwise only initialized with nullptr, 
and consequently will crash. That is something that should be fixed 
(separately).

REPOSITORY
  R293 Baloo

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

To: astippich, #baloo, bruns
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams

D17302: add simple test for string merging

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> resulttest.cpp:2
> +/*
> + * 
> + * Copyright (C) 2018 Alexander Stippich 

^_^

> resulttest.cpp:39
> +
> +Result 
> result(QStringLiteral("test"),QStringLiteral("audo/mpeg3"),KFileMetaData::ExtractionResult::ExtractEverything);
> +result.setDocument(doc);

Spaces and linebreaks ...

> resulttest.cpp:40
> +Result 
> result(QStringLiteral("test"),QStringLiteral("audo/mpeg3"),KFileMetaData::ExtractionResult::ExtractEverything);
> +result.setDocument(doc);
> +

No need to reset the default constructed `Baloo::Document` with a default 
constructed one.

REPOSITORY
  R293 Baloo

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

To: astippich, #baloo, bruns
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams

D17302: add simple test for string merging

[Alexander Stippich]

astippich added a comment.


  Didn't found what I wanted to find, but since the code is there we can add it.

REPOSITORY
  R293 Baloo

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

To: astippich, #baloo, bruns
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams

D17302: add simple test for string merging

[Alexander Stippich]

astippich created this revision.
astippich added reviewers: Baloo, bruns.
Herald added projects: Frameworks, Baloo.
Herald added a subscriber: kde-frameworks-devel.
astippich requested review of this revision.

REVISION SUMMARY
  adds a simple test for the result class
  that tests how multiple entries from the extractors
  with the same property are merged into a list.

TEST PLAN
  test passes

REPOSITORY
  R293 Baloo

BRANCH
  test_result

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

AFFECTED FILES
  autotests/unit/file/CMakeLists.txt
  autotests/unit/file/resulttest.cpp

To: astippich, #baloo, bruns
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams