Fix the test that broke after recent changes to the default Places items
ClosedPublic

Authored by ngraham on Apr 23 2018, 4:59 AM.

Details

Diff Detail

Repository
R318 Dolphin
Branch
fix-placesitemmodeltest (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
ngraham requested review of this revision.Apr 23 2018, 4:59 AM
ngraham created this revision.
markg accepted this revision.Apr 23 2018, 11:07 AM
markg added a subscriber: markg.

Ha, awesome :)

This revision is now accepted and ready to land.Apr 23 2018, 11:07 AM

Thanks! I'll wait for someone from Dolphin to give the patch a thumbs-up too before landing it. Once it's in, I also plan to replace a lot of these hardcoded integers with some more intelligence so that fewer of these tests will pointlessly break when we jigger around the Places items again.

This will only fix the test if you have a recent enough version of KIO. I wonder if we can find a more robust solution (which we'd also need in the 18.04 branch).

That's true. This test is indeed not very robust. I can play around with it a bit.

markg added a comment.EditedMay 22 2018, 5:48 PM

This will only fix the test if you have a recent enough version of KIO. I wonder if we can find a more robust solution (which we'd also need in the 18.04 branch).

I don't think that is much of a problem.
Just increase the version Dolphin depends on.

Quite a bit of work is going into dolphin anyhow so i would be surprised if it even compiles right now with Frameworks 5.43 (that is the current dependency in it's main cmake file).
And if it does the KIO version dependency likely goes up for whatever reason in the near future. No, i don't have a change ready for that but it's just how things work in a active alive project :)

Restricted Application added a project: Dolphin. · View Herald TranscriptMay 22 2018, 5:48 PM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript

@markg Problem is, we can't bump the KIO minimum version in the stable branch.

@renatoo Could you submit a patch that fixes this test for the Applications/18.04 branch (like you did for KIO) ?

markg added a comment.EditedMay 26 2018, 4:32 PM

@markg Problem is, we can't bump the KIO minimum version in the stable branch.

It's only for 2 more patch releases (18.04.2 and 18.04.3). Then we get 18.10.0 i think (there is no schedule yet).
Just leave it as is for the stable branch and fix the master branch.

The stable branch is not build that often (last one right now is 13 days ago).
The master branch is build much more often as most development happens in there and each build for that one gives us a jenkins mail telling us the build is still unstable.

So, i'd say: go for it in master (leave stable as is) and be done with it.
It's only till the tagging of the next stable from master anyhow (3 months or so i guess?).

ngraham updated this revision to Diff 34951.May 27 2018, 2:42 AM

Merge master

Note that this patch only fixes the placesitemmodeltest. kfileitemmodeltest still fails, so we'd still get emails. :(

I'm going to land this and then I'll work on one for the branch too.

ngraham closed this revision.Jun 1 2018, 4:48 AM