ubuntu-system-settings

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

  1. as-activation
  2. Merge into trunk
Proposed by Iain Lane
Status: Merged
Approved by: Sebastien Bacher
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
Sebastien Bacher (community) Approve
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.
Revision history for this message
Brendan Donegan (brendan-donegan) wrote :

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

Revision history for this message
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> ]

Revision history for this message
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.

Revision history for this message
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)
Revision history for this message
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.

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

That makes sense, thanks

review: Approve
Revision history for this message
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)
Revision history for this message
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
=== modified file 'src/accountsservice.cpp'
--- src/accountsservice.cpp 2014-01-06 17:40:07 +0000
+++ src/accountsservice.cpp 2014-07-08 11:06:40 +0000
@@ -45,9 +45,7 @@
45 this,45 this,
46 SLOT (slotNameOwnerChanged (QString, QString, QString)));46 SLOT (slotNameOwnerChanged (QString, QString, QString)));
4747
48 if (m_accountsserviceIface.isValid()) {48 setUpInterface();
49 setUpInterface();
50 }
51}49}
5250
53void AccountsService::slotChanged(QString interface,51void AccountsService::slotChanged(QString interface,
@@ -101,8 +99,6 @@
101QVariant AccountsService::getUserProperty(const QString &interface,99QVariant AccountsService::getUserProperty(const QString &interface,
102 const QString &property)100 const QString &property)
103{101{
104 if (!m_accountsserviceIface.isValid())
105 return QVariant();
106102
107 QDBusInterface iface (103 QDBusInterface iface (
108 "org.freedesktop.Accounts",104 "org.freedesktop.Accounts",
@@ -133,13 +129,11 @@
133 "org.freedesktop.DBus.Properties",129 "org.freedesktop.DBus.Properties",
134 m_systemBusConnection,130 m_systemBusConnection,
135 this);131 this);
136 if (iface.isValid()) {132 // The value needs to be carefully wrapped
137 // The value needs to be carefully wrapped133 iface.call("Set",
138 iface.call("Set",134 interface,
139 interface,135 property,
140 property,136 QVariant::fromValue(QDBusVariant(value)));
141 QVariant::fromValue(QDBusVariant(value)));
142 }
143}137}
144138
145void AccountsService::customSetUserProperty(const QString &method,139void AccountsService::customSetUserProperty(const QString &method,
@@ -151,7 +145,5 @@
151 m_systemBusConnection,145 m_systemBusConnection,
152 this);146 this);
153147
154 if (iface.isValid())148 iface.call(method, value);
155 iface.call(method, value);
156
157}149}

Subscribers

People subscribed via source and target branches