Merge lp:~laney/ubuntu-system-settings/as-activation into lp:ubuntu-system-settings

Proposed by Iain Lane on 2014-07-08
Status: Merged
Approved by: Sebastien Bacher on 2014-07-11
Approved revision: 777
Merged at revision: 792
Proposed branch: lp:~laney/ubuntu-system-settings/as-activation
Merge into: lp:ubuntu-system-settings
Diff against target: 51 lines (+7/-15)
1 file modified
src/accountsservice.cpp (+7/-15)
To merge this branch: bzr merge lp:~laney/ubuntu-system-settings/as-activation
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Needs Fixing on 2014-07-11
Sebastien Bacher (community) 2014-07-08 Approve on 2014-07-11
Review via email: mp+225953@code.launchpad.net

Commit message

[accountsservice] Don't guard D-Bus calls with isValid, because that doesn't work if the service has timed out.

To post a comment you must log in.
Brendan Donegan (brendan-donegan) wrote :

Any chance of adding a test for the failure scenario this fixes?

Iain Lane (laney) wrote :

On Tue, Jul 08, 2014 at 11:31:16AM -0000, Brendan Donegan wrote:
> Any chance of adding a test for the failure scenario this fixes?

If you want a test which takes ten minutes?

--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]

Iain Lane (laney) wrote :

> On Tue, Jul 08, 2014 at 11:31:16AM -0000, Brendan Donegan wrote:
> > Any chance of adding a test for the failure scenario this fixes?
>
> If you want a test which takes ten minutes?

I guess you could mock AS actually.

PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:777
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/940/
Executed test runs:
    ABORTED: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/1688/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/1430
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-amd64-ci/132
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-armhf-ci/132
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-armhf-ci/132/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-i386-ci/132
    None: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/1956/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2728
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2728/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9460
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1188
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1599
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1599/artifact/work/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)
Iain Lane (laney) wrote :

It's going to be a bit of an undertaking to get dbusnock to support bus activation. Might be best to land these now.

Sebastien Bacher (seb128) wrote :

That makes sense, thanks

review: Approve
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:777
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/965/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/1849
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/1549
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-amd64-ci/157
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-armhf-ci/157
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-armhf-ci/157/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-i386-ci/157
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/2108
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2943
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2943/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9680
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1299
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1739
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1739/artifact/work/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:777
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/966/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/1858
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/1555
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-amd64-ci/158
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-armhf-ci/158
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-armhf-ci/158/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-i386-ci/158
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/2118
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2954
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2954/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9693
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1303
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1745
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1745/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/966/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 'src/accountsservice.cpp'
2--- src/accountsservice.cpp 2014-01-06 17:40:07 +0000
3+++ src/accountsservice.cpp 2014-07-08 11:06:40 +0000
4@@ -45,9 +45,7 @@
5 this,
6 SLOT (slotNameOwnerChanged (QString, QString, QString)));
7
8- if (m_accountsserviceIface.isValid()) {
9- setUpInterface();
10- }
11+ setUpInterface();
12 }
13
14 void AccountsService::slotChanged(QString interface,
15@@ -101,8 +99,6 @@
16 QVariant AccountsService::getUserProperty(const QString &interface,
17 const QString &property)
18 {
19- if (!m_accountsserviceIface.isValid())
20- return QVariant();
21
22 QDBusInterface iface (
23 "org.freedesktop.Accounts",
24@@ -133,13 +129,11 @@
25 "org.freedesktop.DBus.Properties",
26 m_systemBusConnection,
27 this);
28- if (iface.isValid()) {
29- // The value needs to be carefully wrapped
30- iface.call("Set",
31- interface,
32- property,
33- QVariant::fromValue(QDBusVariant(value)));
34- }
35+ // The value needs to be carefully wrapped
36+ iface.call("Set",
37+ interface,
38+ property,
39+ QVariant::fromValue(QDBusVariant(value)));
40 }
41
42 void AccountsService::customSetUserProperty(const QString &method,
43@@ -151,7 +145,5 @@
44 m_systemBusConnection,
45 this);
46
47- if (iface.isValid())
48- iface.call(method, value);
49-
50+ iface.call(method, value);
51 }

Subscribers

People subscribed via source and target branches