Autopilot

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

dbus_search_no_seen_connections
Merge into trunk

Proposed by Albert Astals Cid on 2015-03-25

Status:

Merged

Merged at revision:

550

Proposed branch:

lp:~aacid/autopilot/dbus_search_no_seen_connections

Merge into:

lp:autopilot

Diff against target:

151 lines (+26/-83)

2 files modified

autopilot/introspection/_search.py (+1/-18)
autopilot/tests/unit/test_introspection_search.py (+25/-65)

To merge this branch:

bzr merge lp:~aacid/autopilot/dbus_search_no_seen_connections

Undecided

Invalid

Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Needs Fixing on 2015-04-08
Christopher Lee (community)		2015-03-25	Needs Fixing on 2015-04-02
Review via email: mp+254109@code.launchpad.net

Commit Message

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) wrote on 2015-03-25:

PS Jenkins bot (ps-jenkins) wrote on 2015-03-25:

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) wrote on 2015-03-26:

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) wrote on 2015-04-02:

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) wrote on 2015-04-02:

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) wrote on 2015-04-02:

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) wrote on 2015-04-07:

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) wrote on 2015-04-07:

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) wrote on 2015-04-07:

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

PS Jenkins bot (ps-jenkins) wrote on 2015-04-07:

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) wrote on 2015-04-08:

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-04-08:

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Albert Astals Cid

Autopilot Hackers

Chris Gagnon

 === 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)

Autopilot

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

Commit Message

Description of the Change

Preview Diff

Subscribers