Autopilot

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Needs Fixing on 2014-03-19
Max Brustkern (community)			Approve on 2014-03-19
Thomi Richards (community)			Needs Fixing on 2014-02-26
Chris Gagnon (community)		2014-02-05	Needs Fixing on 2014-02-05
Review via email: mp+204976@code.launchpad.net

This proposal has been superseded by a proposal from 2014-03-20.

Commit message

Ignore statenotfound errors in print_tree

Description of the change

Dumping the object tree with print_tree often fails to complete due to StateNotFound errors. This modifies the print call to catch these errors and continue to print the largest possible tree. Other errors could be considered as well to try and print as complete a dump as possible.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-02-05:

FAILED: Continuous integration, rev:421
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~nskaggs/autopilot/catch-stateerror-printtree/+merge/204976/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/autopilot-ci/462/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-trusty-amd64-ci/188/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-trusty-armhf-ci/188/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2917/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2919/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Chris Gagnon (chris.gagnon) wrote on 2014-02-05:

This needs tests

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-02-05:

FAILED: Continuous integration, rev:424
http://jenkins.qa.ubuntu.com/job/autopilot-ci/463/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-trusty-amd64-ci/189/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-trusty-armhf-ci/189/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2919/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2921/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-02-05:

FAILED: Continuous integration, rev:425
http://jenkins.qa.ubuntu.com/job/autopilot-ci/464/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-trusty-amd64-ci/190/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-trusty-armhf-ci/190/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2922/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2924/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-02-05:

FAILED: Continuous integration, rev:426
http://jenkins.qa.ubuntu.com/job/autopilot-ci/465/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-trusty-amd64-ci/191/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-trusty-armhf-ci/191/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2926/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2928/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-02-05:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Nicholas Skaggs (nskaggs) wrote on 2014-02-05:

@ChrisGagnon, I'll try playing with AssertRaises to add a test for this path.

Revision history for this message

Thomi Richards (thomir-deactivatedaccount) wrote on 2014-02-09:

Setting this back to a WIP MP since we all agree that it needs tests before being merged.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-02-20:

FAILED: Continuous integration, rev:432
http://jenkins.qa.ubuntu.com/job/autopilot-ci/502/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-trusty-amd64-ci/228/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-trusty-armhf-ci/228/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-trusty-i386-ci/137/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/3367/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/3369/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-02-24:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-02-24:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-02-25:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-02-25:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Thomi Richards (thomir-deactivatedaccount) wrote on 2014-02-26:

at the end of your test, could you please add an assertion for the error message you expect to see?

Please set this MP back to 'Needs review' when you want another review.

Thanks

review: Needs Fixing

Revision history for this message

Nicholas Skaggs (nskaggs) wrote on 2014-03-07:

Argh, bzr lost my changes I made.. they never pushed I see. I'll redo.

lp:~nskaggs/autopilot/catch-stateerror-printtree updated on 2014-03-17

437. By Nicholas Skaggs on 2014-03-17: revert accidental whitespace removal

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-18:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-18:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-18:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Max Brustkern (nuclearbob) wrote on 2014-03-19:

Seems reasonable to me.

review: Approve

lp:~nskaggs/autopilot/catch-stateerror-printtree updated on 2014-03-19

438. By Nicholas Skaggs on 2014-03-19: leaving sorting alone, but otherwise dropping gettr for direct print from dict
439. By Nicholas Skaggs on 2014-03-19: %r for prints

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-19:

FAILED: Continuous integration, rev:438
http://jenkins.qa.ubuntu.com/job/autopilot-ci/600/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-trusty-amd64-ci/326/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-trusty-armhf-ci/326/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-trusty-i386-ci/235/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-autopilot/52/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4154/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-19:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Nicholas Skaggs (nskaggs) wrote on 2014-03-20:

I have concerns this may have introduced an issue with print_tree, which is not shown by the tests. The dump seems to get stuck in the regression loop. Namely the change from

for p in sorted(self.get_properties()):

for prop, value in sorted(self.get_properties().items()):

is the likely cause.

Can someone help setup a test object that contains a larger tree to vet these changes a bit better?

We create a fake object like so:

        fake_object = DBusIntrospectionObject(
            dict(id=[0, 123], path=[0, '/some/path'], text=[0, 'Hello']),
            '/some/path',
            Mock()
        )

I tried making the dict larger but failed. I'm missing something basic; how can i make a tree like this?

            == /some/path ==
            id: 123
            path: '/some/path'
            text: 'Hello'
                == /some/path/child ==
               id: 456
               path: '/some/path/child'
               text: 'Hello Child'
                == /some/path/child2 ==
               id: 789
               path: '/some/path/child2'
               text: 'Hello Child2'
            == /some/other/path ==
            id: 542
            path: '/some/other/path'
            text: 'Hello'

Revision history for this message

Thomi Richards (thomir-deactivatedaccount) wrote on 2014-03-20:

Hi,

I doubt that turning this:

for p in sorted(self.get_properties()):

into this:

for prop, value in sorted(self.get_properties().items()):

Is the cause of your problems. You're correct though - we need a better way to create fake proxy objects. This is something I've been thinking about for a looong time.

I suggest that you merge in my branch, which already has many of the changes in here, and add your exception handling and test code.

lp:~nskaggs/autopilot/catch-stateerror-printtree updated on 2014-03-20

440. By Nicholas Skaggs on 2014-03-20: merge thomi's tweaks
441. By Nicholas Skaggs on 2014-03-20: tweak merge
442. By Nicholas Skaggs on 2014-03-20: fix spacing :-)

Unmerged revisions

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:

Autopilot Hackers

Chris Gagnon

Nicholas Skaggs

 === modified file 'autopilot/introspection/dbus.py'
 --- autopilot/introspection/dbus.py	2014-03-11 16:39:25 +0000
 +++ autopilot/introspection/dbus.py	2014-03-19 22:17:49 +0000
@@ -651,12 +651,15 @@
              output.write("\n")
          output.write("%s== %s ==\n" % (indent, self._path))
          # print properties
--        for p in sorted(self.get_properties()):
--            output.write("%s%s: %s\n" % (indent, p, repr(getattr(self, p))))
--        # print children
--        if maxdepth is None or _curdepth < maxdepth:
--            for c in self.get_children():
--                c.print_tree(output, maxdepth, _curdepth + 1)
++        try:
++            for prop, value in sorted(self.get_properties().items()):
++                output.write("%s%s: %r\n" % (indent, prop, value))
++                # print children
++                if maxdepth is None or _curdepth < maxdepth:
++                    for c in self.get_children():
++                        c.print_tree(output, maxdepth, _curdepth + 1)
++        except StateNotFoundError as error:
++            output.write("%sError: %s\n" % (indent, error))
      @contextmanager
      def no_automatic_refreshing(self):
 === modified file 'autopilot/tests/unit/test_introspection_features.py'
 --- autopilot/tests/unit/test_introspection_features.py	2014-03-11 17:36:53 +0000
 +++ autopilot/tests/unit/test_introspection_features.py	2014-03-19 22:17:49 +0000
@@ -55,6 +55,7 @@
      _object_passes_filters,
      CustomEmulatorBase,
      DBusIntrospectionObject,
++    StateNotFoundError,
+ )
  from autopilot.utilities import sleep
@@ -305,6 +306,23 @@
              text: 'Hello'
              """))
++    def test_print_tree_exception(self):
++        """print_tree with StateNotFound exception"""
++
++        fake_object = self._print_test_fake_object()
++        child = Mock()
++        child.print_tree.side_effect = StateNotFoundError('child')
++
++        with patch.object(fake_object, 'get_children', return_value=[child]):
++            out = StringIO()
++            print_func = lambda: fake_object.print_tree(out)
++            self.assertThat(print_func, Not(Raises(StateNotFoundError)))
++            self.assertEqual(out.getvalue(), dedent("""\
++            == /some/path ==
++            id: 123
++            Error: Object not found with name 'child'.
++            """))
++
      def test_print_tree_fileobj(self):
          """print_tree with file object output"""