[autotests] Test placement strategies
ClosedPublic

Authored by davidedmundson on Sat, Jun 22, 1:05 PM.

Details

Summary

The maximise test is moved and a simple test is added for smart
placement and placeCorner.

The class tries to make a framework to make it faster to add future
xdg_toplevel placement tests without having to copy too much
boilerplate.

Test Plan

Passes

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
davidedmundson created this revision.Sat, Jun 22, 1:05 PM
Restricted Application added a project: KWin. · View Herald TranscriptSat, Jun 22, 1:05 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
davidedmundson requested review of this revision.Sat, Jun 22, 1:05 PM
davidedmundson retitled this revision from Test placement strategies to [autotests] Test placement strategies.Sat, Jun 22, 1:12 PM
zzag added a subscriber: zzag.Sat, Jun 22, 2:53 PM
zzag added inline comments.
autotests/integration/placement.cpp
24–25
43
62–67

These two can be ordinary functions. Also, "doxygen" comments have to terminate with **/

106–109

I personally see nothing wrong with duplicating this piece of code in each test.

122

Missing QVERIFY.

144

Is it correct? It's null.

162–164

A better name for the test: testPlaceZeroCornered.

davidedmundson marked 3 inline comments as done.

update

autotests/integration/placement.cpp
62–67

Why would someone be generating doxygen of an internal class of an autotest?

Yes they could be functions, but there's a semantic link and we're not making a public API or anything

106–109

Till we change something...

122

You can't QVERIFY outside of a test function. It has an empty return.

If this was failing the shellClient test would be exploding, I think most people know to fix that first, which can leave this test to focus on placement.

144

It was not. Thanks.

zzag added inline comments.Tue, Jun 25, 11:27 AM
autotests/integration/placement.cpp
30–37

You forgot to sort these includes.

30–37

Argh, please ignore it. It's sort of embarrassing..

62–67

Why would someone be generating doxygen of an internal class of an autotest?

Perhaps I should have asked you this question. Initially, this method had a legit Doxygen comment with "wrong" terminator.

122

Oops, sorry.

152

Name of this variable is misleading. It leaves impression that this is a ShellClient. Perhaps don't use auto?

davidedmundson added inline comments.Tue, Jul 2, 1:44 PM
autotests/integration/placement.cpp
62–67

Yeah, I had it because I wanted to indicate the comment was describing what the function does, not commenting the code.

But then as that went into a pointless discussion about doxygen, I didn't bother.

152

Done, but we need to take this "auto" discussion to frameworks-devel and make an actual policy.

zzag accepted this revision.Tue, Jul 2, 3:13 PM
zzag added inline comments.
autotests/integration/placement.cpp
152

A bit radical claim: I suggest to prohibit auto in kwin because many people abuse it. We could use it with only a few things, like iterators.

auto foo = bar();

is just wrong. I know that I did this too, but it's just wrong.

Anyways, that's out of scope of this patch.

This revision is now accepted and ready to land.Tue, Jul 2, 3:13 PM
This revision was automatically updated to reflect the committed changes.