← Back to team overview

ubuntu-touch-coreapps-reviewers team mailing list archive

Re: [Merge] lp:~nik90/ubuntu-clock-app/increase-alarm-height into lp:ubuntu-clock-app

 

Review: Needs Fixing

Hi.

I noticed that there is some issues with Alarm label length.
Please take a look at screenshot:
https://launchpadlibrarian.net/213666708/alarm-height.png

I think 7 characters for Alarm label is not enough, and should be increased.

Also Qt.locale().standaloneDayName(0, Locale.ShortFormat) function should be mocked. Without that it will fail when locale is different than US_EN.

Diff comments:

> 
> === modified file 'tests/unit/tst_alarmUtils.qml'
> --- tests/unit/tst_alarmUtils.qml	2015-05-27 16:03:23 +0000
> +++ tests/unit/tst_alarmUtils.qml	2015-08-05 10:18:06 +0000
> @@ -133,7 +133,7 @@
>      function test_alarmDayString() {
>          var value = Alarm.Monday | Alarm.Tuesday | Alarm.Wednesday | Alarm.Sunday
>          var result = alarmUtils._get_day(value)
> -        compare(result, "Monday, Tuesday, Wednesday, Sunday", "Alarm Day not properly formatted")
> +        compare(result, "Mon, Tue, Wed, Sun", "Alarm Day not properly formatted")

You should compare results to Qt.locale().standaloneDayName(0, Locale.ShortFormat), or mock Qt.locale().standaloneDayName(0, Locale.ShortFormat) function. Without that these tests will fail locally, if some other locales are used.

>      }
>  
>      /*
> @@ -155,7 +155,7 @@
>          var alarmType = Alarm.Repeating
>          var alarmDaysOfWeek = Alarm.Monday | Alarm.Tuesday
>          var result = alarmUtils.format_day_string(alarmDaysOfWeek, alarmType)
> -        compare(result, "Monday, Tuesday", "Repeating alarm days of week is not formatted correctly")
> +        compare(result, "Mon, Tue", "Repeating alarm days of week is not formatted correctly")

You should mock Qt.locale().standaloneDayName(0, Locale.ShortFormat) function. Without that these tests will fail locally, if some other locales are used

>      }
>  
>      /*


-- 
https://code.launchpad.net/~nik90/ubuntu-clock-app/increase-alarm-height/+merge/266864
Your team Ubuntu Clock Developers is subscribed to branch lp:ubuntu-clock-app.


Follow ups

References