Comment 2 for bug 1204528

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.