Unignore and improve tests

matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/common/CryptoTestHelper.kt

@@ -203,17 +203,49 @@ class CryptoTestHelper(private val testHelper: CommonTestHelper) {
         val roomFromAlicePOV = aliceSession.getRoom(aliceRoomId)!!
 
         // Alice sends a message
-        testHelper.sendTextMessage(roomFromAlicePOV, messagesFromAlice[0], 1)
+        testHelper.sendTextMessage(roomFromAlicePOV, messagesFromAlice[0], 1).first().eventId.let { sentEventId ->
+            // ensure bob got it
+            ensureEventReceived(aliceRoomId, sentEventId, bobSession, true)
+        }
 
         // Bob send 3 messages
-        testHelper.sendTextMessage(roomFromBobPOV, messagesFromBob[0], 1)
+        testHelper.sendTextMessage(roomFromBobPOV, messagesFromBob[0], 1).first().eventId.let { sentEventId ->
-        testHelper.sendTextMessage(roomFromBobPOV, messagesFromBob[1], 1)
+            // ensure alice got it
-        testHelper.sendTextMessage(roomFromBobPOV, messagesFromBob[2], 1)
+            ensureEventReceived(aliceRoomId, sentEventId, aliceSession, true)
+        }
+
+        testHelper.sendTextMessage(roomFromBobPOV, messagesFromBob[1], 1).first().eventId.let { sentEventId ->
+            // ensure alice got it
+            ensureEventReceived(aliceRoomId, sentEventId, aliceSession, true)
+        }
+        testHelper.sendTextMessage(roomFromBobPOV, messagesFromBob[2], 1).first().eventId.let { sentEventId ->
+            // ensure alice got it
+            ensureEventReceived(aliceRoomId, sentEventId, aliceSession, true)
+        }
+
         // Alice sends a message
-        testHelper.sendTextMessage(roomFromAlicePOV, messagesFromAlice[1], 1)
+        testHelper.sendTextMessage(roomFromAlicePOV, messagesFromAlice[1], 1).first().eventId.let { sentEventId ->
+            // ensure bob got it
+            ensureEventReceived(aliceRoomId, sentEventId, bobSession, true)
+        }
         return cryptoTestData
     }
 
+    private fun ensureEventReceived(roomId: String, eventId: String, session: Session, andCanDecrypt : Boolean) {
+        testHelper.waitWithLatch { latch ->
+            testHelper.retryPeriodicallyWithLatch(latch) {
+                val timeLineEvent = session.getRoom(roomId)?.timelineService()?.getTimelineEvent(eventId)
+                if (andCanDecrypt) {
+                    timeLineEvent != null &&
+                            timeLineEvent.isEncrypted() &&
+                            timeLineEvent.root.getClearType() == EventType.MESSAGE
+                } else {
+                    timeLineEvent != null
+                }
+            }
+        }
+    }
+
     fun checkEncryptedEvent(event: Event, roomId: String, clearMessage: String, senderSession: Session) {
         assertEquals(EventType.ENCRYPTED, event.type)
         assertNotNull(event.content)
@@ -381,9 +413,9 @@ class CryptoTestHelper(private val testHelper: CommonTestHelper) {
 
         testHelper.waitWithLatch {
             testHelper.retryPeriodicallyWithLatch(it) {
-                    bobVerificationService.getExistingVerificationRequests(alice.myUserId).firstOrNull {
+                bobVerificationService.getExistingVerificationRequests(alice.myUserId).firstOrNull {
-                        it.requestInfo?.fromDevice == alice.sessionParams.deviceId
+                    it.requestInfo?.fromDevice == alice.sessionParams.deviceId
-                    } != null
+                } != null
             }
         }
         val incomingRequest = bobVerificationService.getExistingVerificationRequests(alice.myUserId).first {
@@ -411,7 +443,8 @@ class CryptoTestHelper(private val testHelper: CommonTestHelper) {
                 requestID!!,
                 roomId,
                 bob.myUserId,
-                bob.sessionParams.credentials.deviceId!!)
+                bob.sessionParams.credentials.deviceId!!
+        )
 
         // we should reach SHOW SAS on both
         var alicePovTx: OutgoingSasVerificationTransaction? = null

matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/keysbackup/KeysBackupTest.kt

@@ -56,18 +56,17 @@ import java.util.concurrent.CountDownLatch
 @LargeTest
 class KeysBackupTest : InstrumentedTest {
 
-    private val testHelper = CommonTestHelper(context())
-    private val cryptoTestHelper = CryptoTestHelper(testHelper)
-    private val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper)
-
     /**
      * - From doE2ETestWithAliceAndBobInARoomWithEncryptedMessages, we should have no backed up keys
      * - Check backup keys after having marked one as backed up
      * - Reset keys backup markers
      */
     @Test
-    @Ignore("This test will be ignored until it is fixed")
     fun roomKeysTest_testBackupStore_ok() {
+
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+
         val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoomWithEncryptedMessages()
 
         // From doE2ETestWithAliceAndBobInARoomWithEncryptedMessages, we should have no backed up keys
@@ -106,6 +105,10 @@ class KeysBackupTest : InstrumentedTest {
      */
     @Test
     fun prepareKeysBackupVersionTest() {
+
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+
         val bobSession = testHelper.createAccount(TestConstants.USER_BOB, KeysBackupTestConstants.defaultSessionParams)
 
         assertNotNull(bobSession.cryptoService().keysBackupService())
@@ -134,6 +137,9 @@ class KeysBackupTest : InstrumentedTest {
      */
     @Test
     fun createKeysBackupVersionTest() {
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+
         val bobSession = testHelper.createAccount(TestConstants.USER_BOB, KeysBackupTestConstants.defaultSessionParams)
         cryptoTestHelper.initializeCrossSigning(bobSession)
 
@@ -199,8 +205,11 @@ class KeysBackupTest : InstrumentedTest {
      * - Check the backup completes
      */
     @Test
-    @Ignore("This test will be ignored until it is fixed")
     fun backupAfterCreateKeysBackupVersionTest() {
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+        val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper)
+
         val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoomWithEncryptedMessages()
 
         keysBackupTestHelper.waitForKeybackUpBatching()
@@ -240,8 +249,11 @@ class KeysBackupTest : InstrumentedTest {
      * Check that backupAllGroupSessions() returns valid data
      */
     @Test
-    @Ignore("This test will be ignored until it is fixed")
     fun backupAllGroupSessionsTest() {
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+        val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper)
+
         val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoomWithEncryptedMessages()
 
         val keysBackup = cryptoTestData.firstSession.cryptoService().keysBackupService()
@@ -285,8 +297,11 @@ class KeysBackupTest : InstrumentedTest {
      * - Compare the decrypted megolm key with the original one
      */
     @Test
-    @Ignore("This test will be ignored until it is fixed")
     fun testEncryptAndDecryptKeysBackupData() {
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+        val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper)
+
         val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoomWithEncryptedMessages()
 
         val keysBackup = cryptoTestData.firstSession.cryptoService().keysBackupService() as DefaultKeysBackupService
@@ -329,8 +344,11 @@ class KeysBackupTest : InstrumentedTest {
      * - Restore must be successful
      */
     @Test
-    @Ignore("This test will be ignored until it is fixed")
     fun restoreKeysBackupTest() {
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+        val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper)
+
         val testData = keysBackupTestHelper.createKeysBackupScenarioWithPassword(null)
 
         // - Restore the e2e backup from the homeserver
@@ -414,8 +432,11 @@ class KeysBackupTest : InstrumentedTest {
      * - It must be trusted and must have with 2 signatures now
      */
     @Test
-    @Ignore("This test will be ignored until it is fixed")
     fun trustKeyBackupVersionTest() {
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+        val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper)
+
         // - Do an e2e backup to the homeserver with a recovery key
         // - And log Alice on a new device
         val testData = keysBackupTestHelper.createKeysBackupScenarioWithPassword(null)
@@ -474,8 +495,11 @@ class KeysBackupTest : InstrumentedTest {
      * - It must be trusted and must have with 2 signatures now
      */
     @Test
-    @Ignore("This test will be ignored until it is fixed")
     fun trustKeyBackupVersionWithRecoveryKeyTest() {
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+        val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper)
+
         // - Do an e2e backup to the homeserver with a recovery key
         // - And log Alice on a new device
         val testData = keysBackupTestHelper.createKeysBackupScenarioWithPassword(null)
@@ -532,8 +556,11 @@ class KeysBackupTest : InstrumentedTest {
      * - The backup must still be untrusted and disabled
      */
     @Test
-    @Ignore("This test will be ignored until it is fixed")
     fun trustKeyBackupVersionWithWrongRecoveryKeyTest() {
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+        val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper)
+
         // - Do an e2e backup to the homeserver with a recovery key
         // - And log Alice on a new device
         val testData = keysBackupTestHelper.createKeysBackupScenarioWithPassword(null)
@@ -574,8 +601,11 @@ class KeysBackupTest : InstrumentedTest {
      * - It must be trusted and must have with 2 signatures now
      */
     @Test
-    @Ignore("This test will be ignored until it is fixed")
     fun trustKeyBackupVersionWithPasswordTest() {
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+        val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper)
+
         val password = "Password"
 
         // - Do an e2e backup to the homeserver with a password
@@ -634,8 +664,11 @@ class KeysBackupTest : InstrumentedTest {
      * - The backup must still be untrusted and disabled
      */
     @Test
-    @Ignore("This test will be ignored until it is fixed")
     fun trustKeyBackupVersionWithWrongPasswordTest() {
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+        val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper)
+
         val password = "Password"
         val badPassword = "Bad Password"
 
@@ -675,8 +708,11 @@ class KeysBackupTest : InstrumentedTest {
      * - It must fail
      */
     @Test
-    @Ignore("This test will be ignored until it is fixed")
     fun restoreKeysBackupWithAWrongRecoveryKeyTest() {
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+        val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper)
+
         val testData = keysBackupTestHelper.createKeysBackupScenarioWithPassword(null)
 
         // - Try to restore the e2e backup with a wrong recovery key
@@ -709,8 +745,11 @@ class KeysBackupTest : InstrumentedTest {
      * - Restore must be successful
      */
     @Test
-    @Ignore("This test will be ignored until it is fixed")
     fun testBackupWithPassword() {
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+        val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper)
+
         val password = "password"
 
         val testData = keysBackupTestHelper.createKeysBackupScenarioWithPassword(password)
@@ -766,8 +805,11 @@ class KeysBackupTest : InstrumentedTest {
      * - It must fail
      */
     @Test
-    @Ignore("This test will be ignored until it is fixed")
     fun restoreKeysBackupWithAWrongPasswordTest() {
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+        val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper)
+
         val password = "password"
         val wrongPassword = "passw0rd"
 
@@ -803,8 +845,11 @@ class KeysBackupTest : InstrumentedTest {
      * - Restore must be successful
      */
     @Test
-    @Ignore("This test will be ignored until it is fixed")
     fun testUseRecoveryKeyToRestoreAPasswordBasedKeysBackup() {
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+        val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper)
+
         val password = "password"
 
         val testData = keysBackupTestHelper.createKeysBackupScenarioWithPassword(password)
@@ -833,8 +878,11 @@ class KeysBackupTest : InstrumentedTest {
      * - It must fail
      */
     @Test
-    @Ignore("This test will be ignored until it is fixed")
     fun testUsePasswordToRestoreARecoveryKeyBasedKeysBackup() {
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+        val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper)
+
         val testData = keysBackupTestHelper.createKeysBackupScenarioWithPassword(null)
 
         // - Try to restore the e2e backup with a password
@@ -865,8 +913,11 @@ class KeysBackupTest : InstrumentedTest {
      * - Check the returned KeysVersionResult is trusted
      */
     @Test
-    @Ignore("This test will be ignored until it is fixed")
     fun testIsKeysBackupTrusted() {
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+        val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper)
+
         // - Create a backup version
         val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoomWithEncryptedMessages()
 
@@ -901,66 +952,6 @@ class KeysBackupTest : InstrumentedTest {
         cryptoTestData.cleanUp(testHelper)
     }
 
-    /**
-     * Check backup starts automatically if there is an existing and compatible backup
-     * version on the homeserver.
-     * - Create a backup version
-     * - Restart alice session
-     * -> The new alice session must back up to the same version
-     */
-    @Test
-    @Ignore("This test will be ignored until it is fixed")
-    fun testCheckAndStartKeysBackupWhenRestartingAMatrixSession() {
-        // - Create a backup version
-        val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoomWithEncryptedMessages()
-
-        val keysBackup = cryptoTestData.firstSession.cryptoService().keysBackupService()
-
-        val stateObserver = StateObserver(keysBackup)
-
-        assertFalse(keysBackup.isEnabled)
-
-        val keyBackupCreationInfo = keysBackupTestHelper.prepareAndCreateKeysBackupData(keysBackup)
-
-        assertTrue(keysBackup.isEnabled)
-
-        // - Restart alice session
-        // - Log Alice on a new device
-        val aliceSession2 = testHelper.logIntoAccount(cryptoTestData.firstSession.myUserId, KeysBackupTestConstants.defaultSessionParamsWithInitialSync)
-
-        cryptoTestData.cleanUp(testHelper)
-
-        val keysBackup2 = aliceSession2.cryptoService().keysBackupService()
-
-        val stateObserver2 = StateObserver(keysBackup2)
-
-        // -> The new alice session must back up to the same version
-        val latch = CountDownLatch(1)
-        var count = 0
-        keysBackup2.addListener(object : KeysBackupStateListener {
-            override fun onStateChange(newState: KeysBackupState) {
-                // Check the backup completes
-                if (newState == KeysBackupState.ReadyToBackUp) {
-                    count++
-
-                    if (count == 2) {
-                        // Remove itself from the list of listeners
-                        keysBackup2.removeListener(this)
-
-                        latch.countDown()
-                    }
-                }
-            }
-        })
-        testHelper.await(latch)
-
-        assertEquals(keyBackupCreationInfo.version, keysBackup2.currentBackupVersion)
-
-        stateObserver.stopAndCheckStates(null)
-        stateObserver2.stopAndCheckStates(null)
-        testHelper.signOutAndClose(aliceSession2)
-    }
-
     /**
      * Check WrongBackUpVersion state
      *
@@ -971,6 +962,10 @@ class KeysBackupTest : InstrumentedTest {
      */
     @Test
     fun testBackupWhenAnotherBackupWasCreated() {
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+        val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper)
+
         // - Create a backup version
         val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoomWithEncryptedMessages()
 
@@ -1041,8 +1036,11 @@ class KeysBackupTest : InstrumentedTest {
      * -> It must success
      */
     @Test
-    @Ignore("This test will be ignored until it is fixed")
     fun testBackupAfterVerifyingADevice() {
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+        val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper)
+
         // - Create a backup version
         val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoomWithEncryptedMessages()
 
@@ -1075,6 +1073,8 @@ class KeysBackupTest : InstrumentedTest {
         // - Try to backup all in aliceSession2, it must fail
         val keysBackup2 = aliceSession2.cryptoService().keysBackupService()
 
+        assertFalse("Backup should not be enabled", keysBackup2.isEnabled)
+
         val stateObserver2 = StateObserver(keysBackup2)
 
         var isSuccessful = false
@@ -1092,8 +1092,8 @@ class KeysBackupTest : InstrumentedTest {
         assertFalse(isSuccessful)
 
         // Backup state must be NotTrusted
-        assertEquals(KeysBackupState.NotTrusted, keysBackup2.state)
+        assertEquals("Backup state must be NotTrusted", KeysBackupState.NotTrusted, keysBackup2.state)
-        assertFalse(keysBackup2.isEnabled)
+        assertFalse("Backup should not be enabled", keysBackup2.isEnabled)
 
         // - Validate the old device from the new one
         aliceSession2.cryptoService().setDeviceVerification(
@@ -1139,6 +1139,10 @@ class KeysBackupTest : InstrumentedTest {
      */
     @Test
     fun deleteKeysBackupTest() {
+        val testHelper = CommonTestHelper(context())
+        val cryptoTestHelper = CryptoTestHelper(testHelper)
+        val keysBackupTestHelper = KeysBackupTestHelper(testHelper, cryptoTestHelper)
+
         // - Create a backup version
         val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoomWithEncryptedMessages()
 

matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/keysbackup/KeysBackupTestHelper.kt

@@ -106,14 +106,14 @@ internal class KeysBackupTestHelper(
 
         Assert.assertNotNull(megolmBackupCreationInfo)
 
-        Assert.assertFalse(keysBackup.isEnabled)
+        Assert.assertFalse("Key backup should not be enabled before creation", keysBackup.isEnabled)
 
         // Create the version
         val keysVersion = testHelper.doSync<KeysVersion> {
             keysBackup.createKeysBackupVersion(megolmBackupCreationInfo, it)
         }
 
-        Assert.assertNotNull(keysVersion.version)
+        Assert.assertNotNull("Key backup version should not be null",keysVersion.version)
 
         // Backup must be enable now
         Assert.assertTrue(keysBackup.isEnabled)

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/keysbackup/DefaultKeysBackupService.kt

@@ -347,6 +347,11 @@ internal class DefaultKeysBackupService @Inject constructor(
 
     override fun backupAllGroupSessions(progressListener: ProgressListener?,
                                         callback: MatrixCallback<Unit>?) {
+
+        if (!isEnabled || backupOlmPkEncryption == null || keysBackupVersion == null) {
+            callback?.onFailure(Throwable("Backup not enabled"))
+            return
+        }
         // Get a status right now
         getBackupProgress(object : ProgressListener {
             override fun onProgress(progress: Int, total: Int) {
@@ -1258,7 +1263,6 @@ internal class DefaultKeysBackupService @Inject constructor(
             Timber.v("backupKeys: Invalid configuration")
             backupAllGroupSessionsCallback?.onFailure(IllegalStateException("Invalid configuration"))
             resetBackupAllGroupSessionsListeners()
-
             return
         }