[autotests] Test placement strategies
ClosedPublic

Authored by davidedmundson on Jun 22 2019, 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
Branch
master
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 13287
Build 13305: arc lint + arc unit
davidedmundson created this revision.Jun 22 2019, 1:05 PM
Restricted Application added a project: KWin. · View Herald TranscriptJun 22 2019, 1:05 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
davidedmundson requested review of this revision.Jun 22 2019, 1:05 PM
davidedmundson retitled this revision from Test placement strategies to [autotests] Test placement strategies.Jun 22 2019, 1:12 PM
zzag added a subscriber: zzag.Jun 22 2019, 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.Jun 25 2019, 11:27 AM
autotests/integration/placement.cpp
29–36

You forgot to sort these includes.

29–36

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.

151

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

davidedmundson added inline comments.Jul 2 2019, 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.

151

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

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

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.Jul 2 2019, 3:13 PM
This revision was automatically updated to reflect the committed changes.