Merge lp://staging/~canonical-platform-qa/ubuntu-ota-tests/check-for-running-service into lp://staging/ubuntu-ota-tests

Comments inline.

I like the way you've created a package for utilities to live in. We should definitely do that. Also, I think it's perfectly fine to import things from the systemimage package. There's a lot of useful utilities there, such as systemimage.reactor.Reactor, and since this is the SUT, I think it's perfectly acceptable to use them when appropriate.

(Note, in the case where you locate the system-image-dbus process, si has a similar utility, but it's semantics are somewhat different, so it's not a perfect fit.)

Oh also, re: packaging. Yes, they probably should live outside the debian/tests directory, i.e. get installed into the system Python 3's dist-packages like a normal Python 3 package. That's a little more work though, so I think we can refactor that in later.

Thanks Barry I've made the suggested changes.

I came across an issue today where the example test would fail due to the process not being exited right at that moment. As a fix I'm using some functionality from autopilot, but I'm concerned by this for som ereasons:
  1. This pulls in autopilot as a dep. Not sure if we want to do that for the core parts of the scripts.
  2. There may be a better way to resolve the issue I use ap for.

I agree re: the packaging, we can iterate over it this week. We need to get the base code down so we can continue.

I'll take a closer look at what systemimage provides. Are you suggesting that there is code there already I can use in this instance?

I moved the selftest to a directory, and made it a test case so we get nice results out of it.

When using 3.0 from barry's branch, the test fails with:

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/dbus/bus.py", line 175, in activate_name_owner
    return self.get_name_owner(bus_name)
  File "/usr/lib/python3/dist-packages/dbus/bus.py", line 361, in get_name_owner
    's', (bus_name,), **keywords)
  File "/usr/lib/python3/dist-packages/dbus/connection.py", line 651, in call_blocking
    message, timeout)
dbus.exceptions.DBusException: org.freedesktop.DBus.Error.NameHasNoOwner: Could not get owner of name 'com.canonical.SystemImage': no such name

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/tmp/adt-run.cQbK74/build.KLU/real-tree/debian/tests/ubuntu_ota_tests/selftests/test_services.py", line 28, in test_ensure_system_image_dbus_not_runnig_must_stop_process
    services.get_system_image_interface()
  File "/tmp/adt-run.cQbK74/build.KLU/real-tree/debian/tests/ubuntu_ota_tests/services.py", line 56, in get_system_image_interface
    service = system_bus.get_object('com.canonical.SystemImage', '/Service')
  File "/usr/lib/python3/dist-packages/dbus/bus.py", line 241, in get_object
    follow_name_owner_changes=follow_name_owner_changes)
  File "/usr/lib/python3/dist-packages/dbus/proxies.py", line 248, in __init__
    self._named_service = conn.activate_name_owner(bus_name)
  File "/usr/lib/python3/dist-packages/dbus/bus.py", line 180, in activate_name_owner
    self.start_service_by_name(bus_name)
  File "/usr/lib/python3/dist-packages/dbus/bus.py", line 278, in start_service_by_name
    'su', (bus_name, flags)))
  File "/usr/lib/python3/dist-packages/dbus/connection.py", line 651, in call_blocking
    message, timeout)
dbus.exceptions.DBusException: org.freedesktop.DBus.Error.NoMemory: Launcher could not run (out of memory)

----------------------------------------------------------------------

One inline comment, I think also that we should describe a bit in the README the layout that we have decided for the project in order to make it more understandable: if we are not building any debian package, why is the source code under debian/tests? It seems that you must know about autopkgtest to understand where to find things, the layout seems to be conditioned by the tool used to run the tests.

Cheers!

Now the tests are passing for me. Weird because I didn't change anything, just reflashed. Maybe my phone was broken before.

On Mar 10, 2015, at 03:42 AM, Christopher Lee wrote:

>I came across an issue today where the example test would fail due to the
>process not being exited right at that moment. As a fix I'm using some
>functionality from autopilot, but I'm concerned by this for som ereasons:

Yeah, D-Bus is notoriously unreliable for tests like this. It's a source of
constant sorrow for getting system-image built in a PPA. I'm now testing some
new techniques using the org.freedesktop.DBus interface. It seems to improve
things but it's not perfect. There are still opportunities for race
conditions, and I suspect they're just inherent in D-Bus.

>1. This pulls in autopilot as a dep. Not sure if we want to do that for the
>core parts of the scripts.

I say "whatever works" :)

>2. There may be a better way to resolve the issue I use ap for.

Here's what I'm using now:

    bus = dbus.SystemBus()
    service = dbus.SystemBus().get_object('org.freedesktop.DBus', '/')
    iface = dbus.Interface(service, 'org.freedesktop.DBus')
    reply = 0
    # 2015-03-09 BAW: This could potentially spin forever, but we'll assume
    # D-Bus eventually is successful in starting the service.
    while reply != 2:
        reply = iface.StartServiceByName('com.canonical.SystemImage', 0)
        time.sleep(0.1)

`reply` could be 1 or 2. 1 means it's started but 2 means it's already
running, which is why I wait for that.

On Mar 10, 2015, at 09:18 AM, Federico Gimenez wrote:

>> +def ensure_loop_ready():
>> + global _loop_running
>
>This global variable and the need for the client code of this module to know
>how it works internally (it has to request first a interface and later pass
>that object to the module methods) makes me wonder if we should use better a
>class here, it seems that the design would be more suited to the task.

I have a somewhat different approach in my branch. We'll need to decide which
way we want to go.

On Mar 10, 2015, at 06:37 AM, Leo Arias wrote:

>When using 3.0 from barry's branch, the test fails with:

I can't tell you what's causing the NoMember error. As best I've been able to
determine, the NameHasNoOwner error occurs when the process
(system-image-dbus) associated with the name (com.canonical.SystemImage)
hasn't yet started. See my previous comment for a new approach I'm trying to
take, which works locally, although the proof will be in the PPA pudding.

This is good for me, lets use it as a bootstrap and improve things as we find them.

Me too.