Add DBus mocks for u/i testing

Bug #1204528 reported by Barry Warsaw
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ubuntu system image
Fix Released
High
Barry Warsaw

Bug Description

The system-settings u/i will talk to the com.canonical.SystemImage DBus service. We want to be able to easily test this on a desktop system where the file system layout is different. While system-image-dev adds a --testing option to system-image-dbus, this isn't quite enough really. Details to follow

Tags: client
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

1st case: static call: with the --testing=no-update, I think the different dbus method should returns statically:
(based on http://bazaar.launchpad.net/~ubuntu-system-image/ubuntu-system-image/client/view/head:/systemimage/dbus.py)

BuildNumber: 42 seems obligatory :p
IsUpdateAvailable: False
The other values seems not needed then, right?

2nd case: Success update: --testing=update-success:

BuildNumber: 42
IsUpdateAvailable: True
GetUpdateSize: 1337kB (so, I guess converted in bytes)
GetUpdateVersion: 44
GetDescriptions, should combine cases where we had descriptions on some language, but None on all updates, like:
[{'description': "Ubuntu Edge support", 'description-fr': "Support d'Ubuntu Edge"},
{'description': "Flipped container with 200% boot speed improvment"}]
Please note that the second update doesn't have french description on purpose (I think we should fallback to english in that case, which doesn't have any prefix, right?)
(btw, from that spec, is the list of descriptions ordered from older update to newest?)

GetUpdate(), well start a fake update :)
-> I think this should spawn some "progress" signal, with the bytes downloaded.

Then, after a while:
ReadyToReboot signal is triggered

Reboot: the call should succeed

If we trigger Cancel, it should:
- fail if GetUpdate() wasn't in progress
- trigger Canceled signal otherwise

* 3rd case, update download failed: --testing=update-success
Same values than before, apart that after calling GetUpdate(), UpdateFailed signal should be raised after few seconds (10s?)

* 4rd case: update available when daemon running: --testing=update-newpending (better name needed :p)
First call to IsUpdateAvailable: False
Then, UpdatePending signal raise after few seconds (10s?)
then calling IsUpdateAvailable returns True

Revision history for this message
Barry Warsaw (barry) wrote :

I've added LP: #1204618 to track plumbing the progress reporting through to DBus signals.

Didier, thanks for adding the detail. I really like how you reuse --testing for the mocks. A couple of comments, otherwise it looks quite good.

> Please note that the second update doesn't have french description on purpose (I think we should fallback to english in that case,
> which doesn't have any prefix, right?)

Generally right, but do note that English, even e.g. US English can be specified explicitly, so I think you'll need to look for that in en domains and do normal hierarchical fallback if it's missing. The fallback is always going to be English (kind of like gettext).

> (btw, from that spec, is the list of descriptions ordered from older update to newest?)

Correct.

> GetUpdate(), well start a fake update :)

Just for clarity, "fake update" means don't talk to the actual server. I think it also means, don't touch the file system (i.e. don't put any fake files in the expected locations).

> If we trigger Cancel, it should:
> - fail if GetUpdate() wasn't in progress
> - trigger Canceled signal otherwise

Currently, a Cancel is legal at any time. If it happens after GetUpdate() but before Reboot() it will cancel the reboot and issue a Cancelled signal. You can "pre-cancel" things too, so Cancel doesn't have to be called before the GetUpdate(), but it will have no effect until GetUpdate() and/or Reboot() is called.

> * 3rd case, update download failed: --testing=update-success
s/update-success/update-failed/

> * 4rd case: update available when daemon running: --testing=update-newpending (better name needed :p)
> First call to IsUpdateAvailable: False
> Then, UpdatePending signal raise after few seconds (10s?)
> then calling IsUpdateAvailable returns True

This one's a little weird because it's not how the client works. (Certainly open to suggestions!). The client retains state but only until it exits and is restarted. It's started by dbus activation and its maximum lifetime is 2 minutes by default (this can be changed in the config file, so we'll have to see if 2m is enough). While the client is running, available updates are only checked once. To check again, the client has to exit and restart.

I could potentially see that a second call to IsUpdateAvailable() could throw the state out and re-calculate it, but it would have to do the whole task, e.g. re-download the keyrings. I think I'd rather not do this.

Another option is to add an explicit Exit() call which would kill the server before the timeout. You could then call IsUpdateAvailable() -> False; Exit(); wait; IsUpdateAvailable() -> True (have to probably mock something crazy to make this work :). OTOH, I'm not sure that's a worthwhile testing scenario.

Other than that, this looks great and I'll work on it for the next release.

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Thanks for the --testing feedback! :)

@description: sure, please return this kind of description-en-US, description-fr-CA in addition to description-fr and description (the default)

@Fake updates: I think it also means, don't touch the file system (i.e. don't put any fake files in the expected locations).
Exactly!

@Cancel: So, there is a timeout before a reboot is actually done? is that controlled by the daemon? (and we can cancel the reboot)

@Siganl for pending-update: what's the usage then of this signal? It's only in case 2 minutes are spent, but then… it means the UI should do a new call to wake it up, and we won't have the signal as well, right? So it seems with those considerations that UpdatePending signal isn't needed, right?

Revision history for this message
Barry Warsaw (barry) wrote :

After further discussion on IRC with Didier, here's what needs to change:

IsUpdateAvailable() -> CheckForUpdate() and should not block (i.e. no return value)

Keep UpdatePending signal, which will now provide the "return" status of CheckForUpdate()

Will do everything else in comment #3. In IRC we cleared up the confusion between Cancel/Cancelled, GetUpdate, and Reboot.

Changed in ubuntu-system-image:
status: New → In Progress
Revision history for this message
Barry Warsaw (barry) wrote :

The IUA() change is tracked in LP: #1204976

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

ok, just to clear out once the API for cancel/reboot was better understood:

Cancel case:
1. an update is available
2. call of GetUpdate()
3. call of Cancel
-> cancelled signal received
4. call of Reboot (for the pleasure :p)
-> cancelled signal received

Barry Warsaw (barry)
Changed in ubuntu-system-image:
status: In Progress → Fix Committed
Barry Warsaw (barry)
Changed in ubuntu-system-image:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.