Merge lp:~diegosarmentero/unity8/reset-button into lp:unity8

[Diego Sarmentero]

- Reset button state on cancel

[Michał Sawicz (saviq)]

Could we get a test for this please.

review: Needs Fixing

[lp:~diegosarmentero/unity8/reset-button]

1207. By Diego Sarmentero on 2014-08-27

adding tests

[Diego Sarmentero (diegosarmentero)]

> Could we get a test for this please.

When trying to run the tests, for some reason this is not being properly executed in the mock:

            var paymentClient = findChild(root, "paymentClient");
            paymentClient.storeItemId = "com.example.invalid";
            paymentClient.start();

so the purchaseError signal the test need is not being emitted.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1206
http://jenkins.qa.ubuntu.com/job/unity8-ci/4135/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/4118
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/1136
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/1229
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/1229
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/3950
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5370
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5370/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12206

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/4135/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1207
http://jenkins.qa.ubuntu.com/job/unity8-ci/4143/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/4130
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/1144
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/1237
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/1237
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/3962
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5382
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5382/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12223

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/4143/rebuild

review: Needs Fixing (continuous-integration)

[Michał Sawicz (saviq)]

Hey, this would be my approach to making the test work:

http://paste.ubuntu.com/8167187/

But... Why do we even want this? purchaseError triggers the "purchaseError" action, shouldn't that result in the preview being reloaded anyway, so bringing the button back is weird as it will be replaced in a moment anyway?

If we want to treat cancel and error differently, we need separate purchaseCancel on the client, don't we?

review: Needs Information

[lp:~diegosarmentero/unity8/reset-button]

1208. By Diego Sarmentero on 2014-08-29

fixing tests as suggested

[Diego Sarmentero (diegosarmentero)]

> Hey, this would be my approach to making the test work:
>
> http://paste.ubuntu.com/8167187/
>
> But... Why do we even want this? purchaseError triggers the "purchaseError"
> action, shouldn't that result in the preview being reloaded anyway, so
> bringing the button back is weird as it will be replaced in a moment anyway?
>
> If we want to treat cancel and error differently, we need separate
> purchaseCancel on the client, don't we?

Tests fixed as suggested.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1208
http://jenkins.qa.ubuntu.com/job/unity8-ci/4177/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/4236
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/1178
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/1271
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/1271
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/4050
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5488
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5488/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12357

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/4177/rebuild

review: Needs Fixing (continuous-integration)

[Diego Sarmentero (diegosarmentero)]

> Hey, this would be my approach to making the test work:
>
> http://paste.ubuntu.com/8167187/
>
> But... Why do we even want this? purchaseError triggers the "purchaseError"
> action, shouldn't that result in the preview being reloaded anyway, so
> bringing the button back is weird as it will be replaced in a moment anyway?
>
> If we want to treat cancel and error differently, we need separate
> purchaseCancel on the client, don't we?

After the discussion we had on irc regarding the architectural changes that we need to do here for cancel, success and error, we have been discussing this with alecu and ted to find which are the changes that are needed.
The code in this branch will be needed in the future for those changes anyway, and it's needed now to not block the preview, so the idea was to land this if possible (because right now it's basically a bad naming thing were the signal should be Canceled instead of Error), and include the other signal later when pay-service can differenciate the different errors.

[Michał Sawicz (saviq)]

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Y
 * Did CI run pass? If not, please explain why.
One failure fixed elsewhere.

review: Approve

=== modified file 'qml/Dash/Previews/PreviewPayments.qml'
--- qml/Dash/Previews/PreviewPayments.qml	2014-08-19 13:53:13 +0000
+++ qml/Dash/Previews/PreviewPayments.qml	2014-08-29 11:44:04 +0000
@@ -49,6 +49,7 @@
 
         Payments {
             id: paymentClient
+            objectName: "paymentClient"
 
             property var source: widgetData["source"]
 
@@ -59,6 +60,7 @@
                 root.triggered(widgetId, "purchaseCompleted", source);
             }
             onPurchaseError: {
+                paymentButton.opacity = 1;
                 root.triggered(widgetId, "purchaseError", source);
             }
         }

=== modified file 'tests/mocks/Ubuntu/Payments/MockPayments.cpp'
--- tests/mocks/Ubuntu/Payments/MockPayments.cpp	2014-06-30 14:43:01 +0000
+++ tests/mocks/Ubuntu/Payments/MockPayments.cpp	2014-08-29 11:44:04 +0000
@@ -77,13 +77,16 @@
 
 void MockPayments::start()
 {
-    if (!m_store_item_id.isEmpty()) {
-        if (m_store_item_id == "com.example.invalid") {
-            Q_EMIT purchaseError("Purchase failed.");
-        } else {
-            Q_EMIT purchaseCompleted();
-        }
+    if (m_store_item_id.isEmpty()) {
+        Q_EMIT purchaseError("No item ID supplied.");
+    }
+}
+
+void MockPayments::process()
+{
+    if (m_store_item_id == "com.example.invalid") {
+        Q_EMIT purchaseError("Purchase failed.");
     } else {
-        Q_EMIT purchaseError("No item ID supplied.");
+        Q_EMIT purchaseCompleted();
     }
 }

=== modified file 'tests/mocks/Ubuntu/Payments/MockPayments.h'
--- tests/mocks/Ubuntu/Payments/MockPayments.h	2014-06-27 20:54:41 +0000
+++ tests/mocks/Ubuntu/Payments/MockPayments.h	2014-08-29 11:44:04 +0000
@@ -41,6 +41,7 @@
     void setPrice(const double price);
     void setStoreItemId(const QString& store_item_id);
     Q_INVOKABLE void start();
+    Q_INVOKABLE void process(); // only available for testing
 
 Q_SIGNALS:
     void currencyChanged(const QString& currency);

=== modified file 'tests/qmltests/Dash/Previews/tst_PreviewPayments.qml'
--- tests/qmltests/Dash/Previews/tst_PreviewPayments.qml	2014-08-19 14:48:07 +0000
+++ tests/qmltests/Dash/Previews/tst_PreviewPayments.qml	2014-08-29 11:44:04 +0000
@@ -48,19 +48,28 @@
     UT.UnityTestCase {
         name: "PreviewPaymentsTest"
         when: windowShown
+        property var paymentClient
+
+        function init()
+        {
+            paymentClient = findInvisibleChild(previewPayments, "paymentClient");
+            verify(paymentClient, "Could not find the payment client object.");
+        }
 
         function cleanup()
         {
+            paymentClient = null;
+            previewPayments.widgetData = null;
             spy.clear();
-            var button = findChild(root, "paymentButton");
+            var button = findChild(previewPayments, "paymentButton");
             button.opacity = 1;
         }
 
         function test_purchase_text_display() {
             previewPayments.widgetData = jsonPurchase;
 
-            var button = findChild(root, "paymentButton");
-            verify(button != null, "Button not found.");
+            var button = findChild(previewPayments, "paymentButton");
+            verify(button, "Button not found.");
             compare(button.text, "0.99USD");
         }
 
@@ -68,11 +77,12 @@
             // Exercise the purchaseCompleted signal here.
             previewPayments.widgetData = jsonPurchase;
 
-            var button = findChild(root, "paymentButton");
-            verify(button != null, "Button not found.");
+            var button = findChild(previewPayments, "paymentButton");
+            verify(button, "Button not found.");
 
             mouseClick(button, button.width / 2, button.height / 2);
 
+            paymentClient.process();
             spy.wait();
 
             var args = spy.signalArguments[0];
@@ -85,22 +95,51 @@
             // Make sure the progress bar is shown.
             previewPayments.widgetData = jsonPurchase;
 
-            var button = findChild(root, "paymentButton");
-            var progress = findChild(root, "loadingBar");
-
-            tryCompare(progress, "visible", false);
-            tryCompare(progress, "opacity", 0);
-            tryCompare(button, "visible", true);
-            tryCompare(button, "opacity", 1);
-
-            mouseClick(button, button.width / 2, button.height / 2);
-
-            spy.wait();
-
-            tryCompare(progress, "visible", true);
-            tryCompare(progress, "opacity", 1);
-            tryCompare(button, "visible", false);
-            tryCompare(button, "opacity", 0);
+            var button = findChild(previewPayments, "paymentButton");
+            var progress = findChild(previewPayments, "loadingBar");
+
+            tryCompare(progress, "visible", false);
+            tryCompare(progress, "opacity", 0);
+            tryCompare(button, "visible", true);
+            tryCompare(button, "opacity", 1);
+
+            mouseClick(button, button.width / 2, button.height / 2);
+
+            paymentClient.process();
+            spy.wait();
+
+            tryCompare(progress, "visible", true);
+            tryCompare(progress, "opacity", 1);
+            tryCompare(button, "visible", false);
+            tryCompare(button, "opacity", 0);
+        }
+
+        function test_progress_show_cancel() {
+            // Make sure the progress bar is shown.
+            previewPayments.widgetData = jsonPurchaseError;
+
+            var button = findChild(previewPayments, "paymentButton");
+            var progress = findChild(previewPayments, "loadingBar");
+
+            tryCompare(progress, "visible", false);
+            tryCompare(progress, "opacity", 0);
+            tryCompare(button, "visible", true);
+            tryCompare(button, "opacity", 1);
+
+            mouseClick(button, button.width / 2, button.height / 2);
+
+            tryCompare(progress, "visible", true);
+            tryCompare(progress, "opacity", 1);
+            tryCompare(button, "visible", false);
+            tryCompare(button, "opacity", 0);
+
+            paymentClient.process();
+            spy.wait();
+
+            tryCompare(progress, "visible", false);
+            tryCompare(progress, "opacity", 0);
+            tryCompare(button, "visible", true);
+            tryCompare(button, "opacity", 1);
         }
 
         function test_purchase_error() {
@@ -108,11 +147,12 @@
             // passed to it as store_item_id. Exercise it here
             previewPayments.widgetData = jsonPurchaseError;
 
-            var button = findChild(root, "paymentButton");
-            verify(button != null, "Button not found.");
+            var button = findChild(previewPayments, "paymentButton");
+            verify(button, "Button not found.");
 
             mouseClick(button, button.width / 2, button.height / 2);
 
+            paymentClient.process();
             spy.wait();
 
             var args = spy.signalArguments[0];