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

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
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.

To post a comment you must log in.
lp:~elopio/notes-app/qml_tests updated on 2014-03-11
250. By Leo Arias on 2014-03-11

Updated the copyright.

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Michael Zanetti (mzanetti) wrote :

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.

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Michael Zanetti (mzanetti) wrote :

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 data = new Array();
    for (var i = 0; i < notesList.count; i++) {
        data.push({tag: "index" + i, index: i}
    }
    return
}

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.

PS Jenkins bot (ps-jenkins) wrote :
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.

PS Jenkins bot (ps-jenkins) wrote :
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.

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Leo Arias (elopio) wrote :

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.

250. By Leo Arias on 2014-03-11

Updated the copyright.

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
1=== modified file 'tests/autopilot/notes_app/tests/test_expand_collapse.py'
2--- tests/autopilot/notes_app/tests/test_expand_collapse.py 2013-12-11 17:29:27 +0000
3+++ tests/autopilot/notes_app/tests/test_expand_collapse.py 2014-03-11 18:33:19 +0000
4@@ -1,5 +1,5 @@
5 # -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
6-# Copyright 2013 Canonical
7+# Copyright 2014 Canonical
8 #
9 # This program is free software: you can redistribute it and/or modify it
10 # under the terms of the GNU General Public License version 3, as published
11@@ -9,9 +9,6 @@
12
13 from __future__ import absolute_import
14
15-from testtools.matchers import Equals
16-from autopilot.matchers import Eventually
17-
18 from notes_app.tests import NotesAppTestCase
19
20 import sqlite3
21@@ -44,25 +41,6 @@
22 conn.commit()
23 conn.close()
24
25- def test_expand_and_collapse_many(self):
26- notes = self.main_window.get_notes()
27- first = notes[0]
28- second = notes[1]
29-
30- self.assert_note_eventually_collapsed(first)
31- self.assert_note_eventually_collapsed(second)
32-
33- self.pointing_device.click_object(first)
34- self.assert_note_eventually_expanded(first)
35-
36- self.pointing_device.click_object(second)
37- self.assert_note_eventually_collapsed(first)
38- self.assert_note_eventually_expanded(second)
39-
40- self.pointing_device.click_object(first)
41- self.assert_note_eventually_expanded(first)
42- self.assert_note_eventually_collapsed(second)
43-
44 def test_collapse_header(self):
45 first = self.main_window.get_notes()[0]
46 header = self.main_window.get_header()
47
48=== added directory 'tests/qml'
49=== added file 'tests/qml/tst_NoteList.qml'
50--- tests/qml/tst_NoteList.qml 1970-01-01 00:00:00 +0000
51+++ tests/qml/tst_NoteList.qml 2014-03-11 18:33:19 +0000
52@@ -0,0 +1,120 @@
53+/*
54+ * Copyright 2014 Canonical Ltd.
55+ *
56+ * This program is free software; you can redistribute it and/or modify
57+ * it under the terms of the GNU General Public License as published by
58+ * the Free Software Foundation; version 3.
59+ *
60+ * This program is distributed in the hope that it will be useful,
61+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
62+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
63+ * GNU General Public License for more details.
64+ *
65+ * You should have received a copy of the GNU General Public License
66+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
67+ */
68+
69+import QtQuick 2.0
70+import QtTest 1.0
71+
72+import '../../Components/'
73+
74+
75+Item {
76+ width: units.gu(40)
77+ height: units.gu(40)
78+
79+ ListModel {
80+ id: mockModel
81+
82+ ListElement {
83+ note: 'Test0'
84+ date: 'test date 0'
85+ }
86+ ListElement {
87+ note: 'Test1'
88+ date: 'test date 1'
89+ }
90+ }
91+
92+ ListModel {
93+ // dummy data model.
94+ id: dataModel
95+
96+ function touchNote(index, note) { }
97+ }
98+
99+ NoteList {
100+ id: noteList
101+ anchors.fill: parent
102+ model: mockModel
103+ }
104+
105+ TestCase {
106+ id: noteListTestCase
107+ name: 'noteListTestCase'
108+
109+ when: windowShown
110+
111+ function init() {
112+ waitForRendering(noteList);
113+ }
114+
115+ function cleanup() {
116+ // Make sure all the notes that were expanded are collapsed.
117+ clickOutsideNotes();
118+ }
119+
120+ function clickOutsideNotes() {
121+ var spaceBetweenNotesHeight = noteList.contentItem.children[0].height
122+ - noteList.contentItem.children[1].y;
123+ mouseClick(noteList, noteList.width/2,
124+ noteList.contentItem.children[0].height - spaceBetweenNotesHeight / 2);
125+ }
126+
127+ function test_noteStartsCollapsed_data() {
128+ return getNotesScenarios()
129+ }
130+
131+ function getNotesScenarios() {
132+ var data = new Array();
133+ for (var i = 0; i < noteList.count; i++) {
134+ data.push({tag: "index" + i, index: i});
135+ }
136+ return data;
137+ }
138+
139+ function test_noteStartsCollapsed(data) {
140+ var note = noteList.contentItem.children[data.index];
141+ compare(note.isExpanded, false);
142+ tryCompare(note, 'height', units.gu(11));
143+ }
144+
145+ function test_clickNoteMustExpandIt() {
146+ var note = noteList.contentItem.children[0];
147+ mouseClick(note, note.width/2, note.height/2);
148+ compare(note.isExpanded, true);
149+ tryCompare(note, 'height', units.gu(24));
150+ compare(noteList.contentItem.children[1].isExpanded, false,
151+ 'Only note 0 should expand when clicking note 0')
152+ }
153+
154+ function test_clickOtherNoteMustCollapseCurrentlyExpandedNote_data() {
155+ return getNotesScenarios()
156+ }
157+
158+ function test_clickOtherNoteMustCollapseCurrentlyExpandedNote(data) {
159+ var initiallyExpandedNote = noteList.contentItem.children[data.index];
160+ mouseClick(initiallyExpandedNote, initiallyExpandedNote.width/2,
161+ initiallyExpandedNote.height/2);
162+
163+ var noteToExpand = noteList.contentItem.children[(data.index + 1) % noteList.count];
164+
165+ mouseClick(noteToExpand, noteToExpand.width/2, noteToExpand.height/2);
166+
167+ compare(initiallyExpandedNote.isExpanded, false);
168+ compare(noteToExpand.isExpanded, true);
169+ }
170+
171+ }
172+}

Subscribers

People subscribed via source and target branches

to all changes: