Code review comment for lp:~elopio/notes-app/qml_tests

Revision history for this message
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

« Back to merge proposal