From a2b84ac6177b6769571214b932687b71419e5c53 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 12 Jul 2023 09:40:02 +0200 Subject: [PATCH 1/8] Ensure CI run all the tests. There were some failing tests, but the CI does not see it. It seems that koverMergedReport does not run all the tests (?). --- .github/workflows/nightlyReports.yml | 2 +- .github/workflows/tests.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/nightlyReports.yml b/.github/workflows/nightlyReports.yml index 5cd92d6cb1..4805bf44d1 100644 --- a/.github/workflows/nightlyReports.yml +++ b/.github/workflows/nightlyReports.yml @@ -27,7 +27,7 @@ jobs: java-version: '17' - name: ⚙️ Run unit & screenshot tests, generate kover report - run: ./gradlew koverMergedReport $CI_GRADLE_ARG_PROPERTIES -Pci-build=true + run: ./gradlew test koverMergedReport $CI_GRADLE_ARG_PROPERTIES -Pci-build=true - name: ✅ Upload kover report if: always() diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 2926159a00..44fdf58323 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -38,7 +38,7 @@ jobs: cache-read-only: ${{ github.ref != 'refs/heads/develop' }} - name: ⚙️ Run unit & screenshot tests, generate kover report - run: ./gradlew koverMergedReport $CI_GRADLE_ARG_PROPERTIES -Pci-build=true + run: ./gradlew test koverMergedReport $CI_GRADLE_ARG_PROPERTIES -Pci-build=true - name: 📈 Verify coverage run: ./gradlew koverMergedVerify $CI_GRADLE_ARG_PROPERTIES -Pci-build=true From af520ddc0071a927b2c37bbfae07410e028d1f2e Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 12 Jul 2023 09:43:18 +0200 Subject: [PATCH 2/8] Fix failing test. Code is now aligned with the comment. --- .../android/libraries/core/extensions/BasicExtensions.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/core/src/main/kotlin/io/element/android/libraries/core/extensions/BasicExtensions.kt b/libraries/core/src/main/kotlin/io/element/android/libraries/core/extensions/BasicExtensions.kt index 562f62de6d..db07432df0 100644 --- a/libraries/core/src/main/kotlin/io/element/android/libraries/core/extensions/BasicExtensions.kt +++ b/libraries/core/src/main/kotlin/io/element/android/libraries/core/extensions/BasicExtensions.kt @@ -64,7 +64,7 @@ fun String?.insertBeforeLast(insert: String, delimiter: String = "."): String { * Throws if length is < 1. */ fun String.ellipsize(length: Int): String { - require(length > 1) + require(length >= 1) if (this.length <= length) { return this From e85de6b3007b9f5edfe0dfc5b5fe48aeed1d477e Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 12 Jul 2023 09:59:52 +0200 Subject: [PATCH 3/8] Rework DeeplinkParser to fix a test (and fix a bug in release mode). The test was failing in release mode because there is not check on `RoomId` format, so INVITE_LIST value ("invites") is seen as a valid RoomId. First check for known paths, then try to parse as RoomId. The tryOrNull will return null only in debug mode, so I think we can remove it. Error was: value of: getFromIntent(...) expected: InviteList(sessionId=@alice:server.org) but was : Room(sessionId=@alice:server.org, roomId=invites, threadId=null) at io.element.android.libraries.deeplink.DeeplinkParserTest.nominal cases(DeeplinkParserTest.kt:54) --- .../libraries/deeplink/DeeplinkParser.kt | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/libraries/deeplink/src/main/kotlin/io/element/android/libraries/deeplink/DeeplinkParser.kt b/libraries/deeplink/src/main/kotlin/io/element/android/libraries/deeplink/DeeplinkParser.kt index 7d2c8af135..7a5f9d5772 100644 --- a/libraries/deeplink/src/main/kotlin/io/element/android/libraries/deeplink/DeeplinkParser.kt +++ b/libraries/deeplink/src/main/kotlin/io/element/android/libraries/deeplink/DeeplinkParser.kt @@ -18,7 +18,6 @@ package io.element.android.libraries.deeplink import android.content.Intent import android.net.Uri -import io.element.android.libraries.core.data.tryOrNull import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.matrix.api.core.ThreadId @@ -37,21 +36,15 @@ class DeeplinkParser @Inject constructor() { if (host != HOST) return null val pathBits = path.orEmpty().split("/").drop(1) val sessionId = pathBits.elementAtOrNull(0)?.let(::SessionId) ?: return null - val screenPathComponent = pathBits.elementAtOrNull(1) - val roomId = tryOrNull { screenPathComponent?.let(::RoomId) } - return when { - roomId != null -> { + return when (val screenPathComponent = pathBits.elementAtOrNull(1)) { + null -> DeeplinkData.Root(sessionId) + DeepLinkPaths.INVITE_LIST -> DeeplinkData.InviteList(sessionId) + else -> { + val roomId = screenPathComponent.let(::RoomId) val threadId = pathBits.elementAtOrNull(2)?.let(::ThreadId) DeeplinkData.Room(sessionId, roomId, threadId) } - screenPathComponent == DeepLinkPaths.INVITE_LIST -> { - DeeplinkData.InviteList(sessionId) - } - screenPathComponent == null -> { - DeeplinkData.Root(sessionId) - } - else -> null } } } From 67fd2ebba95ef6cb53502c14e3c4927293f24003 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 12 Jul 2023 14:11:26 +0200 Subject: [PATCH 4/8] Fix warning (rename the base parameter name). --- .../io/element/android/libraries/matrix/api/room/MatrixRoom.kt | 2 +- .../android/libraries/matrix/test/room/FakeMatrixRoom.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt index 3ad391ead8..05dfda714c 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt @@ -86,7 +86,7 @@ interface MatrixRoom : Closeable { suspend fun toggleReaction(emoji: String, eventId: EventId): Result - suspend fun forwardEvent(eventId: EventId, rooms: List): Result + suspend fun forwardEvent(eventId: EventId, roomIds: List): Result suspend fun retrySendMessage(transactionId: String): Result diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt index 9392fb7985..3f265f392a 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt @@ -239,7 +239,7 @@ class FakeMatrixRoom( override suspend fun sendFile(file: File, fileInfo: FileInfo, progressCallback: ProgressCallback?): Result = fakeSendMedia(progressCallback) - override suspend fun forwardEvent(eventId: EventId, rooms: List): Result = simulateLongTask { + override suspend fun forwardEvent(eventId: EventId, roomIds: List): Result = simulateLongTask { forwardEventResult } From 92f5c96936be6a509c61c91fb47a553f42b1dbb2 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 12 Jul 2023 14:14:44 +0200 Subject: [PATCH 5/8] Use the param (bad copy paste) --- .../libraries/push/impl/notifications/NotificationFactory.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationFactory.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationFactory.kt index 0addc1276a..9dd6c7d4cd 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationFactory.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationFactory.kt @@ -123,7 +123,7 @@ class NotificationFactory @Inject constructor( val roomMeta = roomNotifications.filterIsInstance().map { it.meta } val invitationMeta = invitationNotifications.filterIsInstance().map { it.meta } val simpleMeta = simpleNotifications.filterIsInstance().map { it.meta } - val fallbackMeta = simpleNotifications.filterIsInstance().map { it.meta } + val fallbackMeta = fallbackNotifications.filterIsInstance().map { it.meta } return when { roomMeta.isEmpty() && invitationMeta.isEmpty() && simpleMeta.isEmpty() -> SummaryNotification.Removed else -> SummaryNotification.Update( From 19fc90385c8abd8846274387639b51bcb0b62370 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 12 Jul 2023 14:15:25 +0200 Subject: [PATCH 6/8] Fix another warning. --- .../features/messages/forward/ForwardMessagesPresenterTests.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/forward/ForwardMessagesPresenterTests.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/forward/ForwardMessagesPresenterTests.kt index 82526e100d..502305ab1d 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/forward/ForwardMessagesPresenterTests.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/forward/ForwardMessagesPresenterTests.kt @@ -65,7 +65,6 @@ class ForwardMessagesPresenterTests { }.test { val initialState = awaitItem() skipItems(1) - val summary = aRoomSummaryDetail() initialState.eventSink(ForwardMessagesEvents.ToggleSearchActive) assertThat(awaitItem().isSearchActive).isTrue() From c8912060fb349bd4e7087f7844997c26aa67e542 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 12 Jul 2023 14:16:10 +0200 Subject: [PATCH 7/8] Fix another warning. --- .../eventformatter/impl/RoomMembershipContentFormatter.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/RoomMembershipContentFormatter.kt b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/RoomMembershipContentFormatter.kt index f8ac3b491c..6a65a9bd1e 100644 --- a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/RoomMembershipContentFormatter.kt +++ b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/RoomMembershipContentFormatter.kt @@ -34,7 +34,7 @@ class RoomMembershipContentFormatter @Inject constructor( ): CharSequence? { val userId = membershipContent.userId val memberIsYou = matrixClient.isMe(userId) - return when (val change = membershipContent.change) { + return when (membershipContent.change) { MembershipChange.JOINED -> if (memberIsYou) { sp.getString(R.string.state_event_room_join_by_you) } else { From 0d45096b5935690cb8a024c816de8b8b4eb3efb7 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 12 Jul 2023 15:11:40 +0200 Subject: [PATCH 8/8] Split task in 2, due to the fact that when we run kover on the CI, run only debug test variants. Error was: Some problems were found with the configuration of task ':koverMergedHtmlReport' (type 'KoverHtmlTask'). - Gradle detected a problem with the following location: '/home/runner/work/element-x-android/element-x-android/features/analytics/api/build/tmp/kotlin-classes/release'. Reason: Task ':koverMergedHtmlReport' uses this output of task ':features:analytics:api:compileReleaseKotlin' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed. Possible solutions: 1. Declare task ':features:analytics:api:compileReleaseKotlin' as an input of ':koverMergedHtmlReport'. 2. Declare an explicit dependency on ':features:analytics:api:compileReleaseKotlin' from ':koverMergedHtmlReport' using Task#dependsOn. 3. Declare an explicit dependency on ':features:analytics:api:compileReleaseKotlin' from ':koverMergedHtmlReport' using Task#mustRunAfter. ... --- .github/workflows/nightlyReports.yml | 5 ++++- .github/workflows/tests.yml | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/.github/workflows/nightlyReports.yml b/.github/workflows/nightlyReports.yml index 4805bf44d1..cbe9935fc7 100644 --- a/.github/workflows/nightlyReports.yml +++ b/.github/workflows/nightlyReports.yml @@ -26,8 +26,11 @@ jobs: distribution: 'temurin' # See 'Supported distributions' for available options java-version: '17' + - name: ⚙️ Run unit & screenshot tests, debug and release + run: ./gradlew test $CI_GRADLE_ARG_PROPERTIES -Pci-build=true + - name: ⚙️ Run unit & screenshot tests, generate kover report - run: ./gradlew test koverMergedReport $CI_GRADLE_ARG_PROPERTIES -Pci-build=true + run: ./gradlew koverMergedReport $CI_GRADLE_ARG_PROPERTIES -Pci-build=true - name: ✅ Upload kover report if: always() diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 44fdf58323..f5df6b1362 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -37,8 +37,11 @@ jobs: with: cache-read-only: ${{ github.ref != 'refs/heads/develop' }} + - name: ⚙️ Run unit & screenshot tests, debug and release + run: ./gradlew test $CI_GRADLE_ARG_PROPERTIES -Pci-build=true + - name: ⚙️ Run unit & screenshot tests, generate kover report - run: ./gradlew test koverMergedReport $CI_GRADLE_ARG_PROPERTIES -Pci-build=true + run: ./gradlew koverMergedReport $CI_GRADLE_ARG_PROPERTIES -Pci-build=true - name: 📈 Verify coverage run: ./gradlew koverMergedVerify $CI_GRADLE_ARG_PROPERTIES -Pci-build=true