D21996: [autotests] Test placement strategies

[Vlad Zagorodniy]

zzag added inline comments.

INLINE COMMENTS

> placement.cpp:29-36
> +#include <KWayland/Client/compositor.h>
> +#include <KWayland/Client/plasmashell.h>
> +#include <KWayland/Client/server_decoration.h>
> +#include <KWayland/Client/shell.h>
> +#include <KWayland/Client/shm_pool.h>
> +#include <KWayland/Client/surface.h>
> +#include <KWayland/Client/xdgdecoration.h>

You forgot to sort these includes.

> placement.cpp:151
> +    for (int i = 0; i < 4; i++) {
> +        auto window = createAndPlaceWindow(QSize(600, 500), testParent.data());
> +        // smart placement shouldn't define a size on clients

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

> davidedmundson wrote in placement.cpp:61-66
> 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

> 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.

> davidedmundson wrote in placement.cpp:121
> 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.

Oops, sorry.

REPOSITORY
  R108 KWin

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

To: davidedmundson, #kwin
Cc: zzag, kwin, LeGast00n, sbergeron, jraleigh, fbampaloukas, GB_2, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kwin/attachments/20190625/dd2bbf20/attachment.html>