Differential D12101

Adapt PlaceTreeModelTest autotest to KIO change
ClosedPublic
Actions

Authored by rkflx on Apr 10 2018, 10:18 PM.

Details

Reviewers
ngraham
Group Reviewers
Gwenview
Commits
R260:aeeb5d461fe4: Adapt PlaceTreeModelTest autotest to KIO change
Summary

After e8d799c9b522 needed to account for additional entries in
KFilePlacesModel, in KIO's 7e1d2fb84546 the number of default items
returned by the model was changed again, in particular "This Month" and
"Last Month" were removed.

As those entries are not that relevant for Gwenview too, we simply
follow along, meaning apart from changing the number expected in the
test no further changes are necessary.

If in the future more changes are planned in KIO, it might be worth
thinking about removing the test altogether, but for now it serves as a
canary, so any effect on Gwenview can be investigated in a timely
fashion.

Test Plan

./tests/auto/placetreemodeltest testListPlaces does not fail anymore
with KIO 5.45, still works fine with KIO 5.44 (make sure CMake picks up
the change before recompiling).

Diff Detail

Repository
R260 Gwenview
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
rkflx requested review of this revision.Apr 10 2018, 10:18 PM
rkflx created this revision.
rkflx added a comment.Apr 10 2018, 10:19 PM

What's strange though is that only the Baloo URLs are present in the model, and the rest of the entries are created regularly through KIO by populating user-places.xbel. Might be worth checking at some point in the future, so perhaps the number of expected entries can be brought down to 2 again, i.e. just the items we add manually in the test.

huoni added a subscriber: huoni.Apr 11 2018, 12:56 AM

I tested by:

  1. Running the test with KIO < 5.45 - success
  2. Updating KIO, running test on master - failure (expected)
  3. Re-running test with this patch - success

LGTM :)

ngraham accepted this revision.Apr 11 2018, 2:01 AM
ngraham added a subscriber: ngraham.

Yep, fixes the test! Sorry for the fallout; I need to remember to check on Gwenview for every change to KFilePlacesModel.

This revision is now accepted and ready to land.Apr 11 2018, 2:01 AM
rkflx added a comment.Apr 11 2018, 5:56 PM
In D12101#244043, @huoni wrote:

LGTM :)

Thanks for testing ;)

In D12101#244057, @ngraham wrote:

Yep, fixes the test! Sorry for the fallout; I need to remember to check on Gwenview for every change to KFilePlacesModel.

No need to be sorry. It's not really your fault, I suspect the root problem is that Baloo's places are present in the model, while the "regular" places are not. Still good to check Gwenview's places panel when doing changes, though ;)

Closed by commit R260:aeeb5d461fe4: Adapt PlaceTreeModelTest autotest to KIO change (authored by rkflx). · Explain WhyApr 11 2018, 6:21 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mtests/auto/placetreemodeltest.cpp (4 lines)

Diff 31912

View Options

tests/auto/placetreemodeltest.cpp

Loading...