ubuntu-system-settings

Merge lp:~diegosarmentero/ubuntu-system-settings/duplicate-and-credentials into lp:ubuntu-system-settings

  1. duplicate-and-credentials
  2. Merge into trunk
Proposed by Diego Sarmentero
Status: Merged
Approved by: Sebastien Bacher
Approved revision: 690
Merged at revision: 692
Proposed branch: lp:~diegosarmentero/ubuntu-system-settings/duplicate-and-credentials
Merge into: lp:ubuntu-system-settings
Diff against target: 277 lines (+53/-15)
9 files modified
plugins/system-update/PageComponent.qml (+1/-2)
plugins/system-update/system_update.cpp (+1/-1)
plugins/system-update/system_update.h (+2/-0)
plugins/system-update/update_manager.cpp (+25/-7)
plugins/system-update/update_manager.h (+4/-0)
tests/plugins/system-update/fakessoservice.cpp (+8/-3)
tests/plugins/system-update/fakessoservice.h (+4/-0)
tests/plugins/system-update/fakesystemupdate.h (+2/-0)
tests/plugins/system-update/tst_updatemanager.cpp (+6/-2)
To merge this branch: bzr merge lp:~diegosarmentero/ubuntu-system-settings/duplicate-and-credentials
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Needs Fixing
Sebastien Bacher (community) Approve
Review via email: mp+218414@code.launchpad.net

Commit message

- Avoid duplicate result of Image upadte
- Don't ask for credentials on Image update

Description of the change

Some tests has also been updated according the new code.

To post a comment you must log in.
Revision history for this message
Sebastien Bacher (seb128) wrote :

Thanks for the work, the package fails to build with those changes for me though

"FAIL! : UpdateManagerTest::testCheckUpdatesModelSignal() Compared values are not the same
   Actual (manager.get_apps().size()): 0
   Expected (4) : 4
   Loc: [/tmp/build/ubuntu-system-settings-0.1+14.10.20140428/tests/plugins/system-update/tst_updatemanager.cpp(131)]
FAIL! : UpdateManagerTest::testCheckUpdatesUpdateSignal() Compared values are not the same
   Actual (manager.get_apps().size()): 0
   Expected (4) : 4
   Loc: [/tmp/build/ubuntu-system-settings-0.1+14.10.20140428/tests/plugins/system-update/tst_updatemanager.cpp(145)]

Totals: 6 passed, 2 failed, 0 skipped

(that's building on an i386 configuration)

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

FAILED: Continuous integration, rev:689
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/812/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/21
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/15
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-amd64-ci/1
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-armhf-ci/1
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-i386-ci/1/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/532
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/99
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/99/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6817
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/16
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/31
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/31/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/812/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Diego Sarmentero (diegosarmentero) wrote :

> Thanks for the work, the package fails to build with those changes for me
> though
>
> "FAIL! : UpdateManagerTest::testCheckUpdatesModelSignal() Compared values are
> not the same
> Actual (manager.get_apps().size()): 0
> Expected (4) : 4
> Loc: [/tmp/build/ubuntu-system-settings-0.1+14.10.20140428/tests/plugins
> /system-update/tst_updatemanager.cpp(131)]
> FAIL! : UpdateManagerTest::testCheckUpdatesUpdateSignal() Compared values are
> not the same
> Actual (manager.get_apps().size()): 0
> Expected (4) : 4
> Loc: [/tmp/build/ubuntu-system-settings-0.1+14.10.20140428/tests/plugins
> /system-update/tst_updatemanager.cpp(145)]
>
> Totals: 6 passed, 2 failed, 0 skipped
>
> (that's building on an i386 configuration)

Weird, it's building for me in the phone, i'll check what's going on

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

FAILED: Continuous integration, rev:690
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/819/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/59
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/52
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-amd64-ci/8
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-armhf-ci/8
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-i386-ci/8/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/566
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/162
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/162/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6881
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/52
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/79
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/79/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/819/rebuild

review: Needs Fixing (continuous-integration)
lp:~diegosarmentero/ubuntu-system-settings/duplicate-and-credentials updated
691. By Diego Sarmentero

initialize m_validCredentials

Revision history for this message
Sebastien Bacher (seb128) wrote :

Looks good now, thanks

review: Approve
Revision history for this message
Sebastien Bacher (seb128) wrote :

(the autopilot tests fail here, but that's fixed by https://code.launchpad.net/~diegosarmentero/ubuntu-system-settings/ignore-updates-autopilot/+merge/218481 which is going to be included in the same landing)

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

FAILED: Continuous integration, rev:691
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/822/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/71
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/62
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-amd64-ci/11
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-armhf-ci/11
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-i386-ci/11
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/579
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/177
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/177/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6899
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/60
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/89
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/89/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/822/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 'plugins/system-update/PageComponent.qml'
2--- plugins/system-update/PageComponent.qml 2014-04-11 16:34:57 +0000
3+++ plugins/system-update/PageComponent.qml 2014-05-07 15:38:11 +0000
4@@ -101,20 +101,19 @@
5 },
6 State {
7 name: "UPDATE"
8- PropertyChanges { target: notification; visible: false}
9 PropertyChanges { target: updateList; visible: true}
10 PropertyChanges { target: installAllButton; visible: true}
11 PropertyChanges { target: updateNotification; visible: false}
12 },
13 State {
14 name: "NOCREDENTIALS"
15+ PropertyChanges { target: updateList; visible: true}
16 PropertyChanges { target: notification; text: i18n.tr("Please log into your Ubuntu One account.")}
17 PropertyChanges { target: notification; onClicked: root.open_online_accounts() }
18 PropertyChanges { target: notification; progression: true}
19 PropertyChanges { target: notification; visible: true}
20 PropertyChanges { target: updateNotification; text: i18n.tr("Credentials not found")}
21 PropertyChanges { target: updateNotification; visible: true}
22- PropertyChanges { target: installAllButton; visible: false}
23 }
24 ]
25
26
27=== modified file 'plugins/system-update/system_update.cpp'
28--- plugins/system-update/system_update.cpp 2014-02-25 20:27:51 +0000
29+++ plugins/system-update/system_update.cpp 2014-05-07 15:38:11 +0000
30@@ -151,7 +151,7 @@
31 QString errorReason)
32 {
33 update = new Update(this);
34- QString packageName("UbuntuImage");
35+ QString packageName(UBUNTU_PACKAGE_NAME);
36 update->initializeApplication(packageName, "Ubuntu",
37 QString::number(this->currentBuildNumber()));
38
39
40=== modified file 'plugins/system-update/system_update.h'
41--- plugins/system-update/system_update.h 2014-02-25 20:27:51 +0000
42+++ plugins/system-update/system_update.h 2014-05-07 15:38:11 +0000
43@@ -30,6 +30,8 @@
44
45 #include "update.h"
46
47+#define UBUNTU_PACKAGE_NAME "UbuntuImage"
48+
49 namespace UpdatePlugin {
50
51 class SystemUpdate : public QObject
52
53=== modified file 'plugins/system-update/update_manager.cpp'
54--- plugins/system-update/update_manager.cpp 2014-04-11 16:27:17 +0000
55+++ plugins/system-update/update_manager.cpp 2014-05-07 15:38:11 +0000
56@@ -28,6 +28,11 @@
57 #include <QProcessEnvironment>
58
59 #define CLICK_COMMAND "click"
60+#ifdef TESTS
61+ #define CHECK_CREDENTIALS "IGNORE_CREDENTIALS"
62+#else
63+ #define CHECK_CREDENTIALS "CHECK_CREDENTIALS"
64+#endif
65
66 namespace UpdatePlugin {
67
68@@ -118,13 +123,17 @@
69 m_model.clear();
70 m_apps.clear();
71 Q_EMIT modelChanged();
72+ bool enabled = enableAutopilotMode();
73 if (getCheckForCredentials()) {
74 m_systemUpdate.checkForUpdate();
75 m_service.getCredentials();
76- } else {
77+ } else if (enabled) {
78 systemUpdateNotAvailable();
79 Token token("", "", "", "");
80 handleCredentialsFound(token);
81+ } else {
82+ systemUpdateNotAvailable();
83+ clickUpdateNotAvailable();
84 }
85 }
86
87@@ -147,10 +156,17 @@
88 bool UpdateManager::getCheckForCredentials()
89 {
90 QProcessEnvironment environment = QProcessEnvironment::systemEnvironment();
91- QString value = environment.value("IGNORE_CREDENTIALS", QString("CHECK_CREDENTIALS"));
92+ QString value = environment.value("IGNORE_CREDENTIALS", QString(CHECK_CREDENTIALS));
93 return value == "CHECK_CREDENTIALS";
94 }
95
96+bool UpdateManager::enableAutopilotMode()
97+{
98+ QProcessEnvironment environment = QProcessEnvironment::systemEnvironment();
99+ QString value = environment.value("AUTOPILOT_ENABLED", QString("AUTOPILOT_DISABLED"));
100+ return value == "AUTOPILOT_ENABLED";
101+}
102+
103 void UpdateManager::processOutput()
104 {
105 QString output(m_process.readAllStandardOutput());
106@@ -179,7 +195,8 @@
107 bool updateAvailable = false;
108 foreach (QString id, m_apps.keys()) {
109 Update *app = m_apps.value(id);
110- if(app->updateRequired()) {
111+ QString packagename(UBUNTU_PACKAGE_NAME);
112+ if(app->getPackageName() != packagename && app->updateRequired()) {
113 updateAvailable = true;
114 m_model.append(QVariant::fromValue(app));
115 }
116@@ -194,20 +211,21 @@
117
118 void UpdateManager::registerSystemUpdate(const QString& packageName, Update *update)
119 {
120- m_systemCheckingUpdate = false;
121- if (!m_apps.contains(packageName)) {
122+ QString packagename(UBUNTU_PACKAGE_NAME);
123+ if (!m_apps.contains(packagename)) {
124 m_apps[packageName] = update;
125 m_model.insert(0, QVariant::fromValue(update));
126 Q_EMIT modelChanged();
127+ Q_EMIT updateAvailableFound(update->updateState());
128 }
129- Q_EMIT updateAvailableFound(update->updateState());
130+ m_systemCheckingUpdate = false;
131
132 reportCheckState();
133 }
134
135 void UpdateManager::systemUpdatePaused(int value)
136 {
137- QString packagename("UbuntuImage");
138+ QString packagename(UBUNTU_PACKAGE_NAME);
139 if (m_apps.contains(packagename)) {
140 Update *update = m_apps[packagename];
141 update->setSelected(true);
142
143=== modified file 'plugins/system-update/update_manager.h'
144--- plugins/system-update/update_manager.h 2014-04-11 16:27:17 +0000
145+++ plugins/system-update/update_manager.h 2014-05-07 15:38:11 +0000
146@@ -90,6 +90,9 @@
147 void set_token(Token& t) { m_token = t; }
148 Token get_token() { return m_token; }
149 void setCheckintUpdates(int value) { m_checkingUpdates = value; }
150+ void setCheckSystemUpdates(int value) { m_systemCheckingUpdate = value; }
151+ void setCheckClickUpdates(int value) { m_clickCheckingUpdate = value; }
152+ FakeSsoService& getService() { return m_service; }
153 #endif
154
155 public Q_SLOTS:
156@@ -129,6 +132,7 @@
157 void checkForUpdates();
158 QString getClickCommand();
159 bool getCheckForCredentials();
160+ bool enableAutopilotMode();
161 void reportCheckState();
162 void updateNotAvailable();
163 };
164
165=== modified file 'tests/plugins/system-update/fakessoservice.cpp'
166--- tests/plugins/system-update/fakessoservice.cpp 2014-02-26 11:26:01 +0000
167+++ tests/plugins/system-update/fakessoservice.cpp 2014-05-07 15:38:11 +0000
168@@ -3,14 +3,19 @@
169 namespace UpdatePlugin {
170
171 FakeSsoService::FakeSsoService(QObject *parent) :
172- QObject(parent)
173+ QObject(parent),
174+ m_validCredentials(true)
175 {
176 }
177
178 void FakeSsoService::getCredentials()
179 {
180- Token token("token_key", "token_secret", "consumer_key", "consumer_secret");
181- emit this->credentialsFound(token);
182+ if(m_validCredentials) {
183+ Token token("token_key", "token_secret", "consumer_key", "consumer_secret");
184+ emit this->credentialsFound(token);
185+ } else {
186+ emit this->credentialsNotFound();
187+ }
188 }
189
190 }
191
192=== modified file 'tests/plugins/system-update/fakessoservice.h'
193--- tests/plugins/system-update/fakessoservice.h 2014-02-26 11:26:01 +0000
194+++ tests/plugins/system-update/fakessoservice.h 2014-05-07 15:38:11 +0000
195@@ -15,11 +15,15 @@
196 explicit FakeSsoService(QObject *parent = 0);
197
198 void getCredentials();
199+
200+ void setValidCredentials(bool value) { m_validCredentials = value; }
201
202 signals:
203 void credentialsFound(const Token&);
204 void credentialsNotFound();
205
206+private:
207+ bool m_validCredentials;
208 };
209
210 }
211
212=== modified file 'tests/plugins/system-update/fakesystemupdate.h'
213--- tests/plugins/system-update/fakesystemupdate.h 2014-02-26 11:26:01 +0000
214+++ tests/plugins/system-update/fakesystemupdate.h 2014-05-07 15:38:11 +0000
215@@ -22,6 +22,8 @@
216 #include <QObject>
217 #include "../../../plugins/system-update/update.h"
218
219+#define UBUNTU_PACKAGE_NAME "UbuntuImage"
220+
221 namespace UpdatePlugin {
222
223 class FakeSystemUpdate: public QObject
224
225=== modified file 'tests/plugins/system-update/tst_updatemanager.cpp'
226--- tests/plugins/system-update/tst_updatemanager.cpp 2014-04-11 16:27:17 +0000
227+++ tests/plugins/system-update/tst_updatemanager.cpp 2014-05-07 15:38:11 +0000
228@@ -58,6 +58,7 @@
229 void UpdateManagerTest::testRegisterSystemUpdateRequired()
230 {
231 UpdateManager manager;
232+ manager.checkUpdates();
233 QSignalSpy spy(&manager, SIGNAL(modelChanged()));
234 QSignalSpy spy2(&manager, SIGNAL(updateAvailableFound(bool)));
235 QTRY_COMPARE(spy.count(), 0);
236@@ -66,6 +67,7 @@
237
238 Update *update = getUpdate();
239
240+ manager.setCheckSystemUpdates(true);
241 manager.registerSystemUpdate(update->getPackageName(), update);
242 QTRY_COMPARE(spy.count(), 1);
243 QTRY_COMPARE(spy2.count(), 1);
244@@ -102,6 +104,7 @@
245 void UpdateManagerTest::testStartDownload()
246 {
247 UpdateManager manager;
248+ manager.setCheckSystemUpdates(true);
249 Update *update = getUpdate();
250 manager.registerSystemUpdate(update->getPackageName(), update);
251 manager.startDownload(update->getPackageName());
252@@ -111,6 +114,7 @@
253 void UpdateManagerTest::testPauseDownload()
254 {
255 UpdateManager manager;
256+ manager.setCheckSystemUpdates(true);
257 Update *update = getUpdate();
258 manager.registerSystemUpdate(update->getPackageName(), update);
259 update->setUpdateState(true);
260@@ -123,7 +127,7 @@
261 UpdateManager manager;
262 QSignalSpy spy(&manager, SIGNAL(modelChanged()));
263 QTRY_COMPARE(manager.get_apps().size(), 0);
264- manager.checkUpdates();
265+ manager.getService().getCredentials();
266 QTRY_COMPARE(manager.get_apps().size(), 4);
267 QTRY_COMPARE(manager.get_model().size(), 1);
268 Update* app = manager.get_model()[0].value<Update*>();
269@@ -137,7 +141,7 @@
270 UpdateManager manager;
271 QSignalSpy spy(&manager, SIGNAL(updateAvailableFound(bool)));
272 QTRY_COMPARE(manager.get_apps().size(), 0);
273- manager.checkUpdates();
274+ manager.getService().getCredentials();
275 QTRY_COMPARE(manager.get_apps().size(), 4);
276 QTRY_COMPARE(manager.get_model().size(), 1);
277 Update* app = manager.get_model()[0].value<Update*>();

Subscribers

People subscribed via source and target branches