Unity 8

Merge lp:~dandrader/unity8/fixOrientedShellTests into lp:unity8

  1. fixOrientedShellTests
  2. Merge into trunk
Proposed by Daniel d'Andrada
Status: Merged
Approved by: Josh Arenson
Approved revision: 1833
Merged at revision: 1833
Proposed branch: lp:~dandrader/unity8/fixOrientedShellTests
Merge into: lp:unity8
Diff against target: 329 lines (+31/-49)
10 files modified
qml/OrientedShell.qml (+1/-0)
qml/Shell.qml (+13/-15)
src/ApplicationArguments.h (+5/-0)
src/main.cpp (+2/-1)
tests/autopilot/unity8/greeter/tests/test_args.py (+6/-6)
tests/qmltests/Stages/tst_SurfaceContainer.qml (+0/-1)
tests/qmltests/Tutorial/tst_Tutorial.qml (+3/-10)
tests/qmltests/tst_OrientedShell.qml (+1/-12)
tests/qmltests/tst_Shell.qml (+0/-2)
tests/qmltests/tst_ShellWithPin.qml (+0/-2)
To merge this branch: bzr merge lp:~dandrader/unity8/fixOrientedShellTests
Reviewer Review Type Date Requested Status
Lukáš Tinkl (community) Approve
Josh Arenson Approve
PS Jenkins bot (community) continuous-integration Needs Fixing
Albert Astals Cid (community) Abstain
Review via email: mp+262490@code.launchpad.net

Commit message

Fix tst_OrientedShell and tst_Tutorial

+ Clear several warnings in Shell.qml
+ Move shellMode into applicationArguments context property

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

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
Not applicable

* If you changed the UI, has there been a design review?
Not applicable

* Did you have a look at the warnings when running tests? Can they be reduced?
Yes, solved some of them.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1823
http://jenkins.qa.ubuntu.com/job/unity8-ci/5839/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-wily-touch/133
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-amd64-ci/119
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-i386-ci/120
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-wily-mako/86
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/133
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/133/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21335

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

review: Needs Fixing (continuous-integration)
lp:~dandrader/unity8/fixOrientedShellTests updated
1830. By CI Train Bot Account

Resync trunk.

1831. By Launchpad Translations on behalf of unity-team

Launchpad automatic translations update.

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

Text conflict in tests/qmltests/tst_Shell.qml
1 conflicts encountered.

lp:~dandrader/unity8/fixOrientedShellTests updated
1832. By Daniel d'Andrada

Fix tst_OrientedShell

+ Clear several warnings in Shell.qml
+ Move shellMode into applicationArguments context property

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

> Text conflict in tests/qmltests/tst_Shell.qml
> 1 conflicts encountered.

Fixed.

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

FAILED: Continuous integration, rev:1832
http://jenkins.qa.ubuntu.com/job/unity8-ci/5855/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-wily-touch/155
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-amd64-ci/135
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-i386-ci/136
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-wily-mako/105
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/155
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/155/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21397

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

review: Needs Fixing (continuous-integration)
lp:~dandrader/unity8/fixOrientedShellTests updated
1833. By Daniel d'Andrada

Fix tst_Tutorial.qml

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

I do sincerely think the fix for tutorial test is worse than the one in http://bazaar.launchpad.net/~aacid/unity8/fix_test_tutorial/revision/1833 but a fix is better than none so let's have this one.

Revision history for this message
Albert Astals Cid (aacid) :
review: Abstain
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1833
http://jenkins.qa.ubuntu.com/job/unity8-ci/5862/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-wily-touch/164/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-amd64-ci/142
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-i386-ci/143
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-wily-mako/112/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/164
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/164/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21407

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1833
http://jenkins.qa.ubuntu.com/job/unity8-ci/5871/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-wily-touch/176/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-amd64-ci/151
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-i386-ci/152
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-wily-mako/118/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/176
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/176/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21425

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Josh Arenson (josharenson) wrote :

Comments inline.

review: Needs Information
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

On 23/06/15 20:44, Josh Arenson wrote:
>> @@ -434,7 +432,7 @@
>> > Greeter {
>> >
>> > hides: [launcher, panel.indicators]
>> > - tabletMode: shell.sideStageEnabled
>> > + tabletMode: shell.usageScenario != "phone"
> Do you think naming this property 'phoneMode' would make things more clear? The only place 'tabletMode' is being used is in the greeter, so the amount of refactoring is small. I think the way it is currently is confusing.
>

I think it amounts to the same. Don't see it being any better.

Revision history for this message
Josh Arenson (josharenson) wrote :

lgtm.

review: Approve
lp:~dandrader/unity8/fixOrientedShellTests updated
1834. By Daniel d'Andrada

Also fix tst_SurfaceContainer

Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

One minor inline nitpick from me :)

review: Needs Fixing (code-review)
Revision history for this message
Daniel d'Andrada (dandrader) :
Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

See http://stackoverflow.com/questions/2582797/why-pass-by-const-reference-instead-of-by-value for the more elaborate overview of passing by value vs. passing by const ref

review: Needs Fixing (code-review)
Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

Also, https://techbase.kde.org/Development/Tutorials/Common_Programming_Mistakes#Passing_non-POD_types has some more info on the topic

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

> See http://stackoverflow.com/questions/2582797/why-pass-by-const-reference-
> instead-of-by-value for the more elaborate overview of passing by value vs.
> passing by const ref

QString does have a move contructor:
http://pastebin.ubuntu.com/11795003/

What problem do you see in this case?

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

> Also,
> https://techbase.kde.org/Development/Tutorials/Common_Programming_Mistakes
> #Passing_non-POD_types has some more info on the topic

This applies if you're not using C++11, where you don't have move constructors and move semantics.

Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

Ok, I stand corrected then, I didn't know Qt enables it for QStrings under C++11 :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'qml/OrientedShell.qml'
2--- qml/OrientedShell.qml 2015-06-12 16:07:43 +0000
3+++ qml/OrientedShell.qml 2015-06-25 19:25:08 +0000
4@@ -135,6 +135,7 @@
5 nativeOrientation: root.nativeOrientation
6 nativeWidth: root.width
7 nativeHeight: root.height
8+ mode: applicationArguments.mode
9
10 // TODO: Factor in the connected input devices (eg: physical keyboard, mouse, touchscreen),
11 // what's the output device (eg: big TV, desktop monitor, phone display), etc.
12
13=== modified file 'qml/Shell.qml'
14--- qml/Shell.qml 2015-06-15 22:40:10 +0000
15+++ qml/Shell.qml 2015-06-25 19:25:08 +0000
16@@ -55,6 +55,7 @@
17 property alias indicatorAreaShowProgress: panel.indicatorAreaShowProgress
18 property bool beingResized
19 property string usageScenario: "phone" // supported values: "phone", "tablet" or "desktop"
20+ property string mode: "full-greeter"
21 function updateFocusedAppOrientation() {
22 applicationsDisplayLoader.item.updateFocusedAppOrientation();
23 }
24@@ -68,9 +69,9 @@
25
26 readonly property bool orientationChangesEnabled: panel.indicators.fullyClosed
27 && (applicationsDisplayLoader.item && applicationsDisplayLoader.item.orientationChangesEnabled)
28- && !greeter.animating
29+ && (!greeter || !greeter.animating)
30
31- readonly property bool showingGreeter: greeter.shown
32+ readonly property bool showingGreeter: greeter && greeter.shown
33
34 property bool startingUp: true
35 Timer { id: finishStartUpTimer; interval: 500; onTriggered: startingUp = false }
36@@ -79,7 +80,7 @@
37 if (startingUp) {
38 // Ensure we don't rotate during start up
39 return Qt.PrimaryOrientation;
40- } else if (greeter.shown) {
41+ } else if (greeter && greeter.shown) {
42 return Qt.PrimaryOrientation;
43 } else if (mainApp) {
44 return mainApp.supportedOrientations;
45@@ -101,16 +102,13 @@
46
47 // Disable everything while greeter is waiting, so that the user can't swipe
48 // the greeter or launcher until we know whether the session is locked.
49- enabled: !greeter.waiting
50+ enabled: greeter && !greeter.waiting
51
52 property real edgeSize: units.gu(2)
53 property url defaultBackground: Qt.resolvedUrl(shell.width >= units.gu(60) ? "graphics/tablet_background.jpg" : "graphics/phone_background.jpg")
54 property url background: asImageTester.status == Image.Ready ? asImageTester.source
55 : gsImageTester.status == Image.Ready ? gsImageTester.source : defaultBackground
56
57- // This is _only_ used to expose the property to autopilot tests
58- readonly property string testShellMode: shellMode
59-
60 readonly property alias greeter: greeterLoader.item
61
62 function activateApplication(appId) {
63@@ -264,7 +262,7 @@
64 ? "phone"
65 : shell.usageScenario
66 source: {
67- if(shellMode === "greeter") {
68+ if(shell.mode === "greeter") {
69 return "Stages/ShimStage.qml"
70 } else if (applicationsDisplayLoader.usageScenario === "phone") {
71 return "Stages/PhoneStage.qml";
72@@ -276,7 +274,7 @@
73 }
74
75 property bool interactive: tutorial.spreadEnabled
76- && !greeter.shown
77+ && (!greeter || !greeter.shown)
78 && panel.indicators.fullyClosed
79 && launcher.progress == 0
80 && !notifications.useModal
81@@ -307,12 +305,12 @@
82 Binding {
83 target: applicationsDisplayLoader.item
84 property: "spreadEnabled"
85- value: tutorial.spreadEnabled && !greeter.hasLockedApp
86+ value: tutorial.spreadEnabled && (!greeter || !greeter.hasLockedApp)
87 }
88 Binding {
89 target: applicationsDisplayLoader.item
90 property: "inverseProgress"
91- value: greeter.locked ? 0 : launcher.progress
92+ value: greeter && greeter.locked ? 0 : launcher.progress
93 }
94 Binding {
95 target: applicationsDisplayLoader.item
96@@ -422,7 +420,7 @@
97 id: greeterLoader
98 anchors.fill: parent
99 anchors.topMargin: panel.panelHeight
100- sourceComponent: shellMode != "shell" ? integratedGreeter :
101+ sourceComponent: shell.mode != "shell" ? integratedGreeter :
102 Qt.createComponent(Qt.resolvedUrl("Greeter/ShimGreeter.qml"));
103 onLoaded: {
104 item.objectName = "greeter"
105@@ -434,7 +432,7 @@
106 Greeter {
107
108 hides: [launcher, panel.indicators]
109- tabletMode: shell.sideStageEnabled
110+ tabletMode: shell.usageScenario != "phone"
111 launcherOffset: launcher.progress
112 forcedUnlock: tutorial.running
113 background: shell.background
114@@ -550,8 +548,8 @@
115 indicators {
116 hides: [launcher]
117 available: tutorial.panelEnabled
118- && (!greeter.locked || AccountsService.enableIndicatorsWhileLocked)
119- && !greeter.hasLockedApp
120+ && ((!greeter || !greeter.locked) || AccountsService.enableIndicatorsWhileLocked)
121+ && (!greeter || !greeter.hasLockedApp)
122 contentEnabled: tutorial.panelContentEnabled
123 width: parent.width > units.gu(60) ? units.gu(40) : parent.width
124
125
126=== modified file 'src/ApplicationArguments.h'
127--- src/ApplicationArguments.h 2015-06-12 16:07:43 +0000
128+++ src/ApplicationArguments.h 2015-06-25 19:25:08 +0000
129@@ -27,14 +27,19 @@
130 {
131 Q_OBJECT
132 Q_PROPERTY(QString deviceName READ deviceName CONSTANT)
133+ Q_PROPERTY(QString mode READ mode CONSTANT)
134 public:
135 ApplicationArguments(QObject *parent = nullptr);
136
137 void setDeviceName(QString deviceName) { m_deviceName = deviceName; }
138 QString deviceName() const { return m_deviceName; }
139
140+ void setMode(QString mode) { m_mode = mode; }
141+ QString mode() const { return m_mode; }
142+
143 private:
144 QString m_deviceName;
145+ QString m_mode;
146 };
147
148 #endif // APPLICATION_ARGUMENTS_H
149
150=== modified file 'src/main.cpp'
151--- src/main.cpp 2015-06-12 16:07:43 +0000
152+++ src/main.cpp 2015-06-25 19:25:08 +0000
153@@ -60,6 +60,8 @@
154 qmlArgs.setDeviceName(QString(buffer));
155 }
156
157+ qmlArgs.setMode(parser.mode());
158+
159 // The testability driver is only loaded by QApplication but not by QGuiApplication.
160 // However, QApplication depends on QWidget which would add some unneeded overhead => Let's load the testability driver on our own.
161 if (parser.hasTestability() || getenv("QT_LOAD_TESTABILITY")) {
162@@ -92,7 +94,6 @@
163
164 view->engine()->setBaseUrl(QUrl::fromLocalFile(::qmlDirectory()));
165 view->rootContext()->setContextProperty("applicationArguments", &qmlArgs);
166- view->rootContext()->setContextProperty("shellMode", parser.mode());
167 if (parser.hasFrameless()) {
168 view->setFlags(Qt::FramelessWindowHint);
169 }
170
171=== modified file 'tests/autopilot/unity8/greeter/tests/test_args.py'
172--- tests/autopilot/unity8/greeter/tests/test_args.py 2015-04-16 14:48:50 +0000
173+++ tests/autopilot/unity8/greeter/tests/test_args.py 2015-06-25 19:25:08 +0000
174@@ -28,27 +28,27 @@
175 def test_full_greeter_mode(self):
176 unity_proxy = self.launch_unity(mode='full-greeter')
177 shell = self.get_shell(unity_proxy)
178- self.assertTrue(shell.testShellMode == 'full-greeter')
179+ self.assertTrue(shell.mode == 'full-greeter')
180
181 def test_full_shell_mode(self):
182 unity_proxy = self.launch_unity(mode='full-shell')
183 shell = self.get_shell(unity_proxy)
184- self.assertTrue(shell.testShellMode == 'full-shell')
185+ self.assertTrue(shell.mode == 'full-shell')
186
187 def test_greeter_mode(self):
188 unity_proxy = self.launch_unity(mode='greeter')
189 shell = self.get_shell(unity_proxy)
190- self.assertTrue(shell.testShellMode == 'greeter')
191+ self.assertTrue(shell.mode == 'greeter')
192
193 def test_nonexistent_mode(self):
194 unity_proxy = self.launch_unity(mode=self.NONEXISTENT_MODE)
195 shell = self.get_shell(unity_proxy)
196- self.assertTrue(shell.testShellMode == self.DEFAULT_SHELL_MODE,
197+ self.assertTrue(shell.mode == self.DEFAULT_SHELL_MODE,
198 "Shell mode was {} but should have been {}"
199- .format(shell.testShellMode,
200+ .format(shell.mode,
201 self.DEFAULT_SHELL_MODE))
202
203 def test_shell_mode(self):
204 unity_proxy = self.launch_unity(mode='shell')
205 shell = self.get_shell(unity_proxy)
206- self.assertTrue(shell.testShellMode == 'shell')
207+ self.assertTrue(shell.mode == 'shell')
208
209=== modified file 'tests/qmltests/Stages/tst_SurfaceContainer.qml'
210--- tests/qmltests/Stages/tst_SurfaceContainer.qml 2015-03-12 14:45:44 +0000
211+++ tests/qmltests/Stages/tst_SurfaceContainer.qml 2015-06-25 19:25:08 +0000
212@@ -32,7 +32,6 @@
213 id: surfaceContainerComponent
214 SurfaceContainer {
215 anchors.fill: parent
216- orientation: Qt.PortraitOrientation
217 interactive: interactiveCheckbox.checked
218 }
219 }
220
221=== modified file 'tests/qmltests/Tutorial/tst_Tutorial.qml'
222--- tests/qmltests/Tutorial/tst_Tutorial.qml 2015-06-15 22:40:10 +0000
223+++ tests/qmltests/Tutorial/tst_Tutorial.qml 2015-06-25 19:25:08 +0000
224@@ -17,7 +17,6 @@
225 import QtQuick 2.0
226 import QtTest 1.0
227 import AccountsService 0.1
228-import GSettings 1.0
229 import LightDM 0.1 as LightDM
230 import Ubuntu.Components 1.1
231 import Unity.Application 0.1
232@@ -46,11 +45,6 @@
233 }
234 }
235
236- GSettings {
237- id: unity8Settings
238- schema.id: "com.canonical.Unity8"
239- }
240-
241 Component.onCompleted: {
242 // must set the mock mode before loading the Shell
243 LightDM.Greeter.mockMode = "single-pin";
244@@ -73,7 +67,6 @@
245 sourceComponent: Component {
246 Shell {
247 property string indicatorProfile: "phone"
248- property string shellMode: "full-greeter" /* default */
249
250 Component.onDestruction: {
251 shellLoader.itemDestroyed = true;
252@@ -119,7 +112,7 @@
253
254 function init() {
255 tryCompare(shell, "enabled", true); // enabled by greeter when ready
256- unity8Settings.usageMode = "Staged";
257+
258 AccountsService.demoEdges = false;
259 AccountsService.demoEdges = true;
260 swipeAwayGreeter();
261@@ -193,7 +186,7 @@
262 }
263
264 function checkRightEdge() {
265- if (unity8Settings.usageMode === "Staged") {
266+ if (shell.usageScenario === "phone") {
267 touchFlick(shell, shell.width, halfHeight, halfWidth, halfHeight);
268
269 var stage = findChild(shell, "stage");
270@@ -271,7 +264,7 @@
271 }
272
273 function test_walkthroughOnDesktop() {
274- unity8Settings.usageMode = "Windowed";
275+ shell.usageScenario = "desktop";
276 var page = goToPage("tutorialLeftFinish");
277 var tick = findChild(page, "tick");
278 tap(tick);
279
280=== modified file 'tests/qmltests/tst_OrientedShell.qml'
281--- tests/qmltests/tst_OrientedShell.qml 2015-05-11 14:36:03 +0000
282+++ tests/qmltests/tst_OrientedShell.qml 2015-06-25 19:25:08 +0000
283@@ -34,19 +34,8 @@
284
285 QtObject {
286 id: applicationArguments
287-
288- function hasGeometry() {
289- return false;
290- }
291-
292- function width() {
293- return 0;
294- }
295-
296- function height() {
297- return 0;
298- }
299 property string deviceName: "mako"
300+ property string mode: "full-greeter"
301 }
302
303 QtObject {
304
305=== modified file 'tests/qmltests/tst_Shell.qml'
306--- tests/qmltests/tst_Shell.qml 2015-06-18 18:17:09 +0000
307+++ tests/qmltests/tst_Shell.qml 2015-06-25 19:25:08 +0000
308@@ -81,8 +81,6 @@
309 property bool itemDestroyed: false
310 sourceComponent: Component {
311 Shell {
312- property string shellMode: "full-greeter" /* default */
313-
314 usageScenario: usageScenarioSelector.model[usageScenarioSelector.selectedIndex]
315 orientation: Qt.PortraitOrientation
316 primaryOrientation: Qt.PortraitOrientation
317
318=== modified file 'tests/qmltests/tst_ShellWithPin.qml'
319--- tests/qmltests/tst_ShellWithPin.qml 2015-06-15 22:40:10 +0000
320+++ tests/qmltests/tst_ShellWithPin.qml 2015-06-25 19:25:08 +0000
321@@ -69,8 +69,6 @@
322 property bool itemDestroyed: false
323 sourceComponent: Component {
324 Shell {
325- property string shellMode: "full-greeter" /* default */
326-
327 Component.onDestruction: {
328 shellLoader.itemDestroyed = true
329 }

Subscribers

People subscribed via source and target branches