Unity 8

Merge lp:~cimi/unity8/fix-1309135 into lp:unity8

  1. fix-1309135
  2. Merge into trunk
Proposed by Andrea Cimitan
Status: Merged
Approved by: Albert Astals Cid
Approved revision: 856
Merged at revision: 865
Proposed branch: lp:~cimi/unity8/fix-1309135
Merge into: lp:unity8
Diff against target: 69 lines (+23/-4)
2 files modified
qml/Dash/Previews/PreviewRatingInput.qml (+2/-2)
tests/qmltests/Dash/Previews/tst_PreviewRatingInput.qml (+21/-2)
To merge this branch: bzr merge lp:~cimi/unity8/fix-1309135
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Albert Astals Cid (community) Approve
Review via email: mp+216707@code.launchpad.net

Commit message

Fix 1309135

Description of the change

 * Are there any related MPs required for this MP to build/function as expected? Please list.
No
 * Did you perform an exploratory manual test run of your code change and any related functionality?
Yes
 * Did you make sure that your branch does not contain spurious tags?
Yes

To post a comment you must log in.
lp:~cimi/unity8/fix-1309135 updated
856. By Andrea Cimitan

Coding style

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:856
http://jenkins.qa.ubuntu.com/job/unity8-ci/2855/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-trusty-touch/342
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/1719/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/1376
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/1380
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/1380/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/1376
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/314
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4567
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4567/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6247

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Albert Astals Cid (aacid) wrote :

* Did you perform an exploratory manual test run of the code change and any related functionality?
Yes

* Did CI run pass? If not, please explain why.
No, somehow jenkins didn't run, i've run the tests locally and pass, i've retriggered the jenkins build, will wait for it to finish to top-approve

review: Approve
Revision history for this message
Albert Astals Cid (aacid) wrote :

Yep, back to the known unstable in qmluitests now

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:856
http://jenkins.qa.ubuntu.com/job/unity8-ci/2864/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-trusty-touch/399
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/1728
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/1385
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/1389
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/1389/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/1385
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/363
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4642
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4642/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6357

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

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'qml/Dash/Previews/PreviewRatingInput.qml'
2--- qml/Dash/Previews/PreviewRatingInput.qml 2014-04-08 17:09:35 +0000
3+++ qml/Dash/Previews/PreviewRatingInput.qml 2014-04-22 12:51:20 +0000
4@@ -155,8 +155,8 @@
5 objectName: "submitButton"
6
7 readonly property bool readyToSubmit: {
8- if (reviewTextArea.text === "" ||
9- (widgetData["visible"] === "both" && rating.value < 0)) return false;
10+ if ((widgetData["required"] !== "review" && rating.value < 0) ||
11+ (widgetData["required"] !== "rating" && reviewTextArea.text === "")) return false;
12 else return true;
13 }
14
15
16=== modified file 'tests/qmltests/Dash/Previews/tst_PreviewRatingInput.qml'
17--- tests/qmltests/Dash/Previews/tst_PreviewRatingInput.qml 2014-03-18 08:50:05 +0000
18+++ tests/qmltests/Dash/Previews/tst_PreviewRatingInput.qml 2014-04-22 12:51:20 +0000
19@@ -36,6 +36,8 @@
20 property var triggeredDataReviewOnly: { "rating": invalidInputRating, "review": validInputText, "author": null }
21
22 property var widgetDataBoth: { "visible": "both", "required": "both" }
23+ property var widgetDataBothRequireRating: { "visible": "both", "required": "rating" }
24+ property var widgetDataBothRequireReview: { "visible": "both", "required": "review" }
25 property var widgetDataRating: { "visible": "rating", "required": "rating" }
26 property var widgetDataRatingBroken: { "visible": "rating", "required": "review" }
27 property var widgetDataReview: { "visible": "review", "required": "review" }
28@@ -85,6 +87,14 @@
29 {tag: "both, rating-only review", widgetData: widgetDataBoth, inputRating: validInputRating, inputText: invalidInputText, emitted: false},
30 {tag: "both, review-only review", widgetData: widgetDataBoth, inputRating: invalidInputRating, inputText: validInputText, emitted: false},
31 {tag: "both, complete review", widgetData: widgetDataBoth, inputRating: validInputRating, inputText: validInputText, emitted: true},
32+ {tag: "both require rating, null review", widgetData: widgetDataBothRequireRating, inputRating: invalidInputRating, inputText: invalidInputText, emitted: false},
33+ {tag: "both require rating, rating-only review", widgetData: widgetDataBothRequireRating, inputRating: validInputRating, inputText: invalidInputText, emitted: true},
34+ {tag: "both require rating, review-only review", widgetData: widgetDataBothRequireRating, inputRating: invalidInputRating, inputText: validInputText, emitted: false},
35+ {tag: "both require rating, complete review", widgetData: widgetDataBothRequireRating, inputRating: validInputRating, inputText: validInputText, emitted: true},
36+ {tag: "both require review, null review", widgetData: widgetDataBothRequireReview, inputRating: invalidInputRating, inputText: invalidInputText, emitted: false},
37+ {tag: "both require review, rating-only review", widgetData: widgetDataBothRequireReview, inputRating: validInputRating, inputText: invalidInputText, emitted: false},
38+ {tag: "both require review, review-only review", widgetData: widgetDataBothRequireReview, inputRating: invalidInputRating, inputText: validInputText, emitted: true},
39+ {tag: "both require review, complete review", widgetData: widgetDataBothRequireReview, inputRating: validInputRating, inputText: validInputText, emitted: true},
40 {tag: "rating, null review", widgetData: widgetDataRating, inputRating: invalidInputRating, inputText: invalidInputText, emitted: false},
41 {tag: "rating, rating-only review", widgetData: widgetDataRating, inputRating: validInputRating, inputText: invalidInputText, emitted: true},
42 {tag: "rating, review-only review", widgetData: widgetDataRating, inputRating: invalidInputRating, inputText: validInputText, emitted: false},
43@@ -114,7 +124,9 @@
44 verify(rating.visible === true);
45
46 rating.value = data.inputRating;
47- if (data.widgetData["required"] !== "rating" || data.inputRating < 0) {
48+ if (data.widgetData["required"] !== "rating" ||
49+ data.widgetData["visible"] !== "rating" ||
50+ data.inputRating < 0) {
51 compare(spy.count, 0);
52 } else {
53 compare(spy.count, 1);
54@@ -129,7 +141,14 @@
55 reviewTextArea.text = data.inputText;
56 mouseClick(submitButton, submitButton.width / 2, submitButton.height / 2);
57 switch (data.widgetData["required"]) {
58- case "rating": compare(spy.count, 0); break;
59+ case "rating": {
60+ if (rating.visible === false || data.inputRating < 0) {
61+ compare(spy.count, 0);
62+ } else {
63+ compare(spy.count, 1);
64+ }
65+ break;
66+ }
67 case "both":
68 default: {
69 if (data.inputRating < 0 || data.inputText === "") {

Subscribers

People subscribed via source and target branches