ubuntu-touch-coreapps-reviewers team mailing list archive

[Victor Thompson <victor.thompson@xxxxxxxxx>]

Review: Needs Fixing

Hi Girish! Thanks again. I have a few inline comments for you.

Diff comments:

> === added file 'README-Autopilot.md'
> --- README-Autopilot.md	1970-01-01 00:00:00 +0000
> +++ README-Autopilot.md	2015-12-18 00:47:41 +0000
> @@ -0,0 +1,99 @@
> +Running Autopilot tests
> +=======================
> +
> +The Music 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 Music 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 music app, here are some useful links to help you:
> +
> +- [Ubuntu - Quality](http://developer.ubuntu.com/start/quality)
> +- [Autopilot - Python](https://developer.ubuntu.com/api/autopilot/python/1.5.0/)
> +
> +For help and options on running tests, see:
> +
> +- [Autopilot tests](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
> +
> +Running tests on the desktop
> +============================
> +
> +Using terminal:
> +
> +*  Branch the Music app code, for example,
> +
> +    $ bzr branch lp:music-app
> +    
> +*  Navigate to the tests/autopilot directory.
> +
> +    $ cd music-app/tests/autopilot
> +
> +*  run all tests.
> +
> +    $ autopilot3 run -vv music_app
> +
> +* to list all tests:
> +
> +    $ autopilot3 list music_app
> +
> + To run only one test (for instance:  music_app.tests.test_music.TestMainWindow.test_swipe_to_delete_song)
> +
> +    $ autopilot3 run -vv music_app.tests.test_music.TestMainWindow.test_swipe_to_delete_song
> +
> +* Debugging tests using autopilot vis
> +
> +    $ autopilot3 launch -i Qt qmlscene app/music-app.qml
> +
> +    $ autopilot3 vis
> +
> +Running tests using Ubuntu SDK
> +==============================
> +
> +Refer this [tutorial](https://developer.ubuntu.com/en/start/platform/guides/running-autopilot-tests/) to run tests on Ubuntu SDK: 
> +
> +Running tests on device or emulator:
> +====================================
> +
> +## Set up the system
> +
> +Prior to running tests on the device, follow the steps to find music in the system.
> +
> +    $ mv /home/phablet/Music /home/phablet/.Music
> +    $ restart mediascanner-2.0
> +
> +Using autopkg:
> +
> +*  Branch the Music app code, for example,
> +
> +    $ bzr branch lp:music-app
> +
> +*  Navigate to the source directory.
> +
> +    $ cd music-app
> +
> +*  Build a click package
> +    
> +    $ click-buddy .
> +
> +*  Run the tests on device (assumes only one click package in the directory)
> +
> +$ adt-run . *.click --- ssh -s adb -- -p <PASSWORD>

Please indent this line like the others.

> +
> +*  Resolving mediascanner2 schema issues when tests fail
> +
> +Occasionally the schema for the mediascanner2 (ms2) service is incremented and updates to the mocked ms2 database are required. The following steps should be taken to update the database and sql file when this happens.
> +
> +1. Dump a new schema file for the database for debugging:
> +   $ sqlite3 ~/.cache/mediascanner-2.0/mediastore.db .sch > tests/autopilot/music_app/content/mediascanner-2.0/mediastore.sch
> +
> +2. Edit the SQL file to use the new schemaVersion number:
> +   $ vi tests/autopilot/music_app/content/mediascanner-2.0/mediastore.sql
> +
> +3. If the tests still do not pass, execute the following bzr command to debug what has changed in the schema and update the SQL file to insert the values for each column in the 'media' table accordingly:
> +   $ bzr diff tests/autopilot/music_app/content/mediascanner-2.0/mediastore.sch
> +   $ vi tests/autopilot/music_app/content/mediascanner-2.0/mediastore.sql
> +
> 
> === added file 'README-Developers.md'
> --- README-Developers.md	1970-01-01 00:00:00 +0000
> +++ README-Developers.md	2015-12-18 00:47:41 +0000
> @@ -0,0 +1,82 @@
> +Building and running on Desktop
> +===============================
> +
> +Building and running the Ubuntu Music App is quite simple. You will require
> +Ubuntu 15.04 and higher to run on the desktop.
> +
> +   
> +
> +    $ bzr branch lp:music-app branch-name
> +    $ cd branch-name
> +    $ qmlscene app/music-app.qml
> +
> +Submitting a patch upstream
> +===========================
> +
> +If you want to submit a bug fix you can do so by branching the code as shown
> +above, implementing the fixes and running to see if it fixed the issue. We also
> +request that you run the Autopilot tests to check if anything
> +regressed due to the bug fix.
> +
> +If the tests fail, you will have to fix them before your bug fix can be
> +approved and merged into trunk. If the tests pass then commit and push your
> +code by,
> +
> +   
> +
> +    $ bzr commit -m "Implemented bug fix" --fixes lp:bug-number
> +    $ bzr push lp:~launchpadid/music-app/branch-name
> +
> +Running Tests
> +=============
> +
> +Please check README-Autopilot.md on how to run the tests.
> +They are quite explanatory and will help you get started.
> +
> +Code Style
> +==========
> +
> +We are trying to use a common code style throughout the code base to maintain
> +uniformity and improve code clarity. Listed below are the code styles guides
> +that will be followed based on the language used.
> +
> +* [QML](http://qt-project.org/doc/qt-5/qml-codingconventions.html) 
> +* [JS, C++](https://google-styleguide.googlecode.com/svn/trunk/cppguide.xml) 
> +* Python     - Code should follow PEP8 and Flake regulations
> +
> +Note: In the QML code convention, ignore the Javascript code section guidelines.
> +So the sections that should be taken into account in the QML conventions are QML 
> +Object Declarations, Grouped Properties and Lists.
> +
> +Debugging
> +=========
> + 
> +GDB allows one to see what is going on `inside' another program while it executes, 
> +or what another program was doing at the moment it crashed. It is a pretty niffty tool which allows you 
> +to get the crash log that can help a developer pin point the cause of the crash.
> +Before reproducing crash it is good to create symbols table for gdb, by using command:
> +
> +   
> +
> +    $ cd branch-name	
> +
> +To run GDB:
> +
> +   
> +
> +    $ gdb qmlscene
> +
> +At this point, you are inside the gdb prompt. Run your application as you normally would.
> +
> +     run app/music-app.qml

The use of the $ sign to signify that a command is entered at the terminal should be consistent in these cases.

> +
> +Your app is now running and monitored by GDB. Reproduce the steps in your app to make it crash. Once it does crash,
> +
> +     bt
> +
> +That's about it. To quit GDB, type quit to return back to the normal terminal console.
> +
> +     quit
> +
> +
> +
> 
> === added file 'README.md'
> --- README.md	1970-01-01 00:00:00 +0000
> +++ README.md	2015-12-18 00:47:41 +0000
> @@ -0,0 +1,25 @@
> +ReadMe - Ubuntu Music App
> +===========================
> +Ubuntu Music App is the official music app for Ubuntu Touch. We follow an open
> +source model where the code is available to anyone to branch and hack on. The
> +ubuntu music app follows a test driven development (TDD) where tests are
> +written in parallel to feature implementation to help spot regressions easier.
> +
> +Dependencies
> +============
> +**DEPENDENCIES ARE NEEDED TO BE INSTALLED TO BUILD AND RUN THE APP**.
> +
> +A complete list of dependencies for the project can be found in music-app/debian/control
> +
> +The following essential packages are also required to develop this app:
> +* [ubuntu-sdk](http://developer.ubuntu.com/start)
> +* intltool   - run  `$ sudo apt-get install intltool` 

Are you sure the $ should be added to this command? I don't think it should be included.

> +
> +Useful Links
> +============
> +Here are some useful links with regards to the Music App development.
> +
> +* [Home Page](https://developer.ubuntu.com/en/community/core-apps/music/)
> +* [Music App Wiki](https://wiki.ubuntu.com/Touch/CoreApps/Music)
> +* [Designs](https://docs.google.com/presentation/d/1L3eGhOe-0eEmKtUurthpUEaweFradSFb3t6W-KiIZMU/edit#slide=id.g3e005603e_420)
> +* [Project page](https://launchpad.net/music-app) 


-- 
https://code.launchpad.net/~emailgirishrawat/music-app/READMEs/+merge/280920
Your team Music App Developers is subscribed to branch lp:music-app.