Ubuntu UI Toolkit

Merge lp:~elopio/ubuntu-ui-toolkit/fix1314390-click_element_on_unity into lp:ubuntu-ui-toolkit/staging

fix1314390-click_element_on_unity
Merge into staging

Proposed by Leo Arias on 2014-04-29

Status:

Merged

Approved by:

Tim Peeters on 2014-05-17

Approved revision:

1032

Merged at revision:

1057

Proposed branch:

lp:~elopio/ubuntu-ui-toolkit/fix1314390-click_element_on_unity

Merge into:

lp:ubuntu-ui-toolkit/staging

Diff against target:

97 lines (+73/-9)

2 files modified

tests/autopilot/ubuntuuitoolkit/_custom_proxy_objects/_flickable.py (+13/-9)
tests/autopilot/ubuntuuitoolkit/tests/custom_proxy_objects/test_flickable.py (+60/-0)

To merge this branch:

bzr merge lp:~elopio/ubuntu-ui-toolkit/fix1314390-click_element_on_unity

Undecided

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Approve on 2014-05-17
Tim Peeters			Approve on 2014-05-15
Cris Dywan		2014-04-29	Needs Information on 2014-05-06
Allan LeSage (community)			Approve on 2014-05-05
Review via email: mp+217693@code.launchpad.net

This proposal supersedes a proposal from 2014-04-29.

Commit message

On the autopilot helpers, find the top container starting from the flickable.

Revision history for this message

Allan LeSage (allanlesage) wrote on 2014-04-29:

Not sure I understand the error condition: it seems like it should mean that you're already at the topmost container (which wouldn't necessarily be an error?), however we'd want to validate that the parent has a globalRect in this case, no?

review: Needs Information

Revision history for this message

Leo Arias (elopio) wrote on 2014-04-29:

Do you mean this error condition?

25 + if top_container is None:
26 + raise _common.ToolkitException(
27 + "Couldn't find the top-most container.")

It will just happen if no parent has a global rect. Which is an unlikely case, but could happen.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-29:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/133/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-30:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/134/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-30:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/136/rebuild

review: Approve (continuous-integration)

Revision history for this message

Allan LeSage (allanlesage) wrote on 2014-05-05:

Thank you for answering my question elopio.

review: Approve

Revision history for this message

Cris Dywan (kalikiana) wrote on 2014-05-06:

test_get_unity_top_container seems to mock what the API expects but could we have a real QML snippet here that demonstrates the issue?

review: Needs Information

Revision history for this message

Cris Dywan (kalikiana) wrote on 2014-05-08:

See lp:~ubuntu-sdk-team/ubuntu-ui-toolkit/engineLauncher for a way to reproduce the difference in the component hierarchy.

Revision history for this message

Leo Arias (elopio) wrote on 2014-05-10:

kalikiana, we can merge this and once your branch is ready to be used, I'll update the test.
I say it mainly because this branch is a prerequisite of a branch that's a prerequisite for your tumb branch. For my side, I wouldn't mind waiting.

Revision history for this message

Tim Peeters (tpeeters) wrote on 2014-05-13:

I have a branch depending on this one that adds automatic showing of the header in AP tests when needed which is currently blocking the telephony apps, so I would not mind if this one is merged soon.

Revision history for this message

Tim Peeters (tpeeters) wrote on 2014-05-13:

no pressure ;)

Revision history for this message

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

FAILED: Continuous integration, rev:1033
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/234/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/254/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/230/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-amd64-ci/66/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/66/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-i386-ci/66/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/520/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/285/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/234/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Tim Peeters (tpeeters) wrote on 2014-05-15:

Thanks for adding the FIXME.

CI failed on the old 1033 where the repo was messed up, but 1030 was good so the new 1031 should pass soon also.

There are no merge conflicts with staging.

I'm approving so that we can get all the depending branches in, but let's fix the FIXME as soon as possible.

review: Approve

Revision history for this message

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Cris Dywan (kalikiana) wrote on 2014-05-16:

_StringException: Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/ubuntuuitoolkit/tests/custom_proxy_objects/test_flickable.py", line 52, in test_get_unity_top_container
    flickable = ubuntuuitoolkit.Flickable(dummy_state, 'dummy', 'dummy')
  File "/usr/lib/python3/dist-packages/ubuntuuitoolkit/_custom_proxy_objects/_common.py", line 68, in __init__
    super(UbuntuUIToolkitCustomProxyObjectBase, self).__init__(*args)
  File "/usr/lib/python3/dist-packages/autopilot/introspection/dbus.py", line 86, in __init__
    self.id
  File "/usr/lib/python3/dist-packages/autopilot/introspection/_xpathselect.py", line 182, in new_from_path_and_id
    % type(path).__name__
TypeError: 'path' attribute must be bytes, not 'str'

Revision history for this message

Leo Arias (elopio) wrote on 2014-05-16:

It would have been so easy to get this merged without tests ;)

It should work now. I hope.

Revision history for this message

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

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/240/rebuild

review: Approve (continuous-integration)

Revision history for this message

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Tim Peeters (tpeeters) wrote on 2014-05-17:

autolanding failed because of jenkins breakage? happroving again.

Revision history for this message

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

review: Approve (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:

Dan Chapman 

Leo Arias

Tom Mirmar

Ubuntu SDK team

 === modified file 'tests/autopilot/ubuntuuitoolkit/_custom_proxy_objects/_flickable.py'
 --- tests/autopilot/ubuntuuitoolkit/_custom_proxy_objects/_flickable.py	2014-04-16 21:13:39 +0000
 +++ tests/autopilot/ubuntuuitoolkit/_custom_proxy_objects/_flickable.py	2014-05-16 08:50:05 +0000
@@ -68,15 +68,19 @@
      def _get_top_container(self):
          """Return the top-most container with a globalRect."""
          root = self.get_root_instance()
--        containers = [root]
--        while len(containers) == 1:
--            try:
--                containers[0].globalRect
--                return containers[0]
--            except AttributeError:
--                containers = containers[0].get_children()
--
--        raise _common.ToolkitException("Couldn't find the top-most container.")
++        parent = self.get_parent()
++        top_container = None
++        while parent.id != root.id:
++            if hasattr(parent, 'globalRect'):
++                top_container = parent
++
++            parent = parent.get_parent()
++
++        if top_container is None:
++            raise _common.ToolkitException(
++                "Couldn't find the top-most container.")
++        else:
++            return top_container
      def _is_child_visible(self, child, containers):
          """Check if the center of the child is visible.
 === added file 'tests/autopilot/ubuntuuitoolkit/tests/custom_proxy_objects/test_flickable.py'
 --- tests/autopilot/ubuntuuitoolkit/tests/custom_proxy_objects/test_flickable.py	1970-01-01 00:00:00 +0000
 +++ tests/autopilot/ubuntuuitoolkit/tests/custom_proxy_objects/test_flickable.py	2014-05-16 08:50:05 +0000
@@ -0,0 +1,60 @@
++# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
++#
++# Copyright (C) 2014 Canonical Ltd.
++#
++# This program is free software; you can redistribute it and/or modify
++# it under the terms of the GNU Lesser General Public License as published by
++# the Free Software Foundation; version 3.
++#
++# This program is distributed in the hope that it will be useful,
++# but WITHOUT ANY WARRANTY; without even the implied warranty of
++# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
++# GNU Lesser General Public License for more details.
++#
++# You should have received a copy of the GNU Lesser General Public License
++# along with this program. If not, see <http://www.gnu.org/licenses/>.
++
++
++import testtools
++
++import ubuntuuitoolkit
++
++
++class FlickableTestCase(testtools.TestCase):
++
++    def test_get_unity_top_container(self):
++        """Test that we can get the top cointainer in Unity."""
++        # This tests bug http://pad.lv/1314390
++        # On Unity, the top container is not the first child as it is in all
++        # the apps that have a MainView. This makes the first implementation of
++        # _get_top_container fail. Instead of going from the top looking for
++        # a container, we should start from the flickable until we find the
++        # top-most container.
++        # FIXME we are faking the QML tree because we have no way to launch
++        # the app with a tree like the one in Unity8. kalikiana has a branch
++        # with an alternate launcher that will let us clean this test.
++        # --elopio - 2014-05-15.
++        RootClass = type('obj', (object,), {'id': 'root'})
++        mock_root_instance = RootClass()
++        # We consider a container is an object with a globalRect.
++        MockNonContainerClass = type('obj', (object,), {})
++        mock_non_container = MockNonContainerClass()
++        MockContainerClass = type(
++            'obj', (object,), {'id': 'container', 'globalRect': 'dummy'})
++        mock_container = MockContainerClass()
++        mock_container.get_parent = lambda: mock_root_instance
++
++        # The root instance has two children. This exposes the bug.
++        mock_root_instance.get_children = lambda: [
++            mock_non_container, mock_container]
++
++        dummy_state = {'id': '10'}
++        flickable = ubuntuuitoolkit.Flickable(
++            dummy_state, '/dummy'.encode(), 'dummy')
++
++        flickable.get_root_instance = lambda: mock_root_instance
++        # The top container of the flickable is its immediate parent.
++        flickable.get_parent = lambda: mock_container
++
++        top_container = flickable._get_top_container()
++        self.assertEqual(top_container, mock_container)

Ubuntu UI Toolkit

Merge lp:~elopio/ubuntu-ui-toolkit/fix1314390-click_element_on_unity into lp:ubuntu-ui-toolkit/staging

Commit message

Description of the change

Preview Diff

Subscribers