Merge lp:~paulliu/unity/phablet_add-qmluitest1 into lp:unity/phablet

[Ying-Chun Liu]

Add test for Panel/Menus/IndicatorMenuWindow.

Add test for Panel/Menus/IndicatorMenuWindow.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:578
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/453/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/218
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/454
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/454/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/453
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/329
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/329/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/333
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/910

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/453/rebuild

review: Approve (continuous-integration)

[Daniel d'Andrada (dandrader)]

60 + compare(indicatorMenuWindow.headingText, indicatorMenuWindow.name, "IndicatorMenuWindow name != headingText")
61 + compare(indicatorMenuWindow.contentHeight, testIndicatorMenuWindowItem.height, "IndicatorMenuWindow contentHeight != height")

Please avoid such long lines. Avoid having anything longer than 90 chars. Long lines makes having split windows (side-by-side code windows) a pain.

[Daniel d'Andrada (dandrader)]

Looking at IndicatorMenuWindow.qml, I don't think it makes sense to test it at all. There's virtually no logic in there. Nothing worth testing.

This test is also at a 1:1 mapping of the code being tested. It doesn't make much sense.

e.g.:

code under test:
   opacity: shown ? 1.0 : 0.0
test code:
    compare(indicatorMenuWindow.opacity, 0.0, "Init opacity is not zero")
    indicatorMenuWindow.shown=true
    tryCompare(indicatorMenuWindow, "opacity", 1.0)
    indicatorMenuWindow.shown=false
    tryCompare(indicatorMenuWindow, "opacity", 0.0)

I fail to see the value of such tests.

review: Disapprove

[Daniel d'Andrada (dandrader)]

> Looking at IndicatorMenuWindow.qml, I don't think it makes sense to test it at
> all. There's virtually no logic in there. Nothing worth testing.
>
> This test is also at a 1:1 mapping of the code being tested. It doesn't make
> much sense.
>
> e.g.:
>
> code under test:
> opacity: shown ? 1.0 : 0.0
> test code:
> compare(indicatorMenuWindow.opacity, 0.0, "Init opacity is not zero")
> indicatorMenuWindow.shown=true
> tryCompare(indicatorMenuWindow, "opacity", 1.0)
> indicatorMenuWindow.shown=false
> tryCompare(indicatorMenuWindow, "opacity", 0.0)
>
> I fail to see the value of such tests.

I mean, this is like testing if a QML expression is doing what it should do. It's like testing QML itself.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:588
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/506/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-phablet-qmluitests/284/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/507/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/506/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/382/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/386/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/506/rebuild

review: Needs Fixing (continuous-integration)

[Michael Zanetti (mzanetti)]

I agree with Daniel here. I think what could be tested is that the Window in fact does not let any mouse clicks pass through (which is the actual purpose of it).

In a nutshell:
- Open the Window, make it invisible
- Click somewhere in the window, make sure the click passes through
- Make it visible
- Click again in the window, make sure the click does NOT pass through it

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:609
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/624/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/429
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/626
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/626/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/624
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/500
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/500/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/504
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1148

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/624/rebuild

review: Approve (continuous-integration)

[Ying-Chun Liu (paulliu)]

Need review again.

[Andrea Cimitan (cimi)]

Indentation needs fixes and extra lines to be removed

[Michael Zanetti (mzanetti)]

12 === added file 'tests/qmltests/Panel/Menus/CMakeLists.txt'
16 +add_qml_test(IndicatorMenuWindow)

This CMakeLists.txt file is not needed any more. Delete it.

review: Needs Fixing

[Michael Zanetti (mzanetti)]

94 + function test_mouseEvent() {
95 + indicatorMenuWindow.shown=true
96 + tryCompare(indicatorMenuWindow, "opacity", 1.0)
97 +
98 + clickedSpy.clear()
99 + mouseClick(indicatorMenuWindow, indicatorMenuWindow.width / 2,
100 + indicatorMenuWindow.height / 2)
101 + compare(clickedSpy.count, 0,
102 + "Mouse event should be eaten by indicatorMenuWindow")
103 +
104 + indicatorMenuWindow.shown=false
105 + tryCompare(indicatorMenuWindow, "opacity", 0.0)
106 +
107 + clickedSpy.clear()
108 + mouseClick(indicatorMenuWindow, indicatorMenuWindow.width / 2,
109 + indicatorMenuWindow.height / 2)
110 + compare(clickedSpy.count, 1,
111 + "Mouse event should not be eaten by indicatorMenuWindow")
112 + }

I think this would be a perfect candidate for using the _data() mechanism.

Example:

        function test_mouseEvent_data() {
            return [
                {tag: "invisible", shown: true, opacity: 1.0, mouseClicks: 0},
                {tag: "visible", shown: false, opacity: 0.0, mouseClicks: 1},
            ]
        }

        function test_mouseEvent(data) {
            indicatorMenuWindow.shown = data.shown
            tryCompare(indicatorMenuWindow, "opacity", data.opacity)

            clickedSpy.clear()
            mouseClick(indicatorMenuWindow, indicatorMenuWindow.width / 2,
                       indicatorMenuWindow.height / 2)
            compare(clickedSpy.count, data.mouseClicks,
                    "Check for Mouse event eating by indicatorMenuWindow failed")
        }

This way you don't have to copy/paste the test code and the debug output gives better feedback when a test fails.

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:642
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/781/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/662
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/657
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/657/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/661
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1329

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/781/rebuild

review: Approve (continuous-integration)

[Michael Zanetti (mzanetti)]

thanks.

review: Approve

[Daniel d'Andrada (dandrader)]

I would use more meaningful names for those rectangles, like backgroundRect and foregroundRect, instead of simply rectangle1 and rectangle2.

[Daniel d'Andrada (dandrader)]

> I would use more meaningful names for those rectangles, like backgroundRect
> and foregroundRect, instead of simply rectangle1 and rectangle2.

Or rootRectangle for rectangle1

[Daniel d'Andrada (dandrader)]

89 + {tag: "invisible", shown: true, opacity: 1.0, mouseClicks: 0},
90 + {tag: "visible", shown: false, opacity: 0.0, mouseClicks: 1},

Looks like you've swapped the "visible" and "invisible" tags...

[Daniel d'Andrada (dandrader)]

By the way, what's the purpose of this red rectangle (id: rectangle2)? It doesn't seem to be used in the tests.
Looks like you can just remove it.

review: Needs Information

[Daniel d'Andrada (dandrader)]

Also rectangle1 could be the root item itself. Or maybe even that MouseArea.

=== modified file 'tests/qmltests/CMakeLists.txt'
--- tests/qmltests/CMakeLists.txt	2013-04-26 10:02:29 +0000
+++ tests/qmltests/CMakeLists.txt	2013-04-29 16:27:26 +0000
@@ -56,4 +56,5 @@
 add_qml_test(Panel Overview)
 add_qml_test(Panel Panel)
 add_qml_test(Panel SearchIndicator)
+add_qml_test(Panel/Menus IndicatorMenuWindow IMPORT_PATHS ${qmltest_DEFAULT_IMPORT_PATHS})
 add_qml_test(SideStage SideStage IMPORT_PATHS ${qmltest_DEFAULT_IMPORT_PATHS} ${CMAKE_BINARY_DIR}/tests/mocks)

=== added directory 'tests/qmltests/Panel/Menus'
=== added file 'tests/qmltests/Panel/Menus/tst_IndicatorMenuWindow.qml'
--- tests/qmltests/Panel/Menus/tst_IndicatorMenuWindow.qml	1970-01-01 00:00:00 +0000
+++ tests/qmltests/Panel/Menus/tst_IndicatorMenuWindow.qml	2013-04-29 16:27:26 +0000
@@ -0,0 +1,90 @@
+/*
+ * Copyright 2013 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import QtQuick 2.0
+import QtTest 1.0
+import "../.."
+import "../../../../Panel/Menus"
+import Ubuntu.Components 0.1
+import Unity.Test 0.1 as UT
+
+Item {
+    id: testIndicatorMenuWindowItem
+    width: units.gu(9)
+    height: units.gu(3)
+
+    Rectangle {
+        id: rectangle1
+        anchors.fill: parent
+        signal clicked
+
+        MouseArea {
+            anchors.fill: parent
+            onClicked: rectangle1.clicked()
+        }
+
+        Rectangle {
+            anchors.right: rectangle1.right
+            anchors.left: rectangle1.horizontalCenter
+            anchors.top: rectangle1.verticalCenter
+            anchors.bottom: rectangle1.bottom
+            color: "green"
+            IndicatorMenuWindow {
+                id: indicatorMenuWindow
+                name: "TestIndicatorMenuWindow"
+                anchors.fill: parent
+            }
+        }
+
+        Rectangle {
+            id: rectangle2
+            color: "red"
+            anchors.right: rectangle1.horizontalCenter
+            anchors.left: rectangle1.left
+            anchors.top: rectangle1.verticalCenter
+            anchors.bottom: rectangle1.bottom
+        }
+
+        SignalSpy {
+            id: clickedSpy
+            target: rectangle1
+            signalName: "clicked"
+        }
+    }
+
+    UT.UnityTestCase {
+        name: "IndicatorMenuWindow"
+        when: windowShown
+
+        function test_mouseEvent_data() {
+            return [
+                {tag: "invisible", shown: true, opacity: 1.0, mouseClicks: 0},
+                {tag: "visible", shown: false, opacity: 0.0, mouseClicks: 1},
+            ]
+        }
+
+        function test_mouseEvent(data) {
+            indicatorMenuWindow.shown = data.shown
+            tryCompare(indicatorMenuWindow, "opacity", data.opacity)
+
+            clickedSpy.clear()
+            mouseClick(indicatorMenuWindow, indicatorMenuWindow.width / 2,
+                       indicatorMenuWindow.height / 2)
+            compare(clickedSpy.count, data.mouseClicks,
+                  "Check for Mouse event eating by indicatorMenuWindow failed")
+        }
+    }
+}