Merge lp://staging/~zsombi/ubuntu-ui-toolkit/70-cpos into lp://staging/ubuntu-ui-toolkit/staging

 + def _click_on_panel_action(self, swipe_direction, action_object):
101 + self._swipe_content(swipe_direction)
102 + try:
103 + button_name = 'actionbutton_' + action_object
104 + action_button = self.select_single(objectName=button_name)
105 + except dbus.StateNotFound:
106 + raise _common.ToolkitException(
107 + 'The requested action not found on leading side')

the exception text is incorrect, swipe direction may be 'left', then you are looking for an action on trailing side.

swipe_direction is ambiguous. Is it swipe TO this direction or swipe FROM? Perhaps rename the variable to "swipe_to_direction"?

115 + def click_leading_action(self, action_objectName):
120 + def click_trailing_action(self, action_objectName):

Maybe trigger_{leading,trailing}_action(..) is better here. You click on a button to trigger the action.

114 + @autopilot_logging.log_action(logger.info)
115 + def click_leading_action(self, action_objectName):
116 + """Swipe the item in from left to right to open leading actions."""
117 + self._click_on_panel_action('right', action_objectName)
118 +
119 + @autopilot_logging.log_action(logger.info)
120 + def click_trailing_action(self, action_objectName):
121 + """Swipe the item in from right to left to open trailing actions."""
122 + self._click_on_panel_action('left', action_objectName)

You do more than just swipe in these functions.
Add something similar to this to the descriptions: "and click on the button representing the requested action".

244 + Label {
245 + id: triggeredAction
246 + height: paintedHeight
247 + objectName: "triggeredAction"
248 + }

It is better to anchor this somewhere. Who knows where it may end up now?

237 +MainView {

I propose to use a MainView with a Page inside (then there is a header), or simply to use an Item in which you test the ListView with ListItems.

244 + Label {
245 + id: triggeredAction
246 + height: paintedHeight
247 + objectName: "triggeredAction"
248 + }

I prefer an initial label of "no action triggered", and when an action is triggered label.text = objectName + " action triggered". Adds a little bit of clarity when watching the videos of a failed test.

We should also test what happens when you self.test_listitem.click_leading_action('this_action_does_not_exist')

> 244 + Label {
> 245 + id: triggeredAction
> 246 + height: paintedHeight
> 247 + objectName: "triggeredAction"
> 248 + }
>
> It is better to anchor this somewhere. Who knows where it may end up now?

Any item unanchored is positioned to (0,0), you should know that. Everyone should know that.

> + def _click_on_panel_action(self, swipe_direction, action_object):
> 101 + self._swipe_content(swipe_direction)
> 102 + try:
> 103 + button_name = 'actionbutton_' + action_object
> 104 + action_button =
> self.select_single(objectName=button_name)
> 105 + except dbus.StateNotFound:
> 106 + raise _common.ToolkitException(
> 107 + 'The requested action not found on leading side')
>
>
> the exception text is incorrect, swipe direction may be 'left', then you are
> looking for an action on trailing side.

Aah, right, I forgot to adjust that message when moved the code in one function. Fix in the next revision.

> swipe_direction is ambiguous. Is it swipe TO this direction or swipe FROM?
> Perhaps rename the variable to "swipe_to_direction"?

Uhm... I think this naming is widely used and means always the destination direction. You specify "to" and "from" only if you have both parameters in a function.

> 115 + def click_leading_action(self, action_objectName):
> 120 + def click_trailing_action(self, action_objectName):
>
> Maybe trigger_{leading,trailing}_action(..) is better here. You click on a
> button to trigger the action.

Ack, changed.

> > swipe_direction is ambiguous. Is it swipe TO this direction or swipe FROM?
> > Perhaps rename the variable to "swipe_to_direction"?
>
> Uhm... I think this naming is widely used and means always the destination
> direction. You specify "to" and "from" only if you have both parameters in a
> function.

Actually I reformatted that part so now it uses panel_item (leading/trailing) to identify which panel to be swiped in.

> 237 +MainView {
>
> I propose to use a MainView with a Page inside (then there is a header), or
> simply to use an Item in which you test the ListView with ListItems.

Ok, I'll do that. But then having a label in a view and under it the list view makes not much sense, we can use the app title to store the action triggered.

> We should also test what happens when you
> self.test_listitem.click_leading_action('this_action_does_not_exist')

Done.

> > 244 + Label {
> > 245 + id: triggeredAction
> > 246 + height: paintedHeight
> > 247 + objectName: "triggeredAction"
> > 248 + }
> >
> > It is better to anchor this somewhere. Who knows where it may end up now?
>
> Any item unanchored is positioned to (0,0), you should know that. Everyone
> should know that.

Do you have a reference for that?

should we rename _click_on_panel_action() to _trigger_action() or _click_on_panel_button() ?

good

Looks pretty good. Just, please change the last two tests to something like this:

def test_trigger_nonexistent_leading_action(self):
    error = self.assertRaises(
       ubuntuuitoolkit.ToolkitException,
       self.test_listitem.trigger_leading_action,
       'this_action_does_not_exist')
    self.assertEqual(
        str(error),
       'The requested action not found on leading side')

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-autolanding/751/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-autolanding/58/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-autolanding/58/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-autolanding/58/console

thanks