D21996: [autotests] Test placement strategies

zzag added inline comments.

INLINE COMMENTS

> placement.cpp:23-24
> +#include "platform.h"
> +#include "shell_client.h"
> +#include "screens.h"
> +#include "wayland_server.h"

Style: https://techbase.kde.org/Policies/Frameworks_Coding_Style#Includes (needs to be sorted)

> placement.cpp:42
> +
> +struct PlaceWindowResult {
> +    QSize initiallyConfiguredSize;

Style: https://techbase.kde.org/Policies/Frameworks_Coding_Style#Braces

> placement.cpp:61-66
> +    void setPlacementPolicy(Placement::Policy policy);
> +    /**
> +     * Create a window with the lifespan of parent and return relevant results for testing
> +     * defaultSize is the size to use if the compositor returns an empty size in the first configure
> +     */
> +    PlaceWindowResult createAndPlaceWindow(const QSize &defaultSize, QObject *parent);

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

> placement.cpp:105-108
> +    auto group = kwinApp()->config()->group("Windows");
> +    group.writeEntry("Placement", Placement::policyToString(policy));
> +    group.sync();
> +    Workspace::self()->slotReconfigure();

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

> placement.cpp:121
> +    surface->commit(Surface::CommitFlag::None);
> +    configSpy.wait();
> +

Missing QVERIFY.

> placement.cpp:143
> +
> +    QScopedPointer<QObject> testParent; //dumb QObject just for scoping surfaces to the test
> +

Is it correct? It's null.

> placement.cpp:161-163
> +void TestPlacement::testPlaceCorner()
> +{
> +    setPlacementPolicy(Placement::ZeroCornered);

A better name for the test: testPlaceZeroCornered.

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