notes-app

Merge lp:~elopio/notes-app/qml_tests into lp:notes-app

qml_tests
Merge into trunk

Proposed by Leo Arias on 2014-03-11

Status:	Work in progress
Proposed branch:	lp:~elopio/notes-app/qml_tests
Merge into:	lp:notes-app
Diff against target:	172 lines (+121/-23) 2 files modified tests/autopilot/notes_app/tests/test_expand_collapse.py (+1/-23) tests/qml/tst_NoteList.qml (+120/-0)
To merge this branch:	bzr merge lp:~elopio/notes-app/qml_tests
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Approve on 2014-03-11
Michael Zanetti (community)		2014-03-11	Needs Fixing on 2014-03-11
Review via email: mp+210346@code.launchpad.net

Commit message

Replaced the autopilot test_expand_and_collapse_many by qmlrunner tests.

lp:~elopio/notes-app/qml_tests updated on 2014-03-11

250. By Leo Arias on 2014-03-11: Updated the copyright.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/notes-app-ci/277/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/notes-app-ci/279/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Michael Zanetti (mzanetti) wrote on 2014-03-11:

Thanks soooo much for this. Maybe it marks a start for more reliable, faster and easier testing of our apps. I still haven't given up hope :)

Here's my feedback:

* test_note_starts_collapsed

looks mostly good. I for one would probably have made a loop to check all notes for being expanded. Also, add a "waitForRendering(noteList)" at the beginning of the test, or even better into the init() so that we're sure that nothing is animation directly at the start and the test doesn't run _before_ a note is wrongly expanded at start.

* test_click_note_must_expand_it

looks good, again, for completeness I'd probably add a

compare(noteList.contentItem.children[1].isExpanded, false, "Only note 0 should expand when clicking note 0")

* test_click_other_note_must_collapse_currently_expanded_note

It seems you revealed an issue in the Notes-app but unfortunately your test doesn't fail on it. So what happens is this:

the previous test expands a note by clicking on it, then cleanup() kicks in and sets isExpanded = false. This breaks the Note as after this it can't be expanded by clicking any more.

The reason your test doesn't catch it, is that you don't verify the precondition. You only check if the note 0 is collapsed in the end, but the problem is that it never expands.

Adding this in line 131 (of the diff) makes it fail and reveals the issue in the app.

tryCompare(initiallyExpandedNote, "isExpanded", true);

review: Needs Fixing

lp:~elopio/notes-app/qml_tests updated on 2014-03-11

251. By Leo Arias on 2014-03-11: Add waitForRendering on the init.
252. By Leo Arias on 2014-03-11: Added a check that only one note is expanded.
253. By Leo Arias on 2014-03-11: Click outside of the list to collapse the notes.
254. By Leo Arias on 2014-03-11: Just one click outside the list will collapse any item.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/notes-app-ci/282/rebuild

review: Approve (continuous-integration)

Revision history for this message

Michael Zanetti (mzanetti) wrote on 2014-03-11:

looking really good. Would approve this now. However, as this is for learning, here some more tips.

To make the test really test everything regarding collapsing, you could use the _data() helper here.

for example (non-tested pseudo-code):

function test_clickOtherNoteMustCollapseCurrentlyExpandedNote(data) {
var initiallyExpandedNote = noteList.contentItem.children[data.i];
...
}

This would then expand all the existing notes, be flexible enough to still work if you change the modelData and nicely print a PASS/FAIL for each entry in the mocked model.

===

113 + function clickOutsideNoteList() {
114 + mouseClick(noteList, noteList.width/2, noteList.height + 5);
115 + }

FYI: This would fail when the mock model grows to fill the whole window with notes. So maybe it would be good to change it to click into the empty space between the first and second note, or simply to the small white area at the left/right to make it always work.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/notes-app-ci/283/rebuild

review: Approve (continuous-integration)

lp:~elopio/notes-app/qml_tests updated on 2014-03-11

255. By Leo Arias on 2014-03-11: Click on top of the note list to collapse notes. Added a dummy data model.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/notes-app-ci/285/rebuild

review: Needs Fixing (continuous-integration)

lp:~elopio/notes-app/qml_tests updated on 2014-03-11

256. By Leo Arias on 2014-03-11: Use data for scenarios.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/notes-app-ci/286/rebuild

review: Approve (continuous-integration)

Revision history for this message

Leo Arias (elopio) wrote on 2014-07-10:

This still needs to run the tests during the build. TODO: ask if the app will be relevant next months.

Unmerged revisions

256. By Leo Arias on 2014-03-11: Use data for scenarios.
255. By Leo Arias on 2014-03-11: Click on top of the note list to collapse notes. Added a dummy data model.
254. By Leo Arias on 2014-03-11: Just one click outside the list will collapse any item.
253. By Leo Arias on 2014-03-11: Click outside of the list to collapse the notes.
252. By Leo Arias on 2014-03-11: Added a check that only one note is expanded.
251. By Leo Arias on 2014-03-11: Add waitForRendering on the init.
249. By Leo Arias on 2014-03-11: Replaced the autopilot test_expand_and_collapse_many by qmlrunner tests.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Leo Arias

Ubuntu Phablet Team

 === modified file 'tests/autopilot/notes_app/tests/test_expand_collapse.py'
 --- tests/autopilot/notes_app/tests/test_expand_collapse.py	2013-12-11 17:29:27 +0000
 +++ tests/autopilot/notes_app/tests/test_expand_collapse.py	2014-03-11 18:33:19 +0000
@@ -1,5 +1,5 @@
  # -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
--# Copyright 2013 Canonical
++# Copyright 2014 Canonical
+ #
  # 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
@@ -9,9 +9,6 @@
  from __future__ import absolute_import
--from testtools.matchers import Equals
--from autopilot.matchers import Eventually
--
  from notes_app.tests import NotesAppTestCase
  import sqlite3
@@ -44,25 +41,6 @@
          conn.commit()
          conn.close()
--    def test_expand_and_collapse_many(self):
--        notes = self.main_window.get_notes()
--        first = notes[0]
--        second = notes[1]
--
--        self.assert_note_eventually_collapsed(first)
--        self.assert_note_eventually_collapsed(second)
--
--        self.pointing_device.click_object(first)
--        self.assert_note_eventually_expanded(first)
--
--        self.pointing_device.click_object(second)
--        self.assert_note_eventually_collapsed(first)
--        self.assert_note_eventually_expanded(second)
--
--        self.pointing_device.click_object(first)
--        self.assert_note_eventually_expanded(first)
--        self.assert_note_eventually_collapsed(second)
--
      def test_collapse_header(self):
          first = self.main_window.get_notes()[0]
          header = self.main_window.get_header()
 === added directory 'tests/qml'
 === added file 'tests/qml/tst_NoteList.qml'
 --- tests/qml/tst_NoteList.qml	1970-01-01 00:00:00 +0000
 +++ tests/qml/tst_NoteList.qml	2014-03-11 18:33:19 +0000
@@ -0,0 +1,120 @@
++/*
++ * Copyright 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 as published by
++ * the Free Software Foundation; version 3.
++ *
++ * 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/>.
++ */
++
++import QtQuick 2.0
++import QtTest 1.0
++
++import '../../Components/'
++
++
++Item {
++    width: units.gu(40)
++    height: units.gu(40)
++
++    ListModel {
++        id: mockModel
++
++        ListElement {
++            note: 'Test0'
++            date: 'test date 0'
++        }
++        ListElement {
++            note: 'Test1'
++            date: 'test date 1'
++        }
++    }
++
++    ListModel {
++        // dummy data model.
++        id: dataModel
++
++        function touchNote(index, note) { }
++    }
++
++    NoteList {
++        id: noteList
++        anchors.fill: parent
++        model: mockModel
++    }
++
++    TestCase {
++        id: noteListTestCase
++        name: 'noteListTestCase'
++
++        when: windowShown
++
++        function init() {
++            waitForRendering(noteList);
++        }
++
++        function cleanup() {
++            // Make sure all the notes that were expanded are collapsed.
++            clickOutsideNotes();
++        }
++
++        function clickOutsideNotes() {
++            var spaceBetweenNotesHeight = noteList.contentItem.children[0].height
++                - noteList.contentItem.children[1].y;
++            mouseClick(noteList, noteList.width/2,
++                       noteList.contentItem.children[0].height - spaceBetweenNotesHeight / 2);
++        }
++
++        function test_noteStartsCollapsed_data() {
++            return getNotesScenarios()
++        }
++
++        function getNotesScenarios() {
++            var data = new Array();
++            for (var i = 0; i < noteList.count; i++) {
++                data.push({tag: "index" + i, index: i});
++            }
++            return data;
++        }
++
++        function test_noteStartsCollapsed(data) {
++            var note = noteList.contentItem.children[data.index];
++            compare(note.isExpanded, false);
++            tryCompare(note, 'height', units.gu(11));
++        }
++
++        function test_clickNoteMustExpandIt() {
++            var note = noteList.contentItem.children[0];
++            mouseClick(note, note.width/2, note.height/2);
++            compare(note.isExpanded, true);
++            tryCompare(note, 'height', units.gu(24));
++            compare(noteList.contentItem.children[1].isExpanded, false,
++                    'Only note 0 should expand when clicking note 0')
++        }
++
++        function test_clickOtherNoteMustCollapseCurrentlyExpandedNote_data() {
++            return getNotesScenarios()
++        }
++
++        function test_clickOtherNoteMustCollapseCurrentlyExpandedNote(data) {
++            var initiallyExpandedNote = noteList.contentItem.children[data.index];
++            mouseClick(initiallyExpandedNote, initiallyExpandedNote.width/2,
++                       initiallyExpandedNote.height/2);
++
++            var noteToExpand = noteList.contentItem.children[(data.index + 1) % noteList.count];
++
++            mouseClick(noteToExpand, noteToExpand.width/2, noteToExpand.height/2);
++
++            compare(initiallyExpandedNote.isExpanded, false);
++            compare(noteToExpand.isExpanded, true);
++        }
++
++    }
++}

notes-app

Merge lp:~elopio/notes-app/qml_tests into lp:notes-app

Commit message

Description of the change

Unmerged revisions

Preview Diff

Subscribers