Merge lp:~compiz-team/compiz/compiz.fix_1167983 into lp:compiz/0.9.10

[Sam Spilsbury]

Rewrite screen size change tests.

1. Rename the overly-terse variables names to variables names that have some
   significance
2. Split the giant three-test structure into multiple tests each with only one
   assert
3. Remove a lot of redundant calculation
4. Remove magic numbers peppered throughout the code. Use constants and
   express asserts as relationships between those constants.
5. Refactor some of the more common test-advancement code (such as changing
   the screen size) into the test fixture as common methods.

(LP: #1167983)

Rewrite screen size change tests.

1. Rename the overly-terse variables names to variables names that have some
   significance
2. Split the giant three-test structure into multiple tests each with only one
   assert
3. Remove a lot of redundant calculation
4. Remove magic numbers peppered throughout the code. Use constants and
   express asserts as relationships between those constants.
5. Refactor some of the more common test-advancement code (such as changing
   the screen size) into the test fixture as common methods.

(LP: #1167983)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3654
http://jenkins.qa.ubuntu.com/job/compiz-ci/126/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/163/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-pbuilder/./build=pbuilder,distribution=raring,flavor=amd64/515

Click here to trigger a rebuild:
http://s-jenkins:8080/job/compiz-ci/126/rebuild

review: Needs Fixing (continuous-integration)

[Sam Spilsbury (smspillaz)]

^ We're still hitting the valgrind + xorg-gtest race condition sadly :(

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3654
http://jenkins.qa.ubuntu.com/job/compiz-ci/127/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/164/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-pbuilder/./build=pbuilder,distribution=raring,flavor=amd64/516

Click here to trigger a rebuild:
http://s-jenkins:8080/job/compiz-ci/127/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3654
http://jenkins.qa.ubuntu.com/job/compiz-ci/128/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/165
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-pbuilder/./build=pbuilder,distribution=raring,flavor=amd64/517

Click here to trigger a rebuild:
http://s-jenkins:8080/job/compiz-ci/128/rebuild

review: Approve (continuous-integration)

[Steve Langasek (vorlon)]

Generally looks good to me. The refactoring of the workArea/struts handling was something I had already pondered in my previous attempt, but I left it out in the interest of being conservative with my changes (I thought perhaps we would have tests in the future that might require a different workArea).

I see some differences in the tests that I'd like to call out:

- previously, the first test tested that the window geometry remained the same when both width and height were reduced. The new test only tests reducing the width (1200->1024), not the height (800->768).
- the second test previously tested that the window geometry remained the same across a big->little->big screen size transition. The new tests are testing big->little and little->big separately, which means we're not testing for the possibility of wrong saved geometry handling (in the case where there should not be any saved geometry). I think this is important to test.
- WindowOnPreviousViewportSecondMonitorStaysOnPreviousViewport: this has dropped a check that the window geometry remains the same after unplugging the monitor. It's important to test this explicitly, not just an unplug/plug round-trip, as the previous code did not get this case right.

> SetWindowGeometry (cw::Geometry (-(DUAL_MONITOR_WIDTH -
> WINDOW_X),

Would probably be clearer as 'WINDOW_X - DUAL_MONITOR_WIDTH' instead

> MakeScreenSmallerWindowOnSecondViewportOnFirstMonitorSecondViewport

seems to me that this test could do with a shorter name. 'MakeScreenSmallerWindowOnFirstMonitorSecondViewport'?

In total, there seems to be a net reduction of 4 EXPECT_EQ() occurrences in the new code vs. the old. One of these is the dropped check mentioned above for WindowOnPreviousViewportSecondMonitorStaysOnPreviousViewport, a second is in applyGeometry(); I haven't looked to see where the other two have gone and whether they represent a regression in our test coverage.

review: Needs Fixing

[Sam Spilsbury (smspillaz)]

Hey Steve, thanks for the review.

> I see some differences in the tests that I'd like to call out:
>
> - previously, the first test tested that the window geometry remained the same
> when both width and height were reduced. The new test only tests reducing the
> width (1200->1024), not the height (800->768).

Thanks for catching that. I've created a separate test which checks that a reduction in screen height would likewise have no effect.

> - the second test previously tested that the window geometry remained the same
> across a big->little->big screen size transition. The new tests are testing
> big->little and little->big separately, which means we're not testing for the
> possibility of wrong saved geometry handling (in the case where there should
> not be any saved geometry). I think this is important to test.

So, at the moment what I've been doing is trying to get the number of asserts-per-test down to one, and splitting each case where we had a new assert into a different testcase. So there might be some instances where I've taken a test that does multiple things and split it into two and had some code duplicated. For example:

TEST_F (PlaceScreenSizeChange, MakeScreenLargerNoMovement)
{
    SetInitialScreenSize (CompSize (MONITOR_WIDTH,
        MONITOR_HEIGHT));
    SetWindowGeometry (cw::Geometry (WINDOW_X,
         WINDOW_Y,
         WINDOW_WIDTH,
         WINDOW_HEIGHT,
         0));

    /* Making the screen size bigger with no
     * saved geometry should cause the window not to move */
    cw::Geometry expectedWindowGeometryAfterChange =
 GetInitialWindowGeometry ();

    cw::Geometry windowGeometryAfterChange =
 ChangeScreenSizeAndAdjustWindow (CompSize (DUAL_MONITOR_WIDTH,
         DUAL_MONITOR_HEIGHT));

    EXPECT_EQ (expectedWindowGeometryAfterChange, windowGeometryAfterChange);
}

Test little -> big (no change, first assert in the second "test").

TEST_F (PlaceScreenSizeChange, MakeScreenSmallerWindowPlacedAtEdgeToFit)
{
    SetInitialScreenSize (CompSize (DUAL_MONITOR_WIDTH,
        DUAL_MONITOR_HEIGHT));
    /* Move the window to the other "monitor" */
    SetWindowGeometry (cw::Geometry (MONITOR_WIDTH + WINDOW_X,
         WINDOW_Y,
         WINDOW_WIDTH,
         WINDOW_HEIGHT,
         0));

    /* Unplug a "monitor" */
    cw::Geometry windowGeometryAfterChange =
 ChangeScreenSizeAndAdjustWindow (CompSize (MONITOR_WIDTH,
         MONITOR_HEIGHT));

    /* The window should be exactly on-screen at the edge */
    cw::Geometry expectedWindowGeometryAfterChange (MONITOR_WIDTH - WINDOW_WIDTH,
          WINDOW_Y,
          WINDOW_WIDTH,
          WINDOW_HEIGHT,
          0);
    EXPECT_EQ (expectedWindowGeometryAfterChange, windowGeometryAfterChange);
}

Test little -> big -> little (expect that the window remains on the edge of the viewport).

> - WindowOnPreviousViewportSecondMonitorStaysOnPreviousViewport: this has
> dropped a check that the window geometry remains the same after unplugging the
> monitor. It's important to test this explicitly, not just an unplug/plug
> round-trip, as the previous code did not get this case right.

Thanks for catching that. It was meant to conform to the specifics that I mentio...

[lp:~compiz-team/compiz/compiz.fix_1167983]

3655. By Sam Spilsbury on 2013-04-16

Pull out a few additional tests.

WindowOnPreviousViewportFirstMonitorStaysOnPreviousViewport v WindowOnPreviousViewportFirstMonitorStaysOnPreviousViewportWhenReplugged

and

WindowOnPreviousViewportSecondMonitorStaysOnPreviousViewport v
WindowOnPreviousViewportSecondMonitorStaysOnPreviousViewportWhenReplugged

and

UnplugMonitorWindowOnSecondViewportFirstMonitorStaysOnFirstMonitor v
ReplugMonitorWindowOnSecondViewportFirstMonitorStaysOnFirstMonitor

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3655
http://jenkins.qa.ubuntu.com/job/compiz-ci/131/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/168
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-pbuilder/./build=pbuilder,distribution=raring,flavor=amd64/520

Click here to trigger a rebuild:
http://s-jenkins:8080/job/compiz-ci/131/rebuild

review: Approve (continuous-integration)

[lp:~compiz-team/compiz/compiz.fix_1167983]

3656. By Sam Spilsbury on 2013-04-16

More concise names for the tests

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3656
http://jenkins.qa.ubuntu.com/job/compiz-ci/134/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/171
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-pbuilder/./build=pbuilder,distribution=raring,flavor=amd64/523

Click here to trigger a rebuild:
http://s-jenkins:8080/job/compiz-ci/134/rebuild

review: Approve (continuous-integration)

[MC Return (mc-return)]

+1 for the name changes.
Unfortunately I am not able to test this, because I get hit by bug #1171364.

[Sam Spilsbury (smspillaz)]

Steve ?

[MC Return (mc-return)]

If you would rebase this MP on trunk, I could *maybe/probably* test this...

[Steve Langasek (vorlon)]

Looks like it addresses my concerns, thanks!

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'plugins/place/src/screen-size-change/tests/screen-size-change/src/test-place-screen-size-change.cpp'
--- plugins/place/src/screen-size-change/tests/screen-size-change/src/test-place-screen-size-change.cpp	2013-04-07 08:50:44 +0000
+++ plugins/place/src/screen-size-change/tests/screen-size-change/src/test-place-screen-size-change.cpp	2013-04-16 04:26:23 +0000
@@ -25,29 +25,43 @@
 
 #include <test-screen-size-change.h>
 #include <screen-size-change.h>
-#include <iostream>
-#include <stdlib.h>
-#include <cstring>
-
-class CompPlaceScreenSizeChangeTestScreenSizeChange :
-    public CompPlaceScreenSizeChangeTest
-{
-};
-
-class MockScreenSizeChangeObject :
-    public compiz::place::ScreenSizeChangeObject
+
+namespace cp = compiz::place;
+namespace cw = compiz::window;
+
+namespace compiz
+{
+namespace window
+{
+std::ostream &
+operator<< (std::ostream &os, const Geometry &g)
+{
+    return os << "compiz::window::Geometry " << std::endl
+	      << " - x: " << g.x () << std::endl
+	      << " - y: " << g.y () << std::endl
+	      << " - width: " << g.width () << std::endl
+	      << " - height: " << g.height () << std::endl
+	      << " - border: " << g.border ();
+}
+}
+}
+
+namespace
+{
+class StubScreenSizeChangeObject :
+    public cp::ScreenSizeChangeObject
 {
     public:
 
-	MockScreenSizeChangeObject (const compiz::window::Geometry &);
-	~MockScreenSizeChangeObject ();
+	StubScreenSizeChangeObject (const cw::Geometry &);
+	~StubScreenSizeChangeObject ();
 
-	const compiz::window::Geometry & getGeometry () const;
-	void applyGeometry (compiz::window::Geometry &n,
-			    compiz::window::Geometry &o);
+	const cw::Geometry & getGeometry () const;
+	void applyGeometry (cw::Geometry &n,
+			    cw::Geometry &o);
 	const CompPoint & getViewport () const;
-	const CompRect &  getWorkarea (const compiz::window::Geometry &g) const;
-	const compiz::window::extents::Extents & getExtents () const;
+	const CompRect &  getWorkarea (const cw::Geometry &g) const;
+	const cw::extents::Extents & getExtents () const;
 
 	void setVp (const CompPoint &);
 	void setWorkArea (const CompRect &);
@@ -56,91 +70,164 @@
 			 unsigned int top,
 			 unsigned int bottom);
 
-	void setGeometry (const compiz::window::Geometry &g);
-	compiz::window::Geometry sizeAdjustTest (const CompSize &oldSize,
-						 const CompSize &newSize,
-						 CompRect &workArea);
-
-    private:
-
-	CompPoint			 mCurrentVp;
-	CompRect			 mCurrentWorkArea;
-	compiz::window::extents::Extents mCurrentExtents;
-	compiz::window::Geometry         mCurrentGeometry;
-};
-
-MockScreenSizeChangeObject::MockScreenSizeChangeObject (const compiz::window::Geometry &g) :
+	void setGeometry (const cw::Geometry &g);
+	cw::Geometry sizeAdjustTest (const CompSize &oldSize,
+				     const CompSize &newSize);
+
+    private:
+
+	CompPoint            mCurrentVp;
+	CompRect             mCurrentWorkArea;
+	cw::extents::Extents mCurrentExtents;
+	cw::Geometry         mCurrentGeometry;
+};
+
+const unsigned int MOCK_STRUT_SIZE = 24;
+
+const unsigned int WIDESCREEN_MONITOR_WIDTH = 1280;
+const unsigned int TALLER_MONITOR_HEIGHT = 1050;
+const unsigned int MONITOR_WIDTH = 1024;
+const unsigned int MONITOR_HEIGHT = 768;
+
+const unsigned int DUAL_MONITOR_WIDTH = MONITOR_WIDTH * 2;
+const unsigned int DUAL_MONITOR_HEIGHT = MONITOR_HEIGHT * 2;
+
+const unsigned int WINDOW_WIDTH = 300;
+const unsigned int WINDOW_HEIGHT = 400;
+const unsigned int WINDOW_X = (MONITOR_WIDTH / 2) - (WINDOW_WIDTH / 2);
+const unsigned int WINDOW_Y = (MONITOR_HEIGHT / 2) - (WINDOW_HEIGHT / 2);
+
+void
+reserveStruts (CompRect     &workArea,
+	       unsigned int strutSize)
+{
+    workArea.setLeft (workArea.left () + strutSize);
+    workArea.setTop (workArea.top () + strutSize);
+    workArea.setBottom (workArea.bottom () - strutSize);
+}
+}
+
+class PlaceScreenSizeChange :
+    public CompPlaceScreenSizeChangeTest
+{
+    protected:
+
+	PlaceScreenSizeChange () :
+	    windowGeometryBeforeChange (),
+	    stubScreenSizeChangeObject (windowGeometryBeforeChange)
+	{
+	}
+
+	cw::Geometry
+	ChangeScreenSizeAndAdjustWindow (const CompSize &newSize);
+
+	void
+	SetWindowGeometry (const cw::Geometry &geometry);
+
+	void
+	SetInitialScreenSize (const CompSize &size);
+
+	cw::Geometry
+	GetInitialWindowGeometry ();
+
+    private:
+
+	CompSize screenSizeAfterChange;
+	CompSize screenSizeBeforeChange;
+
+	cw::Geometry windowGeometryBeforeChange;
+
+	StubScreenSizeChangeObject stubScreenSizeChangeObject;
+};
+
+cw::Geometry
+PlaceScreenSizeChange::GetInitialWindowGeometry ()
+{
+    return windowGeometryBeforeChange;
+}
+
+void
+PlaceScreenSizeChange::SetInitialScreenSize (const CompSize &size)
+{
+    screenSizeBeforeChange = size;
+}
+
+void
+PlaceScreenSizeChange::SetWindowGeometry (const compiz::window::Geometry &geometry)
+{
+    windowGeometryBeforeChange = geometry;
+    stubScreenSizeChangeObject.setGeometry (windowGeometryBeforeChange);
+}
+
+cw::Geometry
+PlaceScreenSizeChange::ChangeScreenSizeAndAdjustWindow (const CompSize &newSize)
+{
+    cw::Geometry g (stubScreenSizeChangeObject.sizeAdjustTest (screenSizeBeforeChange,
+							       newSize));
+    screenSizeBeforeChange = newSize;
+    return g;
+}
+
+StubScreenSizeChangeObject::StubScreenSizeChangeObject (const cw::Geometry &g) :
     ScreenSizeChangeObject (g),
     mCurrentVp (0, 0),
     mCurrentWorkArea (50, 50, 1000, 1000),
     mCurrentGeometry (g)
 {
-    memset (&mCurrentExtents, 0, sizeof (compiz::window::extents::Extents));
+    memset (&mCurrentExtents, 0, sizeof (cw::extents::Extents));
 }
 
-MockScreenSizeChangeObject::~MockScreenSizeChangeObject ()
+StubScreenSizeChangeObject::~StubScreenSizeChangeObject ()
 {
 }
 
-const compiz::window::Geometry &
-MockScreenSizeChangeObject::getGeometry () const
+const cw::Geometry &
+StubScreenSizeChangeObject::getGeometry () const
 {
     return mCurrentGeometry;
 }
 
 void
-MockScreenSizeChangeObject::applyGeometry (compiz::window::Geometry &n,
-					   compiz::window::Geometry &o)
+StubScreenSizeChangeObject::applyGeometry (cw::Geometry &n,
+					   cw::Geometry &o)
 {
-    EXPECT_EQ (mCurrentGeometry, o);
-
-    std::cout << "DEBUG: new geometry : " << n.x () << " "
-					  << n.y () << " "
-					  << n.width () << " "
-					  << n.height () << " "
-					  << n.border () << std::endl;
-
-    std::cout << "DEBUG: old geometry : " << o.x () << " "
-					  << o.y () << " "
-					  << o.width () << " "
-					  << o.height () << " "
-					  << o.border () << std::endl;
+    ASSERT_EQ (mCurrentGeometry, o) << "incorrect usage of applyGeometry";
 
     mCurrentGeometry = n;
 }
 
 const CompPoint &
-MockScreenSizeChangeObject::getViewport () const
+StubScreenSizeChangeObject::getViewport () const
 {
     return mCurrentVp;
 }
 
 const CompRect &
-MockScreenSizeChangeObject::getWorkarea (const compiz::window::Geometry &g) const
+StubScreenSizeChangeObject::getWorkarea (const cw::Geometry &g) const
 {
     return mCurrentWorkArea;
 }
 
-const compiz::window::extents::Extents &
-MockScreenSizeChangeObject::getExtents () const
+const cw::extents::Extents &
+StubScreenSizeChangeObject::getExtents () const
 {
     return mCurrentExtents;
 }
 
 void
-MockScreenSizeChangeObject::setVp (const CompPoint &p)
+StubScreenSizeChangeObject::setVp (const CompPoint &p)
 {
     mCurrentVp = p;
 }
 
 void
-MockScreenSizeChangeObject::setWorkArea (const CompRect &wa)
+StubScreenSizeChangeObject::setWorkArea (const CompRect &wa)
 {
     mCurrentWorkArea = wa;
 }
 
 void
-MockScreenSizeChangeObject::setExtents (unsigned int left,
+StubScreenSizeChangeObject::setExtents (unsigned int left,
 				        unsigned int right,
 					unsigned int top,
 					unsigned int bottom)
@@ -152,266 +239,464 @@
 }
 
 void
-MockScreenSizeChangeObject::setGeometry (const compiz::window::Geometry &g)
+StubScreenSizeChangeObject::setGeometry (const cw::Geometry &g)
 {
     mCurrentGeometry = g;
 }
 
-void
-reserveStruts (CompRect &workArea)
+cw::Geometry
+StubScreenSizeChangeObject::sizeAdjustTest (const CompSize &oldSize,
+					    const CompSize &newSize)
 {
-    workArea.setLeft (workArea.left () + 24);
-    workArea.setTop (workArea.top () + 24);
-    workArea.setBottom (workArea.bottom () - 24);
-}
+    CompRect workArea (0,
+		       0,
+		       newSize.width (),
+		       newSize.height ());
 
-compiz::window::Geometry
-MockScreenSizeChangeObject::sizeAdjustTest (const CompSize &oldSize,
-					    const CompSize &newSize,
-					    CompRect &workArea)
-{
     /* Reserve top, bottom and left parts of the screen for
      * fake "24px" panels */
-    reserveStruts (workArea);
+    reserveStruts (workArea, MOCK_STRUT_SIZE);
 
     setWorkArea (workArea);
 
-    compiz::window::Geometry g = adjustForSize (oldSize, newSize);
+    cw::Geometry g = adjustForSize (oldSize, newSize);
 
     return g;
 }
 
-
-TEST_F(CompPlaceScreenSizeChangeTestScreenSizeChange, TestScreenSizeChange)
-{
-    CompSize		     current, old;
-    compiz::window::Geometry g (200, 250, 300, 400, 0);
-    compiz::window::Geometry expected;
-
-    MockScreenSizeChangeObject ms (g);
-
-    current = CompSize (1280, 800);
-
-    CompRect workArea;
-
-    /* First test that changing the screen size
-     * to something smaller here doesn't cause our
-     * (small) window to be moved */
-
-    old = current;
-    current = CompSize (1024, 768);
-    workArea = CompRect (0, 0, current.width (), current.height ());
-
-    expected = compiz::window::Geometry (200, 250, 300, 400, 0);
-
-    g = ms.sizeAdjustTest (old, current, workArea);
-
-    EXPECT_EQ (expected, g);
+TEST_F (PlaceScreenSizeChange, NoMovementOnSmallerWidth)
+{
+    SetInitialScreenSize (CompSize (WIDESCREEN_MONITOR_WIDTH,
+				    MONITOR_HEIGHT));
+    SetWindowGeometry (cw::Geometry (WINDOW_X,
+				     WINDOW_Y,
+				     WINDOW_WIDTH,
+				     WINDOW_HEIGHT,
+				     0));
+
+    /* First test that changing the screen size
+     * to something smaller here doesn't cause our
+     * (small) window to be moved */
+    cw::Geometry expectedWindowGeometryAfterChange =
+	GetInitialWindowGeometry ();
+
+    cw::Geometry windowGeometryAfterChange =
+	ChangeScreenSizeAndAdjustWindow (CompSize (MONITOR_WIDTH,
+						   MONITOR_HEIGHT));
+
+    EXPECT_EQ (expectedWindowGeometryAfterChange, windowGeometryAfterChange);
+}
+
+TEST_F (PlaceScreenSizeChange, NoMovementOnSmallerHeight)
+{
+    SetInitialScreenSize (CompSize (MONITOR_WIDTH,
+				    TALLER_MONITOR_HEIGHT));
+    SetWindowGeometry (cw::Geometry (WINDOW_X,
+				     WINDOW_Y,
+				     WINDOW_WIDTH,
+				     WINDOW_HEIGHT,
+				     0));
+
+    /* First test that changing the screen size
+     * to something smaller here doesn't cause our
+     * (small) window to be moved */
+    cw::Geometry expectedWindowGeometryAfterChange =
+	GetInitialWindowGeometry ();
+
+    cw::Geometry windowGeometryAfterChange =
+	ChangeScreenSizeAndAdjustWindow (CompSize (MONITOR_WIDTH,
+						   MONITOR_HEIGHT));
+
+    EXPECT_EQ (expectedWindowGeometryAfterChange, windowGeometryAfterChange);
+}
+
+TEST_F (PlaceScreenSizeChange, NoMovementOnLargerSize)
+{
+    SetInitialScreenSize (CompSize (MONITOR_WIDTH,
+				    MONITOR_HEIGHT));
+    SetWindowGeometry (cw::Geometry (WINDOW_X,
+				     WINDOW_Y,
+				     WINDOW_WIDTH,
+				     WINDOW_HEIGHT,
+				     0));
 
     /* Making the screen size bigger with no
      * saved geometry should cause the window not to move */
-    old = current;
-    current = CompSize (2048, 768);
-    workArea = CompRect (0, 0, current.width (), current.height ());
-
-    g = ms.sizeAdjustTest (old, current, workArea);
-
-    EXPECT_EQ (expected, g);
-
-    /* Move the window to the other "monitor" */
-    ms.setGeometry (compiz::window::Geometry (1025, 250, 300, 400, 0));
-
-    /* Unplug a "monitor" */
-    old = current;
-    current = CompSize (1024, 768);
-    workArea = CompRect (0, 0, current.width (), current.height ());
-
-    expected = compiz::window::Geometry (724, 250, 300, 400, 0);
-
-    g = ms.sizeAdjustTest (old, current, workArea);
-
-    EXPECT_EQ (expected, g);
+    cw::Geometry expectedWindowGeometryAfterChange =
+	GetInitialWindowGeometry ();
+
+    cw::Geometry windowGeometryAfterChange =
+	ChangeScreenSizeAndAdjustWindow (CompSize (DUAL_MONITOR_WIDTH,
+						   DUAL_MONITOR_HEIGHT));
+
+    EXPECT_EQ (expectedWindowGeometryAfterChange, windowGeometryAfterChange);
+}
+
+TEST_F (PlaceScreenSizeChange, MovedToEdgeOfRemainingMonitorOnUnplug)
+{
+    SetInitialScreenSize (CompSize (DUAL_MONITOR_WIDTH,
+				    DUAL_MONITOR_HEIGHT));
+    /* Move the window to the other "monitor" */
+    SetWindowGeometry (cw::Geometry (MONITOR_WIDTH + WINDOW_X,
+				     WINDOW_Y,
+				     WINDOW_WIDTH,
+				     WINDOW_HEIGHT,
+				     0));
+
+    /* Unplug a "monitor" */
+    cw::Geometry windowGeometryAfterChange =
+	ChangeScreenSizeAndAdjustWindow (CompSize (MONITOR_WIDTH,
+						   MONITOR_HEIGHT));
+
+    /* The window should be exactly on-screen at the edge */
+    cw::Geometry expectedWindowGeometryAfterChange (MONITOR_WIDTH - WINDOW_WIDTH,
+						    WINDOW_Y,
+						    WINDOW_WIDTH,
+						    WINDOW_HEIGHT,
+						    0);
+    EXPECT_EQ (expectedWindowGeometryAfterChange, windowGeometryAfterChange);
+}
+
+TEST_F (PlaceScreenSizeChange, MovedBackToOriginalPositionOnReplug)
+{
+    SetInitialScreenSize (CompSize (DUAL_MONITOR_WIDTH, MONITOR_HEIGHT));
+    /* Move the window to the other "monitor" */
+    SetWindowGeometry (cw::Geometry (MONITOR_WIDTH + WINDOW_X,
+				     WINDOW_Y,
+				     WINDOW_WIDTH,
+				     WINDOW_HEIGHT,
+				     0));
+
+    /* Unplug a "monitor" */
+    ChangeScreenSizeAndAdjustWindow (CompSize (MONITOR_WIDTH, MONITOR_HEIGHT));
 
     /* Re-plug the monitor - window should go back
      * to the same position */
-    old = current;
-    current = CompSize (2048, 768);
-    workArea = CompRect (0, 0, current.width (), current.height ());
-
-    expected = compiz::window::Geometry (1025, 250, 300, 400, 0);
-
-    g = ms.sizeAdjustTest (old, current, workArea);
-
-    EXPECT_EQ (expected, g);
+    cw::Geometry windowGeometryAfterChange =
+	ChangeScreenSizeAndAdjustWindow (CompSize (DUAL_MONITOR_WIDTH, MONITOR_HEIGHT));
+
+    /* Window should be at the same position we left it at */
+    cw::Geometry expectedWindowGeometryAfterChange (MONITOR_WIDTH + WINDOW_X,
+						    WINDOW_Y,
+						    WINDOW_WIDTH,
+						    WINDOW_HEIGHT,
+						    0);
+    EXPECT_EQ (expectedWindowGeometryAfterChange, windowGeometryAfterChange);
+}
+
+TEST_F (PlaceScreenSizeChange, MovedBackToOriginalPositionOnExpansion)
+{
+    SetInitialScreenSize (CompSize (DUAL_MONITOR_WIDTH,
+				    MONITOR_HEIGHT));
+    SetWindowGeometry (cw::Geometry (MONITOR_WIDTH + WINDOW_X,
+				     WINDOW_Y,
+				     WINDOW_WIDTH,
+				     WINDOW_HEIGHT,
+				     0));
+
+    /* Unplug a "monitor" */
+    ChangeScreenSizeAndAdjustWindow (CompSize (MONITOR_WIDTH,
+					       MONITOR_HEIGHT));
+
+    /* Re-plug the monitor */
+    ChangeScreenSizeAndAdjustWindow (CompSize (DUAL_MONITOR_WIDTH,
+					       MONITOR_HEIGHT));
 
     /* Plug 2 monitors downwards, no change */
-    old = current;
-    current = CompSize (2048, 1536);
-    workArea = CompRect (0, 0, current.width (), current.height ());
-
-    g = ms.sizeAdjustTest (old, current, workArea);
-
-    EXPECT_EQ (expected, g);
-
-    /* Move the window to the bottom "monitor" */
-    ms.setGeometry (compiz::window::Geometry (1025, 791, 300, 400, 0));
-
-    /* Unplug bottom "monitor" */
-    old = current;
-    current = CompSize (2048, 768);
-    workArea = CompRect (0, 0, current.width (), current.height ());
-
-    expected = compiz::window::Geometry (1025, 344, 300, 400, 0);
-
-    g = ms.sizeAdjustTest (old, current, workArea);
-
-    EXPECT_EQ (expected, g);
+    cw::Geometry windowGeometryAfterChange =
+	ChangeScreenSizeAndAdjustWindow (CompSize (DUAL_MONITOR_WIDTH,
+						   DUAL_MONITOR_HEIGHT));
+
+    cw::Geometry expectedWindowGeometryAfterChange (MONITOR_WIDTH + WINDOW_X,
+						    WINDOW_Y,
+						    WINDOW_WIDTH,
+						    WINDOW_HEIGHT,
+						    0);
+    EXPECT_EQ (expectedWindowGeometryAfterChange, windowGeometryAfterChange);
+}
+
+TEST_F (PlaceScreenSizeChange, NoOverlapStrutsOnRePlacement)
+{
+    SetInitialScreenSize (CompSize (DUAL_MONITOR_WIDTH,
+				    MONITOR_HEIGHT));
+    SetWindowGeometry (cw::Geometry (WINDOW_X,
+				     WINDOW_Y,
+				     WINDOW_WIDTH,
+				     WINDOW_HEIGHT,
+				     0));
+    ChangeScreenSizeAndAdjustWindow (CompSize (DUAL_MONITOR_WIDTH,
+					       DUAL_MONITOR_HEIGHT));
+
+    /* Move the window to the bottom "monitor" */
+    SetWindowGeometry (cw::Geometry (MONITOR_WIDTH + WINDOW_X,
+				     MONITOR_HEIGHT + WINDOW_Y,
+				     WINDOW_WIDTH,
+				     WINDOW_HEIGHT,
+				     0));
+
+    /* Unplug bottom "monitor" */
+    cw::Geometry windowGeometryAfterChange =
+	ChangeScreenSizeAndAdjustWindow (CompSize (DUAL_MONITOR_WIDTH,
+						   MONITOR_HEIGHT));
+
+    cw::Geometry expectedWindowGeometryAfterChange (MONITOR_WIDTH + WINDOW_X,
+						    MONITOR_HEIGHT -
+						    WINDOW_HEIGHT -
+						    MOCK_STRUT_SIZE,
+						    WINDOW_WIDTH,
+						    WINDOW_HEIGHT,
+						    0);
+
+    EXPECT_EQ (expectedWindowGeometryAfterChange, windowGeometryAfterChange);
+}
+
+TEST_F (PlaceScreenSizeChange, MovedToOriginalPositionOnPerpendicularExpansion)
+{
+    SetInitialScreenSize (CompSize (DUAL_MONITOR_WIDTH,
+				    DUAL_MONITOR_HEIGHT));
+    SetWindowGeometry (cw::Geometry (WINDOW_X,
+				     WINDOW_Y,
+				     WINDOW_WIDTH,
+				     WINDOW_HEIGHT,
+				     0));
+
+    ChangeScreenSizeAndAdjustWindow (CompSize (DUAL_MONITOR_WIDTH,
+					       DUAL_MONITOR_HEIGHT));
+
+    /* Move the window to the bottom "monitor" */
+    SetWindowGeometry (cw::Geometry (MONITOR_WIDTH + WINDOW_X,
+				     MONITOR_HEIGHT + WINDOW_Y,
+				     WINDOW_WIDTH,
+				     WINDOW_HEIGHT,
+				     0));
+
+    /* Unplug bottom "monitor" */
+    ChangeScreenSizeAndAdjustWindow (CompSize (DUAL_MONITOR_WIDTH,
+					       MONITOR_HEIGHT));
 
     /* Re-plug bottom "monitor" */
-    old = current;
-    current = CompSize (2048, 1356);
-    workArea = CompRect (0, 0, current.width (), current.height ());
-
-    expected = compiz::window::Geometry (1025, 791, 300, 400, 0);
-
-    g = ms.sizeAdjustTest (old, current, workArea);
-
-    EXPECT_EQ (expected, g);
-
+    cw::Geometry windowGeometryAfterChange =
+	ChangeScreenSizeAndAdjustWindow (CompSize (DUAL_MONITOR_WIDTH,
+						   DUAL_MONITOR_HEIGHT));
+
+    cw::Geometry expectedWindowGeometryAfterChange (MONITOR_WIDTH + WINDOW_X,
+						    MONITOR_HEIGHT + WINDOW_Y,
+						    WINDOW_WIDTH,
+						    WINDOW_HEIGHT,
+						    0);
+    EXPECT_EQ (expectedWindowGeometryAfterChange, windowGeometryAfterChange);
 }
 
-TEST_F(CompPlaceScreenSizeChangeTestScreenSizeChange, TestScreenChangeWindowsOnSecondViewport)
+TEST_F (PlaceScreenSizeChange, RemainOnSecondViewportWhenConstraineToFirst)
 {
-    CompSize		     current, old;
-    compiz::window::Geometry g (1025, 791, 300, 400, 0);
-    compiz::window::Geometry expected;
-
-    MockScreenSizeChangeObject ms (g);
-
-    current = CompSize (2048, 1356);
-
-    CompRect workArea;
-
+    /* Unplug a "monitor" */
+    SetInitialScreenSize (CompSize (DUAL_MONITOR_WIDTH,
+				    DUAL_MONITOR_HEIGHT));
     /* Move the entire window right a viewport */
-    g.setPos (g.pos () + CompPoint (current.width (), 0));
-    ms.setGeometry (g);
+    SetWindowGeometry (cw::Geometry (DUAL_MONITOR_WIDTH +
+				     MONITOR_WIDTH +
+				     WINDOW_X,
+				     WINDOW_Y,
+				     WINDOW_WIDTH,
+				     WINDOW_HEIGHT,
+				     0));
+
+    cw::Geometry expectedWindowGeometryAfterChange (MONITOR_WIDTH +
+						    (MONITOR_WIDTH -
+						     WINDOW_WIDTH),
+						    WINDOW_Y,
+						    WINDOW_WIDTH,
+						    WINDOW_HEIGHT,
+						    0);
 
     /* Now change the screen resolution again - the window should
      * move to be within the constrained size of its current
      * viewport */
+    cw::Geometry windowGeometryAfterChange =
+	ChangeScreenSizeAndAdjustWindow (CompSize (MONITOR_WIDTH,
+						   DUAL_MONITOR_HEIGHT));
+    EXPECT_EQ (expectedWindowGeometryAfterChange, windowGeometryAfterChange);
+}
+
+TEST_F (PlaceScreenSizeChange, RemainOnSecondViewportAfterMovedToOriginalPosition)
+{
+    SetInitialScreenSize (CompSize (DUAL_MONITOR_WIDTH,
+				    DUAL_MONITOR_HEIGHT));
+    /* Move the entire window right a viewport */
+    SetWindowGeometry (cw::Geometry (DUAL_MONITOR_WIDTH +
+				     MONITOR_WIDTH +
+				     WINDOW_X,
+				     WINDOW_Y,
+				     WINDOW_WIDTH,
+				     WINDOW_HEIGHT,
+				     0));
 
     /* Unplug a "monitor" */
-    old = current;
-    current = CompSize (1024, 1356);
-    workArea = CompRect (0, 0, current.width (), current.height ());
-
-    expected = compiz::window::Geometry (current.width () + 724, 791, 300, 400, 0);
-
-    g = ms.sizeAdjustTest (old, current, workArea);
-
-    EXPECT_EQ (expected, g);
+    ChangeScreenSizeAndAdjustWindow (CompSize (MONITOR_WIDTH,
+					       DUAL_MONITOR_HEIGHT));
 
     /* Replug the monitor, make sure that the geometry is restored */
-    old = current;
-    current = CompSize (2048, 1356);
-    workArea = CompRect (0, 0, current.width (), current.height ());
-
-    expected = compiz::window::Geometry (current.width () + 1025, 791, 300, 400, 0);
-
-    g = ms.sizeAdjustTest (old, current, workArea);
-
-    EXPECT_EQ (expected, g);
+    cw::Geometry expectedWindowGeometryAfterChange (DUAL_MONITOR_WIDTH +
+						    MONITOR_WIDTH +
+						    WINDOW_X,
+						    WINDOW_Y,
+						    WINDOW_WIDTH,
+						    WINDOW_HEIGHT,
+						    0);
+
+    cw::Geometry windowGeometryAfterChange =
+	ChangeScreenSizeAndAdjustWindow (CompSize (DUAL_MONITOR_WIDTH,
+						   DUAL_MONITOR_HEIGHT));
+    EXPECT_EQ (expectedWindowGeometryAfterChange, windowGeometryAfterChange);
+}
+
+TEST_F (PlaceScreenSizeChange, RemainAtOriginalPositionOnSecondViewportUnplug)
+{
+    SetInitialScreenSize (CompSize (DUAL_MONITOR_WIDTH,
+				    DUAL_MONITOR_HEIGHT));
+    SetWindowGeometry (cw::Geometry (DUAL_MONITOR_WIDTH + WINDOW_X,
+				     MONITOR_HEIGHT + WINDOW_Y,
+				     WINDOW_WIDTH,
+				     WINDOW_HEIGHT,
+				     0));
+
+    cw::Geometry windowGeometryAfterChange =
+	ChangeScreenSizeAndAdjustWindow (CompSize (MONITOR_WIDTH,
+						   DUAL_MONITOR_HEIGHT));
+
+    cw::Geometry expectedWindowGeometryAfterChange (MONITOR_WIDTH + WINDOW_X,
+						    MONITOR_HEIGHT + WINDOW_Y,
+						    WINDOW_WIDTH,
+						    WINDOW_HEIGHT,
+						    0);
+    EXPECT_EQ (expectedWindowGeometryAfterChange, windowGeometryAfterChange);
+}
+
+TEST_F (PlaceScreenSizeChange, RemainAtOriginalPositionOnSecondViewportReplug)
+{
+    SetInitialScreenSize (CompSize (DUAL_MONITOR_WIDTH,
+				    DUAL_MONITOR_HEIGHT));
+    SetWindowGeometry (cw::Geometry (DUAL_MONITOR_WIDTH + WINDOW_X,
+				     MONITOR_HEIGHT + WINDOW_Y,
+				     WINDOW_WIDTH,
+				     WINDOW_HEIGHT,
+				     0));
+
+    ChangeScreenSizeAndAdjustWindow (CompSize (MONITOR_WIDTH,
+					       DUAL_MONITOR_HEIGHT));
 
     /* Replug the monitor and move the window to where it fits on the first
      * monitor on the second viewport, then make sure it doesn't move */
-    old = CompSize (2048, 1356);
-    current = CompSize (1024, 1356);
-    workArea = CompRect (0, 0, current.width (), current.height ());
-
-    g.setPos (CompPoint (old.width () + 25, 791));
-    ms.setGeometry (g);
-    /* clear the saved geometry */
-    ms.unset();
-
-    expected = compiz::window::Geometry (current.width () + 25, 791, 300, 400, 0);
-
-    g = ms.sizeAdjustTest (old, current, workArea);
-
-    EXPECT_EQ (expected, g);
-
-    old = current;
-    current = CompSize (2048, 1356);
-    workArea = CompRect (0, 0, current.width (), current.height ());
-
-    expected = compiz::window::Geometry (current.width () + 25, 791, 300, 400, 0);
-
-    g = ms.sizeAdjustTest (old, current, workArea);
-
-    EXPECT_EQ (expected, g);
-}
-
-TEST_F(CompPlaceScreenSizeChangeTestScreenSizeChange, TestScreenChangeWindowsOnPreviousViewport)
-{
-    CompSize		     current, old;
-    compiz::window::Geometry g (0, 0, 300, 400, 0);
-    compiz::window::Geometry expected;
-
-    MockScreenSizeChangeObject ms (g);
-
-    current = CompSize (2048, 1356);
-
-    CompRect workArea;
-
-    /* Deal with the case where the position is negative, which means
-     * it's actually wrapped around to the rightmost viewport
-     */
-    g.setPos (CompPoint (-300, 200));
-    ms.setGeometry (g);
-
-    expected = g;
-
-    /* Unplug the right "monitor" */
-    old = current;
-    current = CompSize (1024, 1356);
-    workArea = CompRect (0, 0, current.width (), current.height ());
-
-    g = ms.sizeAdjustTest (old, current, workArea);
-
-    EXPECT_EQ (expected, g);
+    cw::Geometry windowGeometryAfterChange =
+	ChangeScreenSizeAndAdjustWindow (CompSize (DUAL_MONITOR_WIDTH,
+						   DUAL_MONITOR_HEIGHT));
+
+    cw::Geometry expectedWindowGeometryAfterChange = GetInitialWindowGeometry ();
+    EXPECT_EQ (expectedWindowGeometryAfterChange, windowGeometryAfterChange);
+}
+
+TEST_F (PlaceScreenSizeChange, RemainOnPreviousViewportWhenMovedToFirstMonitor)
+{
+    SetInitialScreenSize (CompSize (DUAL_MONITOR_WIDTH,
+				    DUAL_MONITOR_HEIGHT));
+
+    /* Deal with the case where the position is negative, which means
+     * it's actually wrapped around to the rightmost viewport
+     */
+    SetWindowGeometry (cw::Geometry (WINDOW_X - MONITOR_WIDTH,
+				     WINDOW_Y,
+				     WINDOW_WIDTH,
+				     WINDOW_HEIGHT,
+				     0));
+
+    /* Unplug the right "monitor" */
+    cw::Geometry windowGeometryAfterChange =
+	ChangeScreenSizeAndAdjustWindow (CompSize (MONITOR_WIDTH,
+						   DUAL_MONITOR_HEIGHT));
+
+    cw::Geometry expectedWindowGeometryAfterChange (-WINDOW_WIDTH,
+						    WINDOW_Y,
+						    WINDOW_WIDTH,
+						    WINDOW_HEIGHT,
+						    0);
+    EXPECT_EQ (expectedWindowGeometryAfterChange, windowGeometryAfterChange);
+}
+
+TEST_F (PlaceScreenSizeChange, RemainOnPreviousViewportWhenRestored)
+{
+    SetInitialScreenSize (CompSize (DUAL_MONITOR_WIDTH,
+				    DUAL_MONITOR_HEIGHT));
+
+    /* Deal with the case where the position is negative, which means
+     * it's actually wrapped around to the rightmost viewport
+     */
+    SetWindowGeometry (cw::Geometry (WINDOW_X - MONITOR_WIDTH,
+				     WINDOW_Y,
+				     WINDOW_WIDTH,
+				     WINDOW_HEIGHT,
+				     0));
+
+    /* Unplug the right "monitor" */
+    ChangeScreenSizeAndAdjustWindow (CompSize (MONITOR_WIDTH,
+					       DUAL_MONITOR_HEIGHT));
 
     /* Re-plug the right "monitor" */
-    old = current;
-    current = CompSize (2048, 1356);
-    workArea = CompRect (0, 0, current.width (), current.height ());
-
-    g = ms.sizeAdjustTest (old, current, workArea);
-
-    EXPECT_EQ (expected, g);
-
-    /* Move the window to the left monitor, verify that it survives an
-     * unplug/plug cycle
-     */
-    g.setPos (CompPoint (-1324, 200));
-    ms.setGeometry (g);
-
-    old = current;
-    current = CompSize (1024, 1356);
-    workArea = CompRect (0, 0, current.width (), current.height ());
-
-    expected = compiz::window::Geometry (-300, 200, 300, 400, 0);
-
-    g = ms.sizeAdjustTest (old, current, workArea);
-
-    EXPECT_EQ (expected, g);
-
-    old = current;
-    current = CompSize (2048, 1356);
-    workArea = CompRect (0, 0, current.width (), current.height ());
-
-    expected = compiz::window::Geometry (-1324, 200, 300, 400, 0);
-
-    g = ms.sizeAdjustTest (old, current, workArea);
-
-    EXPECT_EQ (expected, g);
-
+    cw::Geometry windowGeometryAfterChange =
+	ChangeScreenSizeAndAdjustWindow (CompSize (DUAL_MONITOR_WIDTH,
+						   DUAL_MONITOR_HEIGHT));
+
+    cw::Geometry expectedWindowGeometryAfterChange = GetInitialWindowGeometry ();
+    EXPECT_EQ (expectedWindowGeometryAfterChange, windowGeometryAfterChange);
+}
+
+TEST_F (PlaceScreenSizeChange, RemainOnPreviousViewportFirstMonitor)
+{
+    SetInitialScreenSize (CompSize (DUAL_MONITOR_WIDTH,
+				    DUAL_MONITOR_HEIGHT));
+
+    /* Move the window to the left monitor, verify that it survives an
+     * unplug/plug cycle
+     */
+    SetWindowGeometry (cw::Geometry (WINDOW_X - DUAL_MONITOR_WIDTH,
+				     WINDOW_Y,
+				     WINDOW_WIDTH,
+				     WINDOW_HEIGHT,
+				     0));
+
+    cw::Geometry windowGeometryAfterChange =
+	ChangeScreenSizeAndAdjustWindow (CompSize (MONITOR_WIDTH,
+						   DUAL_MONITOR_HEIGHT));
+
+    cw::Geometry expectedWindowGeometryAfterChange (WINDOW_X - MONITOR_WIDTH,
+						    WINDOW_Y,
+						    WINDOW_WIDTH,
+						    WINDOW_HEIGHT,
+						    0);
+    EXPECT_EQ (expectedWindowGeometryAfterChange, windowGeometryAfterChange);
+}
+
+TEST_F (PlaceScreenSizeChange, RemainOnPreviousViewportFirstMonitorWhenRestored)
+{
+    SetInitialScreenSize (CompSize (DUAL_MONITOR_WIDTH,
+				    DUAL_MONITOR_HEIGHT));
+
+    /* Move the window to the left monitor, verify that it survives an
+     * unplug/plug cycle
+     */
+    SetWindowGeometry (cw::Geometry (WINDOW_X - DUAL_MONITOR_WIDTH,
+				     WINDOW_Y,
+				     WINDOW_WIDTH,
+				     WINDOW_HEIGHT,
+				     0));
+
+    ChangeScreenSizeAndAdjustWindow (CompSize (MONITOR_WIDTH,
+					       DUAL_MONITOR_HEIGHT));
+
+    cw::Geometry windowGeometryAfterChange =
+	ChangeScreenSizeAndAdjustWindow (CompSize (DUAL_MONITOR_WIDTH,
+						   DUAL_MONITOR_HEIGHT));
+
+    cw::Geometry expectedWindowGeometryAfterChange
+	= GetInitialWindowGeometry ();
+    EXPECT_EQ (expectedWindowGeometryAfterChange, windowGeometryAfterChange);
 }