ubuntu-touch-coreapps-reviewers team mailing list archive

[Nicholas Skaggs <nskaggs@xxxxxxxxxx>]

Review: Needs Fixing

Looks good. A few comments, including requests for more tests (can't help myself!). I wouldn't hold this for the test enhancements, but I'd like to see the other issues fixed.

Diff comments:

> === modified file 'CalendarChoicePopup.qml'
> --- CalendarChoicePopup.qml	2014-11-08 11:26:37 +0000
> +++ CalendarChoicePopup.qml	2015-04-14 19:53:55 +0000
> @@ -24,7 +24,7 @@
>  
>  Page {
>      id: root
> -
> +    objectName: "calendarchoicepopup"
>      property var model;
>  
>      signal collectionUpdated();
> @@ -67,6 +67,7 @@
>              width: aadCalendar.width
>              Button {
>                  id:aadCalendar
> +                objectName: "aadCalendar"
>                  text: i18n.tr("Add new Calendar")
>                  anchors.top :parent.top
>                  anchors.topMargin:units.gu(2)
> @@ -80,9 +81,11 @@
>          model : root.model.getCollections();
>          delegate: ListItem.Standard {
>              id: delegateComp
> +            objectName: "calendarItem"
>  
>              Rectangle {
>                  id: calendarColorCode
> +                objectName: "calendarColorCode"
>  
>                  width: parent.height - units.gu(2)
>                  height: width
> @@ -111,6 +114,7 @@
>              }
>  
>              Label{
> +                objectName: "calendarName"
>                  text: modelData.name
>                  elide: Text.ElideRight
>                  opacity: checkBox.checked ? 1.0 : 0.8
> @@ -125,6 +129,7 @@
>  
>              control: CheckBox {
>                  id: checkBox
> +                objectName: "checkBox"
>                  checked: modelData.extendedMetaData("collection-selected")
>                  enabled:  !root.isInEditMode
>                  onCheckedChanged: {
> 
> === modified file 'ColorPickerDialog.qml'
> --- ColorPickerDialog.qml	2014-09-19 08:46:30 +0000
> +++ ColorPickerDialog.qml	2015-04-14 19:53:55 +0000
> @@ -21,6 +21,7 @@
>  
>  Dialog {
>      id: root
> +    objectName: "colorPickerDialog"
>      title: i18n.tr("Select Color")
>      signal accepted(var color)
>      signal rejected()
> @@ -34,6 +35,7 @@
>              Repeater{
>                  model: ["#2C001E","#333333","#DD4814","#DF382C","#EFB73E","#19B6EE","#38B44A","#001F5C"];
>                  delegate:Rectangle{
> +                    objectName: "color" + index
>                      width: parent.width/5
>                      height: width
>                      color: modelData
> 
> === modified file 'EventActions.qml'
> --- EventActions.qml	2015-03-20 14:25:01 +0000
> +++ EventActions.qml	2015-04-14 19:53:55 +0000
> @@ -55,6 +55,7 @@
>  
>      Action{
>          id: _showCalendarAction
> +        objectName: "calendarsbutton"
>          iconName: "calendar"
>          text: i18n.tr("Calendars")
>          onTriggered: {
> 
> === modified file 'tests/autopilot/calendar_app/__init__.py'
> --- tests/autopilot/calendar_app/__init__.py	2015-04-10 19:33:42 +0000
> +++ tests/autopilot/calendar_app/__init__.py	2015-04-14 19:53:55 +0000
> @@ -142,6 +142,18 @@
>          header.click_action_button('neweventbutton')
>          return self.wait_select_single(NewEvent, objectName='newEventPage')
>  
> +    @autopilot.logging.log_action(logger.info)
> +    def go_to_calendar_choice_popup(self):
> +        """Open the calendar chioce popup.
> +
> +        :return: CalendaChoicePopup.
> +
> +        """
> +        header = self.get_header()
> +        header.click_action_button('calendarsbutton')
> +        return self.wait_select_single(
> +            CalendarChoicePopup, objectName="calendarchoicepopup")
> +
>      def set_picker(self, field, mode, value):
>          # open picker
>          self.pointing_device.click_object(field)
> @@ -285,6 +297,11 @@
>          header = self.get_header()
>          header.click_action_button('todaybutton')
>  
> +    @autopilot.logging.log_action(logger.info)
> +    def get_ColorPickerDialog(self):

ideally I would still avoid using Camelcase here, as it doesn't fit with our naming conventions. 

get_color_picker_dialog?

> +        return self.wait_select_single(
> +            "ColorPickerDialog", objectName="colorPickerDialog")
> +
>  
>  class YearView(ubuntuuitoolkit.UbuntuUIToolkitCustomProxyObjectBase):
>  
> @@ -1003,3 +1020,56 @@
>          delete_button = self.select_single(
>              'Button', objectName='deleteEventButton')
>          self.pointing_device.click_object(delete_button)
> +
> +
> +class CalendarChoicePopup(
> +        ubuntuuitoolkit.UbuntuUIToolkitCustomProxyObjectBase):
> +
> +    """Autopilot helper for the Calendar Choice Popup."""
> +
> +    @autopilot.logging.log_action(logger.debug)
> +    def press_check_box_button(self, calendarName):
> +        """ press check box button to select or unselect it """
> +        calendar = self._get_calendar(calendarName)
> +        check_box = calendar.wait_select_single(
> +            "CheckBox", objectName="checkBox")
> +        self.pointing_device.click_object(check_box)
> +
> +    def _get_calendar(self, calendarName):
> +        calendarItems = self.select_many("Standard", objectName="calendarItem")
> +        for item in calendarItems:
> +            if item.select_single(
> +                    "Label", objectName="calendarName").text == calendarName:
> +                    return item
> +
> +    @autopilot.logging.log_action(logger.debug)
> +    def get_checkbox_status(self, calendarName):
> +        """ press check box button to select or unselect it """
> +        calendar = self._get_calendar(calendarName)
> +        return calendar.wait_select_single(
> +            "CheckBox", objectName="checkBox").checked
> +
> +    @autopilot.logging.log_action(logger.debug)
> +    def get_calendar_color(self, calendarName):
> +        """ get calendar color """
> +        calendar = self._get_calendar(calendarName)
> +        return calendar.select_single(
> +            "QQuickRectangle", objectName="calendarColorCode").color
> +
> +    @autopilot.logging.log_action(logger.debug)
> +    def open_color_picker_dialog(self, calendarName):
> +        """ press color rectangle to open calendar color picker"""
> +        calendar = self._get_calendar(calendarName)
> +        color_rectangle = calendar.wait_select_single(
> +            "QQuickRectangle", objectName="calendarColorCode")
> +        self.pointing_device.click_object(color_rectangle)
> +
> +
> +class ColorPickerDialog(ubuntuuitoolkit.UbuntuUIToolkitCustomProxyObjectBase):
> +    """Autopilot helper for the Color Picker Dialog."""
> +
> +    @autopilot.logging.log_action(logger.debug)
> +    def change_calendar_color(self, new_color):
> +        new_color_circle = self.wait_select_single(
> +            "QQuickRectangle", objectName=new_color)
> +        self.pointing_device.click_object(new_color_circle)
> 
> === added file 'tests/autopilot/calendar_app/tests/test_management.py'
> --- tests/autopilot/calendar_app/tests/test_management.py	1970-01-01 00:00:00 +0000
> +++ tests/autopilot/calendar_app/tests/test_management.py	2015-04-14 19:53:55 +0000
> @@ -0,0 +1,58 @@
> +# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
> +#
> +# Copyright (C) 2013, 2014 Canonical Ltd
> +#
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License version 3 as
> +# published by the Free Software Foundation.
> +#
> +# 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 General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +"""
> +Calendar app autopilot tests calendar management.
> +"""
> +
> +
> +from testtools.matchers import NotEquals
> +
> +from calendar_app.tests import CalendarAppTestCase
> +
> +
> +class TestManagement(CalendarAppTestCase):
> +
> +    def test_change_calendar_color(self):
> +        """ Test changing calendar color   """
> +        calendar_choice_popup = \
> +            self.app.main_view.go_to_calendar_choice_popup()
> +        calendarName = "Personal"

Does it make sense to do this for other calendars, besides Personal?

> +        original_calendar_color = \
> +            calendar_choice_popup.get_calendar_color(calendarName)
> +        calendar_choice_popup.open_color_picker_dialog(calendarName)
> +        colorPickerDialog = self.app.main_view.get_ColorPickerDialog()
> +        colorPickerDialog.change_calendar_color("color6")
> +
> +        final_calendar_color = \
> +            calendar_choice_popup.get_calendar_color(calendarName)
> +
> +        self.assertThat(
> +            original_calendar_color, NotEquals(final_calendar_color))
> +
> +    def test_unselect_calendar(self):
> +        """ Test unselecting calendar    """
> +        calendar_choice_popup = \
> +            self.app.main_view.go_to_calendar_choice_popup()
> +        calendarName = "Personal"
> +        original_checbox_status = \
> +            calendar_choice_popup.get_checkbox_status(calendarName)
> +        calendar_choice_popup.press_check_box_button(calendarName)
> +
> +        self.assertThat(

Can you check and see if the events from this calendar no longer appear? Isn't that the idea? It might require making a new event on the calendar, then checking to ensure it doesn't show up.

> +            original_checbox_status,
> +            NotEquals(
> +                calendar_choice_popup.press_check_box_button(calendarName)))
> 


-- 
https://code.launchpad.net/~carla-sella/ubuntu-calendar-app/calendar-management-tests/+merge/256202
Your team Ubuntu Calendar Developers is subscribed to branch lp:ubuntu-calendar-app.