Merge lp:~boiko/messaging-app/rtm-fix_1373479 into lp:messaging-app/rtm-14.09

[Gustavo Pichorim Boiko]

Scroll new messages into view.

Scroll new messages into view.

[Bill Filler (bfiller)]

if you scroll up the thread view and then recv a new message, it does autoscroll to the new message but the new message is repeated twice. Scrolling it out of view and back then corrects it to only show once.

review: Needs Fixing

[lp:~boiko/messaging-app/rtm-fix_1373479]

211. By Gustavo Pichorim Boiko on 2015-02-18

Use the prepopulated database to the test.

=== modified file 'src/qml/Messages.qml'
--- src/qml/Messages.qml	2015-01-23 20:11:21 +0000
+++ src/qml/Messages.qml	2015-02-18 12:48:57 +0000
@@ -50,7 +50,7 @@
     property alias contactWatcher: contactWatcherInternal
     property string lastFilter: ""
     property string text: ""
-    property bool pendingMessage: false
+    property string latestEventId: ""
     property string scrollToEventId: ""
     property bool isSearching: scrollToEventId !== ""
 
@@ -542,6 +542,13 @@
         }
     ]
 
+    Timer {
+        id: scrollTimer
+        repeat: false
+        interval: 1
+        onTriggered: messageList.positionViewAtBeginning()
+    }
+
     HistoryEventModel {
         id: eventModel
         type: HistoryThreadModel.EventTypeText
@@ -551,9 +558,18 @@
            sortOrder: HistorySort.DescendingOrder
         }
         onCountChanged: {
-            if (pendingMessage) {
-                pendingMessage = false
-                messageList.positionViewAtBeginning()
+            if (count == 0) {
+                latestEventId = ""
+                return
+            }
+            if (latestEventId == "") {
+                latestEventId = eventModel.get(0).eventId
+            } else if (latestEventId != eventModel.get(0).eventId) {
+                latestEventId = eventModel.get(0).eventId
+                // FIXME: if we scroll the ListView here, the view will end up
+                // with duplicate delegates, so we postpone the scrolling to give the model
+                // some time to finish moving items around
+                scrollTimer.start()
             }
             if (isSearching) {
                 // if we ask for more items manually listview will stop working,
@@ -951,14 +967,12 @@
                         attachment.push(item.filePath)
                         newAttachments.push(attachment)
                     }
-                    pendingMessage = true
                     chatManager.sendMMS(participants, textEntry.text, newAttachments, messages.account.accountId)
                     textEntry.text = ""
                     attachments.clear()
                     return
                 }
 
-                pendingMessage = true
                 chatManager.sendMessage(participants, textEntry.text, messages.account.accountId)
                 textEntry.text = ""
             }

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2014-07-22 02:01:38 +0000
+++ tests/CMakeLists.txt	2015-02-18 12:48:57 +0000
@@ -2,6 +2,11 @@
 
 set(AUTOPILOT_DIR autopilot/messaging_app)
 
+file(GLOB AUTOPILOT_PY_FILES ${AUTOPILOT_DIR}/*.py ${AUTOPILOT_DIR}/tests/*.py)
+
+# custom target to get autopilot files visible on QtCreator
+add_custom_target(messaging_app_AUTOPILOT_PY_FILES ALL SOURCES ${AUTOPILOT_PY_FILES})
+
 execute_process(COMMAND python3 -c "from distutils.sysconfig import get_python_lib; print(get_python_lib())"
     OUTPUT_VARIABLE PYTHON_PACKAGE_DIR OUTPUT_STRIP_TRAILING_WHITESPACE)
 

=== modified file 'tests/autopilot/messaging_app/emulators.py'
--- tests/autopilot/messaging_app/emulators.py	2014-12-16 02:13:31 +0000
+++ tests/autopilot/messaging_app/emulators.py	2015-02-18 12:48:57 +0000
@@ -433,8 +433,7 @@
 
     def get_messages_count(self):
         """Return the number of meesages."""
-        return self.wait_select_single(
-            'MessagesListView', objectName='messageList').count
+        return self.get_list_view().count
 
     @autopilot_logging.log_action(logger.info)
     def select_messages(self, *indexes):
@@ -449,6 +448,14 @@
             message_delegate = self._get_message_delegate(index)
             self.pointing_device.click_object(message_delegate)
 
+    def scroll_list(self):
+        list_view = self.get_list_view()
+        x, y, width, height = list_view.globalRect
+        start_x = stop_x = x + (width / 2)
+        start_y = y + 10
+        stop_y = y + height - 10
+        self.pointing_device.drag(start_x, start_y, stop_x, stop_y)
+
     def _get_message_delegate(self, index):
         return self.wait_select_single(
             'MessageDelegateFactory', objectName='message{}'.format(index))
@@ -486,6 +493,11 @@
         return self.wait_select_single(
             'TextArea', objectName='messageTextArea').text
 
+    def get_list_view(self):
+        """Returns the messages list view"""
+        return self.wait_select_single(
+            'MessagesListView', objectName='messageList')
+
 
 class ListItemWithActions(_common.UbuntuUIToolkitCustomProxyObjectBase):
 

=== modified file 'tests/autopilot/messaging_app/fixture_setup.py'
--- tests/autopilot/messaging_app/fixture_setup.py	2015-02-18 12:48:57 +0000
+++ tests/autopilot/messaging_app/fixture_setup.py	2015-02-18 12:48:57 +0000
@@ -23,61 +23,65 @@
     def setUp(self):
         super(MessagingTestEnvironment, self).setUp()
         self.useFixture(OfonoPhoneSIM())
-        self.useFixture(BackupHistory())
         if self.use_testdata_db:
             self.useFixture(FillCustomSmsHistory())
+        else:
+            self.useFixture(UseEmptySmsHistory())
         self.useFixture(RespawnService())
 
 
-class BackupHistory(fixtures.Fixture):
-
-    def setUp(self):
-        super(BackupHistory, self).setUp()
-        self.addCleanup(self._restore_history)
-        self._backup_history()
-
-    def _backup_history(self):
-        self.history = os.path.expanduser(
-            '~/.local/share/history-service/history.sqlite')
-        if os.path.exists(self.history):
-            os.rename(self.history, self.history + '.orig')
-
-    def _restore_history(self):
-        try:
-            os.unlink(self.history)
-        except OSError:
-            pass
-        if os.path.exists(self.history + '.orig'):
-            os.rename(self.history + '.orig', self.history)
-
-
 class FillCustomSmsHistory(fixtures.Fixture):
 
     history_service_dir = os.path.expanduser("~/.local/share/history-service/")
     history_db = "history.sqlite"
     testdata_sys = "/usr/lib/python3/dist-packages/messaging_app/testdata/"
     testdata_local = "messaging_app/testdata/"
+    database_path = '/tmp/' + history_db
 
     prefilled_history_local = os.path.join(testdata_local, history_db)
     prefilled_history_system = os.path.join(testdata_sys, history_db)
 
     def setUp(self):
         super(FillCustomSmsHistory, self).setUp()
-        self.addCleanup(self._clear_test_data())
-        self._copy_test_data_history()
+        self.addCleanup(self._clear_test_data)
+        self.addCleanup(self._kill_service_to_respawn)
+        self._clear_test_data()
+        self._prepare_history_data()
+        self._kill_service_to_respawn()
+        self._start_service_with_custom_data()
 
-    def _copy_test_data_history(self):
+    def _prepare_history_data(self):
         if os.path.exists(self.prefilled_history_local):
-            shutil.copy(
-                self.prefilled_history_local, self.history_service_dir)
+            shutil.copy(self.prefilled_history_local, self.database_path)
         else:
-            shutil.copy(
-                self.prefilled_history_system, self.history_service_dir)
-
-    def _clear_test_data(self):
-        test_data = os.path.join(self.history_service_dir, self.history_db)
-        if os.path.exists(test_data):
-            os.remove(test_data)
+            shutil.copy(self.prefilled_history_system, self.database_path)
+
+    def _clear_test_data(self):
+        if os.path.exists(self.database_path):
+            os.remove(self.database_path)
+
+    def _kill_service_to_respawn(self):
+        subprocess.call(['pkill', 'history-daemon'])
+
+    def _start_service_with_custom_data(self):
+        os.environ['HISTORY_SQLITE_DBPATH'] = self.database_path
+        with open(os.devnull, 'w') as devnull:
+            subprocess.Popen(['history-daemon'], stderr=devnull)
+
+
+class UseEmptySmsHistory(FillCustomSmsHistory):
+    database_path = ':memory:'
+
+    def setUp(self):
+        super(UseEmptySmsHistory, self).setUp()
+
+    def _prepare_history_data(self):
+        # just avoid doing anything
+        self.database_path = ':memory:'
+
+    def _clear_test_data(self):
+        # don't do anything
+        self.database_path = ''
 
 
 class RespawnService(fixtures.Fixture):
@@ -88,8 +92,10 @@
         self._kill_services_to_respawn()
 
     def _kill_services_to_respawn(self):
-        subprocess.call(['pkill', 'history-daemon'])
         subprocess.call(['pkill', '-f', 'telephony-service-handler'])
+        # on desktop, notify-osd may generate persistent popups (like for "SMS
+        # received"), don't make that stay around for the tests
+        subprocess.call(['pkill', '-f', 'notify-osd'])
 
 
 class OfonoPhoneSIM(fixtures.Fixture):

=== modified file 'tests/autopilot/messaging_app/testdata/history.sqlite'
Binary files tests/autopilot/messaging_app/testdata/history.sqlite	2015-02-18 12:48:57 +0000 and tests/autopilot/messaging_app/testdata/history.sqlite	2015-02-18 12:48:57 +0000 differ
=== modified file 'tests/autopilot/messaging_app/tests/test_messaging.py'
--- tests/autopilot/messaging_app/tests/test_messaging.py	2015-02-18 12:48:57 +0000
+++ tests/autopilot/messaging_app/tests/test_messaging.py	2015-02-18 12:48:57 +0000
@@ -11,7 +11,6 @@
 
 from __future__ import absolute_import
 
-import subprocess
 import time
 
 from autopilot.matchers import Eventually
@@ -39,13 +38,6 @@
         self.assertThat(self.thread_list.visible, Equals(True))
         self.assertThat(self.thread_list.count, Equals(0))
 
-    def tearDown(self):
-        super(BaseMessagingTestCase, self).tearDown()
-
-        # on desktop, notify-osd may generate persistent popups (like for "SMS
-        # received"), don't make that stay around for the tests
-        subprocess.call(['pkill', '-f', 'notify-osd'])
-
 
 class TestMessaging(BaseMessagingTestCase):
     """Tests for the communication panel."""
@@ -301,19 +293,22 @@
         self.assertThat(list_view.count, Eventually(Equals(0)))
 
 
-class MessagingTestCaseWithExistingThread(BaseMessagingTestCase):
+class MessagingTestCaseWithExistingThread(MessagingAppTestCase):
 
     def setUp(self):
+        test_setup = fixture_setup.MessagingTestEnvironment(
+            use_testdata_db=True)
+        self.useFixture(test_setup)
+
         super(MessagingTestCaseWithExistingThread, self).setUp()
         self.main_page = self.main_view.select_single(emulators.MainPage)
-        self.number = '5555559876'
-        self.messages = self.receive_messages()
+        self.number = '08154'
 
-    def receive_messages(self):
-        # send 3 messages. Reversed because on the QML, the one with the
-        # 0 index is the latest received.
+    def receive_messages(self, count=3):
+        # send the required number of messages. Reversed because on the QML,
+        # the one with the 0 index is the latest received.
         messages = []
-        message_indexes = list(reversed(range(3)))
+        message_indexes = list(reversed(range(count)))
         for index in message_indexes:
             message_text = 'test message {}'.format(index)
             helpers.receive_sms(
@@ -322,28 +317,40 @@
             # Prepend to make sure that the indexes match.
             messages.insert(0, message_text)
         # Wait for the thread.
-        self.assertThat(
-            self.main_page.get_thread_count, Eventually(Equals(1)))
+        self.main_page.get_thread_from_number(self.number)
         return messages
 
     def test_delete_multiple_messages(self):
         """Verify we can delete multiple messages"""
         messages_page = self.main_page.open_thread(self.number)
+        messages = messages_page.get_messages()
 
         self.main_view.enable_messages_selection_mode()
         messages_page.select_messages(1, 2)
 
-        # Wait a few seconds before clicking the header
-        # to make sure the OSD is already gone
-        # FIXME: check if there is a better way to detect OSD visibility
-        time.sleep(10)
         self.main_view.click_messages_header_delete()
 
         remaining_messages = messages_page.get_messages()
         self.assertThat(remaining_messages, HasLength(1))
-        _, remaining_message_text = remaining_messages[0]
+        remaining_message = remaining_messages[0]
         self.assertEqual(
-            remaining_message_text, self.messages[0])
+            remaining_message, messages[0])
+
+    def test_scroll_to_new_message(self):
+        """Verify that the view is scrolled to display a new message"""
+        # use the number of an existing thread to avoid OSD problems
+        self.number = '08155'
+        messages_page = self.main_page.open_thread(self.number)
+
+        # scroll the list to display older messages
+        messages_page.scroll_list()
+
+        # now receive a new message
+        self.receive_messages(1)
+
+        # and make sure that the list gets scrolled to the new message again
+        list_view = messages_page.get_list_view()
+        self.assertThat(list_view.atYEnd, Eventually(Equals(True)))
 
 
 class MessagingTestSearch(MessagingAppTestCase):
@@ -352,6 +359,7 @@
         test_setup = fixture_setup.MessagingTestEnvironment(
             use_testdata_db=True)
         self.useFixture(test_setup)
+
         super(MessagingTestSearch, self).setUp()
         self.thread_list = self.app.select_single(objectName='threadList')
 
@@ -380,7 +388,7 @@
         text_field.clear()
         text_field.write('Ubuntu1')
         self.assertThat(
-            lambda: count_visible_threads(threads), Eventually(Equals(1)))
+            lambda: count_visible_threads(threads), Eventually(Equals(2)))
 
         text_field.clear()
         text_field.write('Ubuntu')
@@ -388,6 +396,11 @@
             lambda: count_visible_threads(threads), Eventually(Equals(5)))
 
         text_field.clear()
+        text_field.write('08154')
+        self.assertThat(
+            lambda: count_visible_threads(threads), Eventually(Equals(1)))
+
+        text_field.clear()
         text_field.write('%')
         # as we are testing for items NOT to appear, there is no other way
         # other than sleeping for awhile before checking if the threads are