Merge lp:~aacid/autopilot/dbus_search_no_seen_connections into lp:autopilot

[Albert Astals Cid]

Do not store seen_connections

Paths can appear later than the bus has been created, so storing a bus to ignore it in the next loop can
make it ignore a bus that did not have the match previously but now does

[Albert Astals Cid (aacid)]

semi-related to https://bugs.launchpad.net/ubuntu/+source/unity8/+bug/1421009

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:549
http://jenkins.qa.ubuntu.com/job/autopilot-ci/1033/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-amd64-ci/96
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-amd64-ci/96/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-armhf-ci/96
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-armhf-ci/96/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-i386-ci/96
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-i386-ci/96/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/1972
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid-autopilot/141
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/1739
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/1970
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/1970/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19156
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-vivid-autopilot/143
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/892
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/892/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/autopilot-ci/1033/rebuild

review: Approve (continuous-integration)

[Timo Jyrinki (timo-jyrinki)]

Submitted for Autopilot 1.5 (the current release branch) at https://code.launchpad.net/~timo-jyrinki/autopilot/ap1.5_dbus_search_no_seen_connections/+merge/254200

[Christopher Lee (veebers)]

Good catch, this may make the search for a process take a little longer but I can't see another solution off the top of my head.

Generally looking good, have one concern listed inline with the diff.

review: Needs Fixing

[Albert Astals Cid (aacid)]

Done, not a python devel myself so the way i sorted the tests cases may not be the best, suggestions welcome

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:551
http://jenkins.qa.ubuntu.com/job/autopilot-ci/1034/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-amd64-ci/97
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-amd64-ci/97/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-armhf-ci/97
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-armhf-ci/97/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-i386-ci/97
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-i386-ci/97/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid-autopilot/144
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-vivid-autopilot/146
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/921
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/921/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/autopilot-ci/1034/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:551
http://jenkins.qa.ubuntu.com/job/autopilot-ci/1035/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-amd64-ci/98
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-amd64-ci/98/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-armhf-ci/98
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-armhf-ci/98/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-i386-ci/98/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid-autopilot/145
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-vivid-autopilot/147
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/927
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/927/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/autopilot-ci/1035/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:551
http://jenkins.qa.ubuntu.com/job/autopilot-ci/1036/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-amd64-ci/99
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-amd64-ci/99/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-armhf-ci/99
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-armhf-ci/99/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-i386-ci/99/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid-autopilot/146
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-vivid-autopilot/148
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/928
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/928/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/autopilot-ci/1036/rebuild

review: Needs Fixing (continuous-integration)

[Christopher Lee (veebers)]

Ugh, seems we're having jenkins/CI issues. Have pinged cihelp so hopefully it gets cleared out soon.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:551
http://jenkins.qa.ubuntu.com/job/autopilot-ci/1037/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-amd64-ci/100
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-amd64-ci/100/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-armhf-ci/100
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-armhf-ci/100/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-i386-ci/104
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-i386-ci/104/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2134
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid-autopilot/147
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/1880
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2132
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2132/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19457
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-vivid-autopilot/149
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/930
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/930/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/autopilot-ci/1037/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/autopilot-autolanding/322/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-amd64-autolanding/39
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-amd64-autolanding/39/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-armhf-autolanding/39
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-armhf-autolanding/39/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-i386-autolanding/39
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-i386-autolanding/39/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid-autopilot/149/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-vivid-autopilot/151/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/935
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/935/artifact/work/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/autopilot-autolanding/323/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-amd64-autolanding/40
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-amd64-autolanding/40/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-armhf-autolanding/40
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-armhf-autolanding/40/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-i386-autolanding/40
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-i386-autolanding/40/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid-autopilot/150/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-vivid-autopilot/152/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/936
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/936/artifact/work/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)

=== modified file 'autopilot/introspection/_search.py'
--- autopilot/introspection/_search.py	2014-07-31 20:02:49 +0000
+++ autopilot/introspection/_search.py	2015-04-02 08:29:36 +0000
@@ -307,17 +307,11 @@
         while we're searching for it.
 
     """
-    seen_connections = []
-
     for _ in Timeout.default():
         _get_child_pids.reset_cache()
         _raise_if_process_has_exited(process)
 
-        connections = _get_buses_unchecked_connection_names(
-            bus,
-            seen_connections
-        )
-        seen_connections.extend(connections)
+        connections = bus.list_names()
 
         valid_connections = [
             c for c
@@ -345,17 +339,6 @@
     return process.poll() is None
 
 
-def _get_buses_unchecked_connection_names(dbus_bus, previous_connections=None):
-    """Return a list of connections found on dbus_bus.
-
-    If previous_connections is supplied then those connections are removed from
-    the returned list.
-
-    """
-    all_conns = dbus_bus.list_names()
-    return list(set(all_conns).difference(set(previous_connections or [])))
-
-
 def _dedupe_connections_on_pid(valid_connections, bus):
     seen_pids = []
     deduped_connections = []

=== modified file 'autopilot/tests/unit/test_introspection_search.py'
--- autopilot/tests/unit/test_introspection_search.py	2014-05-29 19:44:44 +0000
+++ autopilot/tests/unit/test_introspection_search.py	2015-04-02 08:29:36 +0000
@@ -522,47 +522,6 @@
         )
 
 
-class FindMatchingConnectionsTests(TestCase):
-
-    """Testing the behaviour of _find_matching_connections and helper methods
-    used within.
-
-    """
-
-    def test_unchecked_connection_names_returns_all_buses_initially(self):
-        mock_connection_list = ['conn1', 'conn2', 'conn3']
-        dbus_bus = Mock()
-        dbus_bus.list_names = Mock(return_value=mock_connection_list)
-
-        self.assertThat(
-            _s._get_buses_unchecked_connection_names(dbus_bus),
-            ListContainsOnly(mock_connection_list)
-        )
-
-    def test_unchecked_connection_names_returns_only_unseen_connections(self):
-        mock_connection_list = ['conn1', 'conn2', 'conn3']
-        dbus_bus = Mock()
-        dbus_bus.list_names = Mock(return_value=mock_connection_list)
-
-        self.assertThat(
-            _s._get_buses_unchecked_connection_names(dbus_bus, ['conn3']),
-            ListContainsOnly(['conn1', 'conn2'])
-        )
-
-    def test_unchecked_connection_names_returns_empty_list_when_all_seen(self):
-        mock_connection_list = ['conn1', 'conn2', 'conn3']
-        dbus_bus = Mock()
-        dbus_bus.list_names = Mock(return_value=mock_connection_list)
-
-        self.assertThat(
-            _s._get_buses_unchecked_connection_names(
-                dbus_bus,
-                mock_connection_list
-            ),
-            Equals([])
-        )
-
-
 class ProcessAndPidErrorCheckingTests(TestCase):
 
     def test_raises_ProcessSearchError_when_process_is_not_running(self):
@@ -740,33 +699,34 @@
     def test_raise_if_not_single_result_doesnt_raise_with_single_result(self):
         _s._raise_if_not_single_result([1], "")
 
-    @patch.object(_s, '_get_buses_unchecked_connection_names')
-    def test_find_matching_connections_calls_connection_matcher(self, gbycn):
-        gbycn.return_value = ["conn1"]
-        connection_matcher = Mock(return_value=False)
-
-        with sleep.mocked():
-            _s._find_matching_connections("bus", connection_matcher)
-
-        connection_matcher.assert_called_with(("bus", "conn1"))
-
-    @patch.object(_s, '_get_buses_unchecked_connection_names')
-    def test_find_matching_connections_attempts_multiple_times(self, gbycn):
-        gbycn.return_value = ["conn1"]
-        connection_matcher = Mock(return_value=False)
-
-        with sleep.mocked():
-            _s._find_matching_connections("bus", connection_matcher)
-
-        connection_matcher.assert_called_with(("bus", "conn1"))
+    class FMCTest:
+        def list_names(self):
+            return ["conn1"]
+
+    def test_find_matching_connections_calls_connection_matcher(self):
+        bus = ProxyObjectTests.FMCTest()
+        connection_matcher = Mock(return_value=False)
+
+        with sleep.mocked():
+            _s._find_matching_connections(bus, connection_matcher)
+
+        connection_matcher.assert_called_with((bus, "conn1"))
+
+    def test_find_matching_connections_attempts_multiple_times(self):
+        bus = ProxyObjectTests.FMCTest()
+        connection_matcher = Mock(return_value=False)
+
+        with sleep.mocked():
+            _s._find_matching_connections(bus, connection_matcher)
+
+        connection_matcher.assert_called_with((bus, "conn1"))
         self.assertEqual(connection_matcher.call_count, 11)
 
-    @patch.object(_s, '_get_buses_unchecked_connection_names')
-    def test_find_matching_connections_dedupes_results_on_pid(self, gbycn):
-        gbycn.return_value = ["conn1"]
+    def test_find_matching_connections_dedupes_results_on_pid(self):
+        bus = ProxyObjectTests.FMCTest()
 
         with patch.object(_s, '_dedupe_connections_on_pid') as dedupe:
             with sleep.mocked():
-                _s._find_matching_connections("bus", lambda *args: True)
+                _s._find_matching_connections(bus, lambda *args: True)
 
-                dedupe.assert_called_once_with(["conn1"], "bus")
+                dedupe.assert_called_once_with(["conn1"], bus)