ubuntu-touch-coreapps-reviewers team mailing list archive

[Dan Chapman  <danchapman1819@xxxxxxxxx>]

Review: Needs Fixing

Looks good. Just a few issues for which I've left comments inline. 

Diff comments:

> === modified file 'README'
> --- README	2013-09-26 05:15:16 +0000
> +++ README	2015-12-12 17:42:02 +0000
> @@ -1,9 +1,23 @@
> -Welcome the Ubuntu Calendar!
> -
> -Currently this application is fully implemented in QML/JS.
> -All you need to launch this all is Qt5 and the qmlscene viewer:
> +ReadMe - Ubuntu Calendar App 
> +============================
> +
> +Ubuntu Calendar App is the official calendar app for Ubuntu Touch. We follow an open source model where the code is available to anyone to branch and to hack on. 
> +The ubuntu calendaar app follows a test driven development (TDD) where tests are written in parallel to feature implementation to help spot regressions easier.
> +
> +Useful Links
> +============
> +
> +Here are some useful links with regards to the Calendar App development.
> +
> +* Home Page      - https://developer.ubuntu.com/en/community/core-apps/calendar/
> +* Calendar App Wiki - https://wiki.ubuntu.com/Touch/CoreApps/Calendar
> +* Designs        - https://docs.google.com/presentation/d/14NIPecPFKb_8Ad3O4suEGJqVOw9bC4n0s63uWalbZ98/edit#slide=id.p
> +* Project page   - https://launchpad.net/ubuntu-calendar-app
> +
> +Launch
> +======
> +This application is implemented in QML/JS.
> +You need Qt5 and the qmlscene viewe to launch this application.

missing "r" from viewer

>  
>  $ cd ubuntu-calendar-app
>  $ qmlscene calendar.qml
> -
> -To test the application on the device check out the "testrun.sh" script.
> 
> === added file 'README.autopilot'
> --- README.autopilot	1970-01-01 00:00:00 +0000
> +++ README.autopilot	2015-12-12 17:42:02 +0000
> @@ -0,0 +1,58 @@
> +Running Autopilot tests
> +=======================
> +
> +Ubuntu Calendar App follows a test driven development where autopilot tests are run before every merge into trunk.
> +If you are submitting your bugfix/patch to the calendar app, please follow the following steps below to ensure that all tests pass before proposing a merge request.
> +
> +If you are looking for more info about Autopilot or writing AP tests for the calendar app, here are some useful links to help you:
> +
> +* http://developer.ubuntu.com/start/quality
> +* https://developer.ubuntu.com/api/autopilot/python/1.5.0/
> +
> +For help and options on running tests, see:
> +
> +* https://developer.ubuntu.com/en/start/platform/guides/running-autopilot-tests/
> +
> +Prerequisites
> +=============
> +
> +Install the following autopilot packages required to run the tests,
> +    $ sudo apt-get install python3-autopilot libautopilot-qt ubuntu-ui-toolkit-autopilot python3-autopilot-vis

You probably want to add a note that address-book-service-testability package may also be required if not already installed

> +
> +Running tests on the desktop
> +============================
> +
> +Using terminal:
> +
> +*  Branch the calendar app code,
> +    $ bzr branch lp:ubuntu-calendar-app
> +
> +*  Build the calendar app,
> +    $ mkdir builddir && cd builddir
> +    $ cmake .. && cmake --build . -- -j 3
> +    $ cd ..
> +
> +*  Navigate to the tests/autopilot directory.
> +    $ cd tests/autopilot
> +
> +*  run all tests.
> +    $ autopilot3 run -vv ubuntu_calendar_app
> +
> +    to list all tests:
> +    $ autopilot3 list ubuntu_calendar_app
> +
> +    To run only one test (for instance: test_change_week_across_year in TestWeekview.py):
> +    $ autopilot3 run -vv ubuntu_calendar_app.tests.test_weekview.TestWeekview.test_change_week_across_year 
> +
> +    Debugging tests using autopilot vis
> +    $ autopilot3 launch -i Qt qmlscene ../../app/ubuntu-calendar-app.qml  -I ../../builddir/backend/
> +    $ autopilot3 vis
> +
> +
> +Running tests on device or emulator
> +===================================
> +
> +Using autopkg:
> +
> +1. navigate to the directory where the ubuntu-calendar-app branch is and run:
> +  $ adt-run ubuntu-calendar-app --click=com.ubuntu.calendar --- ssh -s adb -p YOURPASSWORD
> 
> === added file 'README.mergeproposal'
> --- README.mergeproposal	1970-01-01 00:00:00 +0000
> +++ README.mergeproposal	2015-12-12 17:42:02 +0000
> @@ -0,0 +1,33 @@
> +Prerequisites to approving a Merge Proposal (MP)
> +================================================
> +
> +Over time, it has been found that insufficient testing by reviewers sometimes
> +leads to calendar app trunk not buildable in Qtcreator due to manifest errors, or
> +translation pot file not updated. As such, please follow the checklist below
> +before top-approving a MP.
> +
> +Checklist
> +=========
> +
> +*   Does the MP add/remove user visible strings? If Yes, has the pot file been
> +    updated?
> +
> +*   Does the MP change the UI? If Yes, has it been approved by design?
> +
> +*   Did you perform an exploratory manual test run of your code change and any
> +    related functionality?
> +
> +*   If the MP fixes a bug or implements a feature, are there accompanying unit
> +    and autopilot tests?
> +
> +*   Is the calendar app trunk buildable and runnable using Qtcreator?
> +
> +*   Was the debian changelog updated?
> +
> +*   Was the copyright years updated if necessary?
> +
> +The above checklist is more of a guideline to help calendar app trunk stay buildable,
> +stable and up to date.
> +
> +Note: As of vivid 15.04, Autopilot are broken in trunk. As such autopilot failures

Would "As of vivid 15.04, the autopilot tests are broken in trunk" be better here?

> +can be excused until they are fixed in trunk.
> 
> === added file 'README.unittest'
> --- README.unittest	1970-01-01 00:00:00 +0000
> +++ README.unittest	2015-12-12 17:42:02 +0000
> @@ -0,0 +1,39 @@
> +Running QML Unit Tests
> +======================
> +
> +QML Unit Tests help with testing the internal working of components while
> +autopilot tests help with testing the UI workflow as experience by the user.
> +Running QML tests is quite simple and very fast.
> +
> +If you are submitting your bugfix/patch to the calendar app, please follow the
> +following steps below to check whether that all tests pass before proposing a
> +merge request.
> +
> +* Branch the calendar app code, 
> +   $ bzr branch lp:ubuntu-calendar-app
> +
> +   $ mkdir builddir && cd builddir
> +   $ cmake ..  && cmake --build . -- -j 3

ANy reason to be using "cmake --build . -- -j3" over just running "make -j3" ? make -j3 is far less typing ;-p

> +   
> +Running all unit test at once
> +=============================
> +
> +If you want to run all tests, then run the following command from the builddir,
> +
> +   $ ctest --output-on-failure
> +   
> +If you want more verbose output, then run,
> +
> +   $ ctest -VV
> +
> +Running individual test cases,
> +==============================
> +
> +If you want to run testcases individually, you can do so by navigating to the
> +unit tests folder by,
> +
> +* Navigate to the tests/unit directory
> +   $ cd tests/unittests
> +   
> +* Run the test by providing their filenames
> +   $ qmltestrunner tst_date.qml


-- 
https://code.launchpad.net/~emailgirishrawat/ubuntu-calendar-app/READMEs/+merge/280392
Your team Ubuntu Calendar Developers is subscribed to branch lp:ubuntu-calendar-app.