Created attachment 255068 [details] [review]
power: Add a test with a failing mouse battery

Created attachment 255117 [details] [review]
power: Add a test with a failing mouse battery

FIXME, fails with:
ERROR:dbus.connection:Unable to set arguments ('/org/freedesktop/UPower/devices/mock_MOUSE_BAT1', 'org.freedesktop.UPower.Device', {'TimeToEmpty': dbus.Int64(1600L, variant_level=1), 'State': dbus.UInt32(2L, variant_level=1), 'Energy': dbus.Double(40.0, variant_level=1), 'Percentage': dbus.Double(40.0, variant_level=1), 'IsPresent': dbus.Boolean(True, variant_level=1), 'EnergyFull': dbus.Double(100.0, variant_level=1), 'Model': dbus.String(u'Bat1', variant_level=1), 'Type': dbus.UInt32(2L, variant_level=1), 'PowerSupply': dbus.Boolean(False, variant_level=1)}, []) according to signature None: <type 'exceptions.ValueError'>: Unable to guess signature from an empty list

I suppose this is happening with

  obj_bat2.EmitSignal('', 'Changed', '', [], dbus_interface='org.freedesktop.DBus.Mock')

?

In order to pass an empty array with dbus-python, you need to wrap it in a type description, I think for this case dbus.Array([], signature='v') ought to work. Can test it more in-depth last week, still rather busy at Plumbers this week, sorry.

(In reply to comment #3)
> I suppose this is happening with
> 
>   obj_bat2.EmitSignal('', 'Changed', '', [],
> dbus_interface='org.freedesktop.DBus.Mock')
> 
> ?
> 
> In order to pass an empty array with dbus-python, you need to wrap it in a type
> description, I think for this case dbus.Array([], signature='v') ought to work.
> Can test it more in-depth last week, still rather busy at Plumbers this week,
> sorry.

Nope, it's the:
self.obj_upower.AddObject() call that fails.

With this patch it fails with

  TypeError: Fewer items found in D-Bus signature than in Python arguments

because as the third argument AddObject() expects a list of methods, but it finds "
signature='sv'" (which isn't tied to any dbus.WhateverObject() constructor). From the backtrace it seems your real code uses

+                                               }, [])

instead, which would explain the "empty array" error. That can be fixed with explicitly specifying it:

+                                               }, dbus.Array([], signature='(ssss)'))

The next error happens because you save the return value of AddObject() to bat2_path, but AddObject() doesn't actually return anything. That can be fixed with assigning it beforehand:

  bat2_path = '/org/freedesktop/UPower/devices/mock_MOUSE_BAT1'
  self.obj_upower.AddObject(bat2_path,
                            'org.freedesktop.UPower.Device',
                            { ...
                            },
                            dbus.Array([], signature='(ssss)'))

With this, the mocking works.

BTW, if you want to make this whole thing a bit easier, I suggest you use the convenience AddDischargingBattery() method and afterwards change some of its properties, e. g.

  bat2_path = self.obj_upower.AddDischargingBattery('mock_MOUSE_BAT1', 'Mouse1', 40.0, 1200)
  obj_bat2 = self.system_bus_con.get_object('org.freedesktop.UPower', bat2_path)
  props_bat2 = dbus.Interface(obj_bat2, dbus.PROPERTIES_IFACE)
  props_bat2.Set('org.freedesktop.UPower.Device', 'PowerSupply', False)

etc.

I also suggest to change this:

                                      # UP_DEVICE_KIND_BATTERY
                                      'Type': dbus.UInt32(2, variant_level=1),

to using Type 5 (KIND_MOUSE); this recently got fixed in upower (http://cgit.freedesktop.org/upower/commit/?id=5c0a57cfa). Also, the test should actually expect a suspend after the "real" battery (bat0) goes flat, as the mouse battery is not a system battery (PowerSupply is False).

With above changes, the test case now fails like half of the other ones with

    log = self.logind.stdout.read()
IOError: [Errno 11] Resource temporarily unavailable

It seems the tests expect something to trigger a logind action, but nothing does that any more? We can certainly clean up the exception to say something like "expected a logind Suspend() method call" (in fact with recent dbusmock versions this can be done a whole lot more elegantly, without having to read file descriptors), but that doesn't change the root cause. I guess that's related to "after pushing the idle monitors to mutter", but shouldn't mutter talk to logind as well if it sees that it should suspend, etc?

Or is our own logind mock in gsdtestcase.py not sufficient? dbusmock ships a much more complete logind mock these days, so we could actually throw out the gsdtestcase.py implementation and just use the standard template.

Attachment 255117 [details] pushed as d7bbbd3 - power: Add a test with a failing mouse battery