indicator-power

Merge lp://staging/~charlesk/indicator-power/lp-1373511-low-battery-warning into lp://staging/indicator-power/14.10

  1. lp-1373511-low-battery-warning
  2. Merge into trunk.14.10
Proposed by Charles Kerr
Status: Merged
Approved by: Ted Gould
Approved revision: 276
Merged at revision: 269
Proposed branch: lp://staging/~charlesk/indicator-power/lp-1373511-low-battery-warning
Merge into: lp://staging/indicator-power/14.10
Diff against target: 917 lines (+547/-183)
11 files modified
data/com.canonical.indicator.power.Testing.xml (+39/-0)
src/CMakeLists.txt (+6/-0)
src/device-provider-mock.c (+1/-1)
src/main.c (+6/-7)
src/notifier.c (+34/-24)
src/service.c (+13/-0)
src/testing.c (+351/-0)
src/testing.h (+66/-0)
tests/CMakeLists.txt (+0/-9)
tests/indicator-power-service-cmdline-battery.cc (+0/-127)
tests/manual (+31/-15)
To merge this branch: bzr merge lp://staging/~charlesk/indicator-power/lp-1373511-low-battery-warning
Reviewer Review Type Date Requested Status
Ted Gould (community) Approve
Antti Kaijanmäki (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+238034@code.staging.launchpad.net

Commit message

Add mock battery support to make QA testing easier.

Description of the change

Add mock battery support for QA testing.

From the tests/manual's notes:

The indicator's schema name is "com.canonical.indicator.power" and there are four keys: "mock-battery-enabled" (a boolean), "mock-battery-level" (charger percent, an integer from 0-100), "mock-battery-charging" (a boolean of whether the mock battery is charging or discharging), and "mock-battery-minutes-left" (minutes remaining to charge/discharge).

Example use:

$ gsettings list-recursively com.canonical.indicator.power | grep mock # get mock state
$ gsettings set com.canonical.indicator.power mock-battery-enabled true
$ gsettings set com.canonical.indicator.power mock-battery-level 10
$ gsettings reset-recursively com.canonical.indicator.power # reset to non-test state

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:270
http://jenkins.qa.ubuntu.com/job/indicator-power-ci/140/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-power-utopic-amd64-ci/41
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-power-utopic-armhf-ci/41
        deb: http://jenkins.qa.ubuntu.com/job/indicator-power-utopic-armhf-ci/41/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-power-ci/140/rebuild

review: Approve (continuous-integration)
Revision history for this message
Antti Kaijanmäki (kaijanmaki) wrote :

LGTM.

review: Approve
Revision history for this message
Ted Gould (ted) wrote :

I like the idea, but I don't love using gsettings. I think that the
problem could be that if a test fails half way, without its cleanup
step, things could be in a confusing state. I think we want a reboot "to
solve all ills." I think perhaps a couple of GActions or a DBus testing
interface would make more sense.

Revision history for this message
Charles Kerr (charlesk) wrote :

Ted, that was my first thought as well but the problem with implementing that is the service handles acquiring / owning the bus, and we don't necessarily want it to know anything about where the DeviceProvider is coming from.

To use GActions and a DBus interface for this, we'd need to pull name ownership out of the service, and sync the tests to match that too.

IMO there are positive arguments for that, but the refactoring would be much larger and would drift away from the goal of "let's make short-term QA testing easier".

Revision history for this message
Ted Gould (ted) wrote :

I'm not sure we're talking about the same thing, I was thinking,
modifying your example, of something like this:

- gsettings list-recursively com.canonical.indicator.power | grep mock #
show the current mock settings
+ gdbus call --session --dest com.canonical.indicator.power
--object-path /com/canonical/indicator/power/mock --method
org.freedesktop.DBus.Properties.GetAll

- gsettings set com.canonical.indicator.power mock-battery-enabled true
+ gdbus call --session --dest com.canonical.indicator.power
--object-path /com/canonical/indicator/power/mock --method
org.freedesktop.DBus.Properties.Set MockBattery "<@b true>"

- gsettings set com.canonical.indicator.power mock-battery-level 10
+ gdbus call --session --dest com.canonical.indicator.power
--object-path /com/canonical/indicator/power/mock --method
org.freedesktop.DBus.Properties.Set BatteryLevel "<@i 10>"

So it'd still be the same process, just not stored anywhere on disk.

Revision history for this message
Ted Gould (ted) :
review: Needs Information
lp://staging/~charlesk/indicator-power/lp-1373511-low-battery-warning updated
271. By Charles Kerr

move the testing settings to a DBus interface

272. By Charles Kerr

make service's bus connection a property so that IndicatorPowerTesting can listen for it and export an object before the name is acquired

273. By Charles Kerr

fix notify get-capabilities startup blocking bug found in testing

274. By Charles Kerr

move the mock battery from GSettings to DBus

Revision history for this message
Charles Kerr (charlesk) wrote :

Ted, okay here you go. :)

lp://staging/~charlesk/indicator-power/lp-1373511-low-battery-warning updated
275. By Charles Kerr

in testing.c, make 'bus' a private field instead of a public GObject property

Revision history for this message
Ted Gould (ted) :
review: Approve
lp://staging/~charlesk/indicator-power/lp-1373511-low-battery-warning updated
276. By Charles Kerr

r273 tried to fix the blocking bug listed at http://paste.ubuntu.com/8562226/, but in the process introduced a new bug that manifested in RTM. This removes r273 and uses a different fix.

Revision history for this message
Charles Kerr (charlesk) wrote :

Ted/Antti, r276 was needed to fix a bug that showed up when I tested the silo on RTM.

Commit r276 tested & works -- mako + rtm r87

Revision history for this message
Ted Gould (ted) wrote :

WFM

review: Approve

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