Merge lp://qastaging/~indicator-applet-developers/indicator-network/secret-agent into lp://qastaging/indicator-network/13.10

Proposed by Pete Woods
Status: Merged
Approved by: Pete Woods
Approved revision: 296
Merged at revision: 291
Proposed branch: lp://qastaging/~indicator-applet-developers/indicator-network/secret-agent
Merge into: lp://qastaging/indicator-network/13.10
Diff against target: 934 lines (+499/-42)
18 files modified
CMakeLists.txt (+3/-0)
cmake/FindValgrind.cmake (+7/-0)
data/indicator-secret-agent.conf.in (+1/-4)
data/org.freedesktop.Notifications.xml (+47/-0)
debian/changelog (+7/-0)
debian/control (+1/-0)
secret-agent/CMakeLists.txt (+18/-0)
secret-agent/DBusTypes.h (+1/-2)
secret-agent/SecretAgent.cpp (+19/-8)
secret-agent/SecretAgent.h (+15/-4)
secret-agent/SecretRequest.cpp (+57/-8)
secret-agent/SecretRequest.h (+8/-2)
secret-agent/gtk/PasswordMenu.cpp (+134/-0)
secret-agent/gtk/PasswordMenu.h (+45/-0)
secret-agent/main.cpp (+2/-1)
tests/CMakeLists.txt (+1/-0)
tests/TestSecretAgent.cpp (+119/-13)
tests/data/valgrind.suppression (+14/-0)
To merge this branch: bzr merge lp://qastaging/~indicator-applet-developers/indicator-network/secret-agent
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Ted Gould (community) Approve
Michał Sawicz Needs Fixing
Pete Woods (community) Abstain
Review via email: mp+182898@code.qastaging.launchpad.net

Commit message

Support unity8's system dialogues

Description of the change

Support unity8's system dialogues

To post a comment you must log in.
Revision history for this message
Pete Woods (pete-woods) wrote :

Do not merge yet. This is dependent on MacSlow's branches for Unity and the notification backend:

lp:~macslow/unity-notifications/extended-snap-decisions
lp:~macslow/unity8/extended-snap-decisions

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Pete Woods (pete-woods) :
review: Abstain
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Michał Sawicz (saviq) wrote :

Hmm I tried building this locally in sbuild and got two test failures:

[==========] 8 tests from 5 test cases ran. (9618 ms total)
[ PASSED ] 6 tests.
[ FAILED ] 2 tests, listed below:
[ FAILED ] WpaPsk/TestSecretAgentGetSecrets.ProvidesPasswordForWpaPsk/0, where GetParam() = 12-byte object <78-
D7 DD-00 D0-D4 DD-00 C0-69 DE-00>
[ FAILED ] None/TestSecretAgentGetSecrets.ProvidesPasswordForWpaPsk/0, where GetParam() = 12-byte object <A0-D7
 DD-00 00-D7 DD-00 50-6C DE-00>

 2 FAILED TESTS

Any idea?

Revision history for this message
Pete Woods (pete-woods) wrote :

Just going to check it out myself.

On Mon, Sep 30, 2013 at 10:59 PM, Michał Sawicz <<email address hidden>
> wrote:

> Hmm I tried building this locally in sbuild and got two test failures:
>
> [==========] 8 tests from 5 test cases ran. (9618 ms total)
> [ PASSED ] 6 tests.
> [ FAILED ] 2 tests, listed below:
> [ FAILED ] WpaPsk/TestSecretAgentGetSecrets.ProvidesPasswordForWpaPsk/0,
> where GetParam() = 12-byte object <78-
> D7 DD-00 D0-D4 DD-00 C0-69 DE-00>
> [ FAILED ] None/TestSecretAgentGetSecrets.ProvidesPasswordForWpaPsk/0,
> where GetParam() = 12-byte object <A0-D7
> DD-00 00-D7 DD-00 50-6C DE-00>
>
> 2 FAILED TESTS
>
> Any idea?
> --
>
> https://code.launchpad.net/~indicator-applet-developers/indicator-network/secret-agent/+merge/182898
> You proposed
> lp:~indicator-applet-developers/indicator-network/secret-agent for merging.
>

Revision history for this message
Pete Woods (pete-woods) wrote :

Is this on the desktop, or on the device? Is it inside bzr bd, or running
the tests manually?

Revision history for this message
Michał Sawicz (saviq) wrote :

On a second try:

│[ FAILED ] 3 tests, listed below:
│Errors while running CTest
│[ FAILED ] WpaPsk/TestSecretAgentGetSecrets.ProvidesPasswordForWpaPsk/0, where GetParam() = 12-byte object <78-
│F7 8B-00 D0-F4 8B-00 C0-89 8C-00>
│[ FAILED ] WpaNone/TestSecretAgentGetSecrets.ProvidesPasswordForWpaPsk/0, where GetParam() = 12-byte object <50
│-F7 8B-00 D0-F4 8B-00 D0-47 8C-00>
│[ FAILED ] None/TestSecretAgentGetSecrets.ProvidesPasswordForWpaPsk/0, where GetParam() = 12-byte object <A0-F7
│ 8B-00 00-F7 8B-00 50-8C 8C-00>

Racy tests?

Revision history for this message
Pete Woods (pete-woods) wrote :

> On a second try:
>
> │[ FAILED ] 3 tests, listed below:
> │Errors while running CTest
> │[ FAILED ] WpaPsk/TestSecretAgentGetSecrets.ProvidesPasswordForWpaPsk/0,
> where GetParam() = 12-byte object <78-
> │F7 8B-00 D0-F4 8B-00 C0-89 8C-00>
> │[ FAILED ] WpaNone/TestSecretAgentGetSecrets.ProvidesPasswordForWpaPsk/0,
> where GetParam() = 12-byte object <50
> │-F7 8B-00 D0-F4 8B-00 D0-47 8C-00>
> │[ FAILED ] None/TestSecretAgentGetSecrets.ProvidesPasswordForWpaPsk/0,
> where GetParam() = 12-byte object <A0-F7
> │ 8B-00 00-F7 8B-00 50-8C 8C-00>
>
> Racy tests?

Okay, I've pushed a fix to this (I hope). I've moved the (only) sleep to before the UnityMenuModel is destructed. This is literally the only sleep in the whole test suite. I knew it would come back to bite me.

The problem is that I need to wait until the GLib dbus connection in a UnityMenuMenu finishes dispatching all its messages. Unfortunately there's no signal I can wait for or property I can poll to ascertain when the connection is flushed. Probably the UnityMenuModel should do a connection flush in its destructor.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Michał Sawicz (saviq) wrote :
review: Approve
Revision history for this message
Pete Woods (pete-woods) wrote :

> Yeah, that's working!
>
> Some comments on unity8's merge:
>
> https://code.launchpad.net/~macslow/unity8/extended-snap-decisions-
> part1/+merge/187312

:D

Glad it works for you. Do we need to coordinate a unity8 MR to happen at the same time as this one that removes the network manager agent from there?

Revision history for this message
Michał Sawicz (saviq) wrote :

On 01.10.2013 21:11, Pete Woods wrote:
> Glad it works for you. Do we need to coordinate a unity8 MR to happen at the same time as this one that removes the network manager agent from there?

Let's get everything to support the extended SDs to land in unity8 and
unity-notifications. What would be the implications of having two agents
for the transition period?

I've a branch up to remove it from unity8¹, so assuming we merge
everything and have one ASK for unity8 and indicator-network, we can
probably coordinate that.

¹ https://code.launchpad.net/~saviq/unity8/drop-network-agents/+merge/188700
--
Michał (Saviq) Sawicz <email address hidden>
Canonical Services Ltd.

Revision history for this message
Pete Woods (pete-woods) wrote :

The problem is that the agents fight each other, and only one of them gets
to be the agent. Actually, it might be okay if they are both there, but
only one of them would work at once (whichever started first). We'd want to
check this first , obviously.

Revision history for this message
Michał Sawicz (saviq) wrote :

Question... how will this behave on the desktop? If at all?

review: Needs Information
Revision history for this message
Pete Woods (pete-woods) wrote :

> Question... how will this behave on the desktop? If at all?

It's not present on the desktop images.

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

With all new unity8, unity-notifications and indicator-network from http://people.canonical.com/~msawicz/ext-snaps/ it works!

review: Approve (functional)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Michał Sawicz (saviq) wrote :

130 + unity8 >= 7.82,

Should be (>= 7.82).

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

Woot! Excited about this landing.

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

Stupid Jenkins failure "host key verification failed".

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: