From f870f9b58f516668b66d64e0202b8c5805e2bbf4 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 16 May 2024 10:28:55 +0200 Subject: [PATCH 1/9] Kotlin 1.9.24 --- .idea/kotlinc.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.idea/kotlinc.xml b/.idea/kotlinc.xml index fe63bb677d..148fdd2469 100644 --- a/.idea/kotlinc.xml +++ b/.idea/kotlinc.xml @@ -1,6 +1,6 @@ - \ No newline at end of file From 5cef7f6f2217b6cd89eb92c13bf06c0b3f05762b Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 16 May 2024 10:29:35 +0200 Subject: [PATCH 2/9] Ignore .idea/deploymentTargetSelector.xml --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index e7029fd3cd..342d97a933 100644 --- a/.gitignore +++ b/.gitignore @@ -45,6 +45,7 @@ captures/ .idea/assetWizardSettings.xml .idea/compiler.xml .idea/deploymentTargetDropDown.xml +.idea/deploymentTargetSelector.xml .idea/gradle.xml .idea/jarRepositories.xml .idea/misc.xml From 2e7e5633f5ba7f8452c29c5ea4b43740a82301b0 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 16 May 2024 10:49:14 +0200 Subject: [PATCH 3/9] Add test about redacting an Event that has not been sent #2855 --- .../messages/impl/MessagesPresenterTest.kt | 26 +++++++++++++++++++ .../impl/fixtures/MessageEventFixtures.kt | 3 +++ 2 files changed, 29 insertions(+) diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt index 3adc7059b0..46241b3def 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt @@ -69,12 +69,14 @@ import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState import io.element.android.libraries.matrix.api.room.MessageEventType import io.element.android.libraries.matrix.api.room.RoomMembershipState +import io.element.android.libraries.matrix.api.timeline.item.event.LocalEventSendState import io.element.android.libraries.matrix.api.user.CurrentSessionIdHolder import io.element.android.libraries.matrix.test.AN_AVATAR_URL import io.element.android.libraries.matrix.test.AN_EVENT_ID import io.element.android.libraries.matrix.test.A_ROOM_ID import io.element.android.libraries.matrix.test.A_SESSION_ID import io.element.android.libraries.matrix.test.A_SESSION_ID_2 +import io.element.android.libraries.matrix.test.A_TRANSACTION_ID import io.element.android.libraries.matrix.test.FakeMatrixClient import io.element.android.libraries.matrix.test.core.aBuildMeta import io.element.android.libraries.matrix.test.permalink.FakePermalinkBuilder @@ -456,6 +458,30 @@ class MessagesPresenterTest { } } + @Test + fun `present - handle action redact message in error, in this case the message is just cancelled`() = runTest { + val coroutineDispatchers = testCoroutineDispatchers(useUnconfinedTestDispatcher = true) + val matrixRoom = FakeMatrixRoom() + val presenter = createMessagesPresenter(matrixRoom = matrixRoom, coroutineDispatchers = coroutineDispatchers) + moleculeFlow(RecompositionMode.Immediate) { + presenter.present() + }.test { + skipItems(1) + val initialState = awaitItem() + initialState.eventSink.invoke( + MessagesEvents.HandleAction( + TimelineItemAction.Redact, aMessageEvent( + transactionId = A_TRANSACTION_ID, + sendState = LocalEventSendState.SendingFailed("Failed to send message") + ) + ) + ) + assertThat(matrixRoom.cancelSendCount).isEqualTo(1) + assertThat(matrixRoom.redactEventEventIdParam).isNull() + assertThat(awaitItem().actionListState.target).isEqualTo(ActionListState.Target.None) + } + } + @Test fun `present - handle action report content`() = runTest { val navigator = FakeMessagesNavigator() diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/fixtures/MessageEventFixtures.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/fixtures/MessageEventFixtures.kt index b959ff151f..b6a605bdbf 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/fixtures/MessageEventFixtures.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/fixtures/MessageEventFixtures.kt @@ -28,6 +28,7 @@ import io.element.android.features.messages.impl.timeline.model.event.TimelineIt import io.element.android.libraries.designsystem.components.avatar.AvatarData import io.element.android.libraries.designsystem.components.avatar.AvatarSize import io.element.android.libraries.matrix.api.core.EventId +import io.element.android.libraries.matrix.api.core.TransactionId import io.element.android.libraries.matrix.api.timeline.item.TimelineItemDebugInfo import io.element.android.libraries.matrix.api.timeline.item.event.LocalEventSendState import io.element.android.libraries.matrix.test.AN_EVENT_ID @@ -38,6 +39,7 @@ import kotlinx.collections.immutable.toImmutableList internal fun aMessageEvent( eventId: EventId? = AN_EVENT_ID, + transactionId: TransactionId? = null, isMine: Boolean = true, isEditable: Boolean = true, content: TimelineItemEventContent = TimelineItemTextContent(body = A_MESSAGE, htmlDocument = null, formattedBody = null, isEdited = false), @@ -48,6 +50,7 @@ internal fun aMessageEvent( ) = TimelineItem.Event( id = eventId?.value.orEmpty(), eventId = eventId, + transactionId = transactionId, senderId = A_USER_ID, senderProfile = aProfileTimelineDetailsReady(displayName = A_USER_NAME), senderAvatar = AvatarData(A_USER_ID.value, A_USER_NAME, size = AvatarSize.TimelineSender), From f886cd00a19b08d8958809f046370699c4c71871 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 16 May 2024 11:07:31 +0200 Subject: [PATCH 4/9] Format --- .../features/messages/impl/MessagesPresenterTest.kt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt index 46241b3def..0fe401584b 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt @@ -470,10 +470,11 @@ class MessagesPresenterTest { val initialState = awaitItem() initialState.eventSink.invoke( MessagesEvents.HandleAction( - TimelineItemAction.Redact, aMessageEvent( - transactionId = A_TRANSACTION_ID, - sendState = LocalEventSendState.SendingFailed("Failed to send message") - ) + action = TimelineItemAction.Redact, + event = aMessageEvent( + transactionId = A_TRANSACTION_ID, + sendState = LocalEventSendState.SendingFailed("Failed to send message") + ) ) ) assertThat(matrixRoom.cancelSendCount).isEqualTo(1) From 16ce4cbfcc3b4f5001439e78df50b4c3843b6787 Mon Sep 17 00:00:00 2001 From: Jorge Martin Espinosa Date: Fri, 17 May 2024 11:08:34 +0200 Subject: [PATCH 5/9] Restore legacy shrinking configuration for AGP `8.4.x` (#2867) * Restore legacy shrinking configuration for AGP `8.4.x` The current one is causing issues with release builds and no changes in proguard rules seem to fix them. Co-authored-by: Benoit Marty --------- Co-authored-by: Benoit Marty --- gradle.properties | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gradle.properties b/gradle.properties index cc75862b1f..19237b74cf 100644 --- a/gradle.properties +++ b/gradle.properties @@ -59,3 +59,6 @@ android.enableBuildConfigAsBytecode=true # By default, the plugin applies itself to all subprojects, but we don't want that as it would cause issues with builds using local AARs dependency.analysis.autoapply=false + +# Disable new R8 shrinking for local dependencies as it causes issues with release builds +android.disableMinifyLocalDependenciesForLibraries=false From 46107a9cff02a402358ab11cb542e0546c576e87 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Fri, 17 May 2024 17:07:19 +0200 Subject: [PATCH 6/9] Update kotlin to v0.8.0 (#2854) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Update kotlin to v0.8.0 * Adapt our setup to `v0.8.0`'s changes * Make sure verification tasks run on `check` tasks --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Jorge Martín --- .github/workflows/nightlyReports.yml | 2 +- .github/workflows/tests.yml | 2 +- gradle/libs.versions.toml | 2 +- .../main/kotlin/extension/KoverExtension.kt | 340 ++++++++++-------- 4 files changed, 197 insertions(+), 149 deletions(-) diff --git a/.github/workflows/nightlyReports.yml b/.github/workflows/nightlyReports.yml index 65a2717f36..b27183cc59 100644 --- a/.github/workflows/nightlyReports.yml +++ b/.github/workflows/nightlyReports.yml @@ -33,7 +33,7 @@ jobs: run: ./gradlew verifyPaparazziDebug $CI_GRADLE_ARG_PROPERTIES - name: 📈 Generate kover report and verify coverage - run: ./gradlew :app:koverXmlReportGplayDebug :app:koverHtmlReportGplayDebug :app:koverVerifyGplayDebug $CI_GRADLE_ARG_PROPERTIES + run: ./gradlew :app:koverXmlReportGplayDebug :app:koverHtmlReportGplayDebug :app:koverVerifyAll $CI_GRADLE_ARG_PROPERTIES - name: ✅ Upload kover report if: always() diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 4952ac435c..a678cb54cb 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -55,7 +55,7 @@ jobs: run: ./gradlew verifyPaparazziDebug $CI_GRADLE_ARG_PROPERTIES - name: 📈Generate kover report and verify coverage - run: ./gradlew :app:koverXmlReportGplayDebug :app:koverHtmlReportGplayDebug :app:koverVerifyGplayDebug $CI_GRADLE_ARG_PROPERTIES + run: ./gradlew :app:koverXmlReportGplayDebug :app:koverHtmlReportGplayDebug :app:koverVerifyAll $CI_GRADLE_ARG_PROPERTIES - name: 🚫 Upload kover failed coverage reports if: failure() diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index aaa82ef687..38f879f28b 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -57,7 +57,7 @@ autoservice = "1.1.1" junit = "4.13.2" androidx-test-ext-junit = "1.1.5" espresso-core = "3.5.1" -kover = "0.7.6" +kover = "0.8.0" [libraries] # Project diff --git a/plugins/src/main/kotlin/extension/KoverExtension.kt b/plugins/src/main/kotlin/extension/KoverExtension.kt index e107291918..68f89f300f 100644 --- a/plugins/src/main/kotlin/extension/KoverExtension.kt +++ b/plugins/src/main/kotlin/extension/KoverExtension.kt @@ -16,10 +16,24 @@ package extension -import kotlinx.kover.gradle.plugin.dsl.KoverReportExtension +import kotlinx.kover.gradle.plugin.dsl.AggregationType +import kotlinx.kover.gradle.plugin.dsl.CoverageUnit +import kotlinx.kover.gradle.plugin.dsl.GroupingEntityType +import kotlinx.kover.gradle.plugin.dsl.KoverProjectExtension +import kotlinx.kover.gradle.plugin.dsl.KoverVariantCreateConfig import org.gradle.api.Action import org.gradle.api.Project +import org.gradle.configurationcache.extensions.capitalized import org.gradle.kotlin.dsl.apply +import org.gradle.kotlin.dsl.assign + +enum class KoverVariant(val variantName: String) { + Presenters("presenters"), + States("states"), + Views("views"), +} + +val koverVariants = KoverVariant.values().map { it.variantName } val localAarProjects = listOf( ":libraries:rustsdk", @@ -44,160 +58,179 @@ val excludedKoverSubProjects = listOf( ":libraries:di", ) + localAarProjects -private fun Project.koverReport(action: Action) { - (this as org.gradle.api.plugins.ExtensionAware).extensions.configure("koverReport", action) +private fun Project.kover(action: Action) { + (this as org.gradle.api.plugins.ExtensionAware).extensions.configure("kover", action) } fun Project.setupKover() { + // Create verify all task joining all existing verification tasks + task("koverVerifyAll") { + group = "verification" + description = "Verifies the code coverage of all subprojects." + val dependencies = listOf(":app:koverVerifyGplayDebug") + koverVariants.map { ":app:koverVerify${it.capitalized()}" } + dependsOn(dependencies) + + } // https://kotlin.github.io/kotlinx-kover/ // Run `./gradlew :app:koverHtmlReport` to get report at ./app/build/reports/kover // Run `./gradlew :app:koverXmlReport` to get XML report - koverReport { - filters { - excludes { - classes( - // Exclude generated classes. - "*_ModuleKt", - "anvil.hint.binding.io.element.*", - "anvil.hint.merge.*", - "anvil.hint.multibinding.io.element.*", - "anvil.module.*", - "com.airbnb.android.showkase*", - "io.element.android.libraries.designsystem.showkase.*", - "io.element.android.x.di.DaggerAppComponent*", - "*_Factory", - "*_Factory_Impl", - "*_Factory$*", - "*_Module", - "*_Module$*", - "*Module_Provides*", - "Dagger*Component*", - "*ComposableSingletons$*", - "*_AssistedFactory_Impl*", - "*BuildConfig", - // Generated by Showkase - "*Ioelementandroid*PreviewKt$*", - "*Ioelementandroid*PreviewKt", - // Other - // We do not cover Nodes (normally covered by maestro, but code coverage is not computed with maestro) - "*Node", - "*Node$*", - "*Presenter\$present\$*", - // Forked from compose - "io.element.android.libraries.designsystem.theme.components.bottomsheet.*", - // Test presenter - "io.element.android.features.leaveroom.fake.FakeLeaveRoomPresenter", - ) - annotatedBy( - "androidx.compose.ui.tooling.preview.Preview", - "io.element.android.libraries.architecture.coverage.ExcludeFromCoverage", - "io.element.android.libraries.designsystem.preview.PreviewsDayNight", - "io.element.android.libraries.designsystem.preview.PreviewWithLargeHeight", - ) + kover { + reports { + filters { + excludes { + classes( + // Exclude generated classes. + "*_ModuleKt", + "anvil.hint.binding.io.element.*", + "anvil.hint.merge.*", + "anvil.hint.multibinding.io.element.*", + "anvil.module.*", + "com.airbnb.android.showkase*", + "io.element.android.libraries.designsystem.showkase.*", + "io.element.android.x.di.DaggerAppComponent*", + "*_Factory", + "*_Factory_Impl", + "*_Factory$*", + "*_Module", + "*_Module$*", + "*Module_Provides*", + "Dagger*Component*", + "*ComposableSingletons$*", + "*_AssistedFactory_Impl*", + "*BuildConfig", + // Generated by Showkase + "*Ioelementandroid*PreviewKt$*", + "*Ioelementandroid*PreviewKt", + // Other + // We do not cover Nodes (normally covered by maestro, but code coverage is not computed with maestro) + "*Node", + "*Node$*", + "*Presenter\$present\$*", + // Forked from compose + "io.element.android.libraries.designsystem.theme.components.bottomsheet.*", + // Test presenter + "io.element.android.features.leaveroom.fake.FakeLeaveRoomPresenter", + ) + annotatedBy( + "androidx.compose.ui.tooling.preview.Preview", + "io.element.android.libraries.architecture.coverage.ExcludeFromCoverage", + "io.element.android.libraries.designsystem.preview.PreviewsDayNight", + "io.element.android.libraries.designsystem.preview.PreviewWithLargeHeight", + ) + } } - } - defaults { - // add reports of both 'debug' and 'release' Android build variants to default reports - mergeWith("gplayDebug") + total { + verify { + onCheck = true + // General rule: minimum code coverage. + rule("Global minimum code coverage.") { + groupBy = GroupingEntityType.APPLICATION + bound { + minValue = 70 + // Setting a max value, so that if coverage is bigger, it means that we have to change minValue. + // For instance if we have minValue = 20 and maxValue = 30, and current code coverage is now 31.32%, update + // minValue to 25 and maxValue to 35. + maxValue = 80 + coverageUnits = CoverageUnit.INSTRUCTION + aggregationForGroup = AggregationType.COVERED_PERCENTAGE + } + } + } + } + variant(KoverVariant.Presenters.variantName) { + verify { + onCheck = true + // Rule to ensure that coverage of Presenters is sufficient. + rule("Check code coverage of presenters") { + groupBy = GroupingEntityType.CLASS - verify { - onCheck = true - // General rule: minimum code coverage. - rule("Global minimum code coverage.") { - isEnabled = true - entity = kotlinx.kover.gradle.plugin.dsl.GroupingEntityType.APPLICATION - bound { - minValue = 70 - // Setting a max value, so that if coverage is bigger, it means that we have to change minValue. - // For instance if we have minValue = 20 and maxValue = 30, and current code coverage is now 31.32%, update - // minValue to 25 and maxValue to 35. - maxValue = 80 - metric = kotlinx.kover.gradle.plugin.dsl.MetricType.INSTRUCTION - aggregation = kotlinx.kover.gradle.plugin.dsl.AggregationType.COVERED_PERCENTAGE + bound { + minValue = 85 + coverageUnits = CoverageUnit.INSTRUCTION + aggregationForGroup = AggregationType.COVERED_PERCENTAGE + } } } - // Rule to ensure that coverage of Presenters is sufficient. - rule("Check code coverage of presenters") { - isEnabled = true - entity = kotlinx.kover.gradle.plugin.dsl.GroupingEntityType.CLASS - filters { - includes { - classes( - "*Presenter", - ) - } - excludes { - classes( - "*Fake*Presenter", - "io.element.android.appnav.loggedin.LoggedInPresenter$*", - // Some options can't be tested at the moment - "io.element.android.features.preferences.impl.developer.DeveloperSettingsPresenter$*", - "*Presenter\$present\$*", - ) - } + filters { + includes { + classes( + "*Presenter", + ) } - bound { - minValue = 85 - metric = kotlinx.kover.gradle.plugin.dsl.MetricType.INSTRUCTION - aggregation = kotlinx.kover.gradle.plugin.dsl.AggregationType.COVERED_PERCENTAGE + excludes { + classes( + "*Fake*Presenter", + "io.element.android.appnav.loggedin.LoggedInPresenter$*", + // Some options can't be tested at the moment + "io.element.android.features.preferences.impl.developer.DeveloperSettingsPresenter$*", + "*Presenter\$present\$*", + ) } } - // Rule to ensure that coverage of States is sufficient. - rule("Check code coverage of states") { - isEnabled = true - entity = kotlinx.kover.gradle.plugin.dsl.GroupingEntityType.CLASS - filters { - includes { - classes( - "^*State$", - ) + } + variant(KoverVariant.States.variantName) { + verify { + onCheck = true + // Rule to ensure that coverage of States is sufficient. + rule("Check code coverage of states") { + groupBy = GroupingEntityType.CLASS + bound { + minValue = 90 + coverageUnits = CoverageUnit.INSTRUCTION + aggregationForGroup = AggregationType.COVERED_PERCENTAGE } - excludes { - classes( - "io.element.android.appnav.root.RootNavState*", - "io.element.android.libraries.matrix.api.timeline.item.event.OtherState$*", - "io.element.android.libraries.matrix.api.timeline.item.event.EventSendState$*", - "io.element.android.libraries.matrix.api.room.RoomMembershipState*", - "io.element.android.libraries.matrix.api.room.MatrixRoomMembersState*", - "io.element.android.libraries.push.impl.notifications.NotificationState*", - "io.element.android.features.messages.impl.media.local.pdf.PdfViewerState", - "io.element.android.features.messages.impl.media.local.LocalMediaViewState", - "io.element.android.features.location.impl.map.MapState*", - "io.element.android.libraries.matrix.api.timeline.item.event.LocalEventSendState*", - "io.element.android.libraries.designsystem.swipe.SwipeableActionsState*", - "io.element.android.features.messages.impl.timeline.components.ExpandableState*", - "io.element.android.features.messages.impl.timeline.model.bubble.BubbleState*", - "io.element.android.libraries.maplibre.compose.CameraPositionState*", - "io.element.android.libraries.maplibre.compose.SaveableCameraPositionState", - "io.element.android.libraries.maplibre.compose.SymbolState*", - "io.element.android.features.ftue.api.state.*", - "io.element.android.features.ftue.impl.welcome.state.*", - ) - } - } - bound { - minValue = 90 - metric = kotlinx.kover.gradle.plugin.dsl.MetricType.INSTRUCTION - aggregation = kotlinx.kover.gradle.plugin.dsl.AggregationType.COVERED_PERCENTAGE } } - // Rule to ensure that coverage of Views is sufficient (deactivated for now). - rule("Check code coverage of views") { - isEnabled = true - entity = kotlinx.kover.gradle.plugin.dsl.GroupingEntityType.CLASS - filters { - includes { - classes( - "*ViewKt", - ) + filters { + includes { + classes( + "^*State$", + ) + } + excludes { + classes( + "io.element.android.appnav.root.RootNavState*", + "io.element.android.libraries.matrix.api.timeline.item.event.OtherState$*", + "io.element.android.libraries.matrix.api.timeline.item.event.EventSendState$*", + "io.element.android.libraries.matrix.api.room.RoomMembershipState*", + "io.element.android.libraries.matrix.api.room.MatrixRoomMembersState*", + "io.element.android.libraries.push.impl.notifications.NotificationState*", + "io.element.android.features.messages.impl.media.local.pdf.PdfViewerState", + "io.element.android.features.messages.impl.media.local.LocalMediaViewState", + "io.element.android.features.location.impl.map.MapState*", + "io.element.android.libraries.matrix.api.timeline.item.event.LocalEventSendState*", + "io.element.android.libraries.designsystem.swipe.SwipeableActionsState*", + "io.element.android.features.messages.impl.timeline.components.ExpandableState*", + "io.element.android.features.messages.impl.timeline.model.bubble.BubbleState*", + "io.element.android.libraries.maplibre.compose.CameraPositionState*", + "io.element.android.libraries.maplibre.compose.SaveableCameraPositionState", + "io.element.android.libraries.maplibre.compose.SymbolState*", + "io.element.android.features.ftue.api.state.*", + "io.element.android.features.ftue.impl.welcome.state.*", + ) + } + } + } + variant(KoverVariant.Views.variantName) { + verify { + onCheck = true + // Rule to ensure that coverage of Views is sufficient (deactivated for now). + rule("Check code coverage of views") { + groupBy = GroupingEntityType.CLASS + bound { + // TODO Update this value, for now there are too many missing tests. + minValue = 0 + coverageUnits = CoverageUnit.INSTRUCTION + aggregationForGroup = AggregationType.COVERED_PERCENTAGE } } - bound { - // TODO Update this value, for now there are too many missing tests. - minValue = 0 - metric = kotlinx.kover.gradle.plugin.dsl.MetricType.INSTRUCTION - aggregation = kotlinx.kover.gradle.plugin.dsl.AggregationType.COVERED_PERCENTAGE + } + filters { + includes { + classes( + "*ViewKt", + ) } } } @@ -205,22 +238,37 @@ fun Project.setupKover() { } } -fun Project.applyKoverPluginToAllSubProjects() = rootProject.allprojects { +fun Project.applyKoverPluginToAllSubProjects() = rootProject.subprojects { if (project.path !in localAarProjects) { apply(plugin = "org.jetbrains.kotlinx.kover") + kover { + currentProject { + for (variant in koverVariants) { + createVariant(variant) { + defaultVariants() + } + } + } + } } } +fun KoverVariantCreateConfig.defaultVariants() { + addWithDependencies("gplayDebug", "debug", optional = true) +} + +fun Project.koverSubprojects() = project.rootProject.subprojects + .filter { + it.project.projectDir.resolve("build.gradle.kts").exists() + } + .map { it.path } + .sorted() + .filter { + it !in excludedKoverSubProjects + } + fun Project.koverDependencies() { - project.rootProject.subprojects - .filter { - it.project.projectDir.resolve("build.gradle.kts").exists() - } - .map { it.path } - .sorted() - .filter { - it !in excludedKoverSubProjects - } + project.koverSubprojects() .forEach { // println("Add $it to kover") dependencies.add("kover", project(it)) From 6e22f5bea35a95350c53e31c2dfbc2de1c312549 Mon Sep 17 00:00:00 2001 From: Jorge Martin Espinosa Date: Mon, 20 May 2024 12:15:43 +0200 Subject: [PATCH 7/9] Try to fix CI quality flow (#2880) * Fix CI quality step: - Remove the `onCheck` property in the different Kover verify variants in case they caused this. - Try splitting quality flow per check to avoid OOM issues. --- .github/workflows/quality.yml | 147 +++++++++++++++++- .../main/kotlin/extension/KoverExtension.kt | 4 - 2 files changed, 141 insertions(+), 10 deletions(-) diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index b62dbb0127..c3e29a2306 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -10,7 +10,7 @@ on: # Enrich gradle.properties for CI/CD env: GRADLE_OPTS: -Dorg.gradle.jvmargs="-Xmx6g -Dfile.encoding=UTF-8 -XX:+HeapDumpOnOutOfMemoryError" -Dkotlin.incremental=false -XX:+UseParallelGC - CI_GRADLE_ARG_PROPERTIES: --stacktrace -PpreDexEnable=false --max-workers 8 --no-daemon --warn + CI_GRADLE_ARG_PROPERTIES: --stacktrace -PpreDexEnable=false --max-workers 8 --no-daemon jobs: checkScript: @@ -33,12 +33,13 @@ jobs: - name: Search for invalid screenshot files run: ./tools/test/checkInvalidScreenshots.py - check: - name: Project Check Suite + # Code checks + konsist: + name: Konsist tests runs-on: ubuntu-latest # Allow all jobs on main and develop. Just one per PR. concurrency: - group: ${{ github.ref == 'refs/heads/main' && format('check-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('check-develop-{0}', github.sha) || format('check-{0}', github.ref) }} + group: ${{ github.ref == 'refs/heads/main' && format('check-konsist-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('check-konsist-develop-{0}', github.sha) || format('check-konsist-{0}', github.ref) }} cancel-in-progress: true steps: - uses: actions/checkout@v4 @@ -55,8 +56,40 @@ jobs: uses: gradle/actions/setup-gradle@v3 with: cache-read-only: ${{ github.ref != 'refs/heads/develop' }} - - name: Run code quality check suite - run: ./gradlew runQualityChecks $CI_GRADLE_ARG_PROPERTIES + - name: Run Konsist tests + run: ./gradlew :tests:konsist:testDebugUnitTest $CI_GRADLE_ARG_PROPERTIES --no-daemon + - name: Upload reports + if: always() + uses: actions/upload-artifact@v4 + with: + name: konsist-report + path: | + **/build/reports/**/*.* + + lint: + name: Android lint check + runs-on: ubuntu-latest + # Allow all jobs on main and develop. Just one per PR. + concurrency: + group: ${{ github.ref == 'refs/heads/main' && format('check-lint-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('check-lint-develop-{0}', github.sha) || format('check-lint-{0}', github.ref) }} + cancel-in-progress: true + steps: + - uses: actions/checkout@v4 + with: + # Ensure we are building the branch and not the branch after being merged on develop + # https://github.com/actions/checkout/issues/881 + ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.ref }} + - name: Use JDK 17 + uses: actions/setup-java@v4 + with: + distribution: 'temurin' # See 'Supported distributions' for available options + java-version: '17' + - name: Configure gradle + uses: gradle/actions/setup-gradle@v3 + with: + cache-read-only: ${{ github.ref != 'refs/heads/develop' }} + - name: Run lint + run: ./gradlew :app:lintGplayDebug :app:lintFdroidDebug $CI_GRADLE_ARG_PROPERTIES - name: Upload reports if: always() uses: actions/upload-artifact@v4 @@ -64,6 +97,108 @@ jobs: name: linting-report path: | **/build/reports/**/*.* + + detekt: + name: Detekt checks + runs-on: ubuntu-latest + # Allow all jobs on main and develop. Just one per PR. + concurrency: + group: ${{ github.ref == 'refs/heads/main' && format('check-detekt-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('check-detekt-develop-{0}', github.sha) || format('check-detekt-{0}', github.ref) }} + cancel-in-progress: true + steps: + - uses: actions/checkout@v4 + with: + # Ensure we are building the branch and not the branch after being merged on develop + # https://github.com/actions/checkout/issues/881 + ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.ref }} + - name: Use JDK 17 + uses: actions/setup-java@v4 + with: + distribution: 'temurin' # See 'Supported distributions' for available options + java-version: '17' + - name: Configure gradle + uses: gradle/actions/setup-gradle@v3 + with: + cache-read-only: ${{ github.ref != 'refs/heads/develop' }} + - name: Run Detekt + run: ./gradlew detekt $CI_GRADLE_ARG_PROPERTIES --no-daemon + - name: Upload reports + if: always() + uses: actions/upload-artifact@v4 + with: + name: detekt-report + path: | + **/build/reports/**/*.* + + ktlint: + name: Ktlint checks + runs-on: ubuntu-latest + # Allow all jobs on main and develop. Just one per PR. + concurrency: + group: ${{ github.ref == 'refs/heads/main' && format('check-ktlint-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('check-ktlint-develop-{0}', github.sha) || format('check-ktlint-{0}', github.ref) }} + cancel-in-progress: true + steps: + - uses: actions/checkout@v4 + with: + # Ensure we are building the branch and not the branch after being merged on develop + # https://github.com/actions/checkout/issues/881 + ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.ref }} + - name: Use JDK 17 + uses: actions/setup-java@v4 + with: + distribution: 'temurin' # See 'Supported distributions' for available options + java-version: '17' + - name: Configure gradle + uses: gradle/actions/setup-gradle@v3 + with: + cache-read-only: ${{ github.ref != 'refs/heads/develop' }} + - name: Run Ktlint check + run: ./gradlew ktlintCheck $CI_GRADLE_ARG_PROPERTIES + - name: Upload reports + if: always() + uses: actions/upload-artifact@v4 + with: + name: ktlint-report + path: | + **/build/reports/**/*.* + + knit: + name: Knit checks + runs-on: ubuntu-latest + # Allow all jobs on main and develop. Just one per PR. + concurrency: + group: ${{ github.ref == 'refs/heads/main' && format('check-knit-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('check-knit-develop-{0}', github.sha) || format('check-knit-{0}', github.ref) }} + cancel-in-progress: true + steps: + - uses: actions/checkout@v4 + with: + # Ensure we are building the branch and not the branch after being merged on develop + # https://github.com/actions/checkout/issues/881 + ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.ref }} + - name: Use JDK 17 + uses: actions/setup-java@v4 + with: + distribution: 'temurin' # See 'Supported distributions' for available options + java-version: '17' + - name: Configure gradle + uses: gradle/actions/setup-gradle@v3 + with: + cache-read-only: ${{ github.ref != 'refs/heads/develop' }} + - name: Run Knit + run: ./gradlew knitCheck $CI_GRADLE_ARG_PROPERTIES + + upload_reports: + name: Project Check Suite + runs-on: ubuntu-latest + needs: [konsist, lint, ktlint, detekt] + steps: + - uses: actions/checkout@v4 + with: + # Ensure we are building the branch and not the branch after being merged on develop + # https://github.com/actions/checkout/issues/881 + ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.ref }} + - name: Download reports from previous jobs + uses: actions/download-artifact@v4 - name: Prepare Danger if: always() run: | diff --git a/plugins/src/main/kotlin/extension/KoverExtension.kt b/plugins/src/main/kotlin/extension/KoverExtension.kt index 68f89f300f..ff66d1d6c6 100644 --- a/plugins/src/main/kotlin/extension/KoverExtension.kt +++ b/plugins/src/main/kotlin/extension/KoverExtension.kt @@ -122,7 +122,6 @@ fun Project.setupKover() { total { verify { - onCheck = true // General rule: minimum code coverage. rule("Global minimum code coverage.") { groupBy = GroupingEntityType.APPLICATION @@ -140,7 +139,6 @@ fun Project.setupKover() { } variant(KoverVariant.Presenters.variantName) { verify { - onCheck = true // Rule to ensure that coverage of Presenters is sufficient. rule("Check code coverage of presenters") { groupBy = GroupingEntityType.CLASS @@ -171,7 +169,6 @@ fun Project.setupKover() { } variant(KoverVariant.States.variantName) { verify { - onCheck = true // Rule to ensure that coverage of States is sufficient. rule("Check code coverage of states") { groupBy = GroupingEntityType.CLASS @@ -214,7 +211,6 @@ fun Project.setupKover() { } variant(KoverVariant.Views.variantName) { verify { - onCheck = true // Rule to ensure that coverage of Views is sufficient (deactivated for now). rule("Check code coverage of views") { groupBy = GroupingEntityType.CLASS From 115919e687c8e0de4274d0c4e0d479dcfe27ae06 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 20 May 2024 12:44:43 +0200 Subject: [PATCH 8/9] Update dependency io.mockk:mockk to v1.13.11 (#2872) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Jorge Martin Espinosa --- gradle/libs.versions.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 38f879f28b..5061006ec0 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -136,7 +136,7 @@ test_arch_core = "androidx.arch.core:core-testing:2.2.0" test_junit = "junit:junit:4.13.2" test_runner = "androidx.test:runner:1.5.2" test_junitext = "androidx.test.ext:junit:1.1.5" -test_mockk = "io.mockk:mockk:1.13.10" +test_mockk = "io.mockk:mockk:1.13.11" test_konsist = "com.lemonappdev:konsist:0.13.0" test_turbine = "app.cash.turbine:turbine:1.1.0" test_truth = "com.google.truth:truth:1.4.2" From cec0db8a0eb3251ead1912b137a94ee5c6e60a36 Mon Sep 17 00:00:00 2001 From: Jorge Martin Espinosa Date: Mon, 20 May 2024 13:09:37 +0200 Subject: [PATCH 9/9] When linkifying HTML messages, give priority to explicit link tags (#2879) * When linkifying HTML messages, give priority to explicit link tags --- changelog.d/2291.bugfix | 1 + .../TimelineItemContentMessageFactory.kt | 10 ++++-- .../TimelineItemContentMessageFactoryTest.kt | 32 ++++++++++++++++++- 3 files changed, 39 insertions(+), 4 deletions(-) create mode 100644 changelog.d/2291.bugfix diff --git a/changelog.d/2291.bugfix b/changelog.d/2291.bugfix new file mode 100644 index 0000000000..73e84559a7 --- /dev/null +++ b/changelog.d/2291.bugfix @@ -0,0 +1 @@ +Make sure explicit links in messages take priority over links found by linkification (urls, emails, phone numbers, etc.) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactory.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactory.kt index 2aba9c3669..5c0623b480 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactory.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactory.kt @@ -268,12 +268,16 @@ class TimelineItemContentMessageFactory @Inject constructor( } // Find and set as URLSpans any links present in the text LinkifyCompat.addLinks(this, Linkify.WEB_URLS or Linkify.PHONE_NUMBERS or Linkify.EMAIL_ADDRESSES) - // Restore old spans if they don't conflict with the new ones + // Restore old spans, remove new ones if there is a conflict for ((urlSpan, location) in oldURLSpans) { val (start, end) = location - if (getSpans(start, end).isEmpty()) { - setSpan(urlSpan, start, end, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE) + val addedSpans = getSpans(start, end).orEmpty() + if (addedSpans.isNotEmpty()) { + for (span in addedSpans) { + removeSpan(span) + } } + setSpan(urlSpan, start, end, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE) } return this } diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactoryTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactoryTest.kt index 35de78f65b..6d8fb1ad9a 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactoryTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentMessageFactoryTest.kt @@ -16,7 +16,9 @@ package io.element.android.features.messages.impl.timeline.factories.event +import android.net.Uri import android.text.SpannableString +import android.text.SpannableStringBuilder import android.text.Spanned import android.text.style.URLSpan import androidx.core.text.buildSpannedString @@ -46,6 +48,7 @@ import io.element.android.libraries.matrix.api.media.ImageInfo import io.element.android.libraries.matrix.api.media.MediaSource import io.element.android.libraries.matrix.api.media.ThumbnailInfo import io.element.android.libraries.matrix.api.media.VideoInfo +import io.element.android.libraries.matrix.api.permalink.PermalinkData import io.element.android.libraries.matrix.api.timeline.item.event.AudioMessageType import io.element.android.libraries.matrix.api.timeline.item.event.EmoteMessageType import io.element.android.libraries.matrix.api.timeline.item.event.FileMessageType @@ -75,6 +78,7 @@ import org.robolectric.RobolectricTestRunner import kotlin.time.Duration import kotlin.time.Duration.Companion.minutes +@Suppress("LargeClass") @RunWith(RobolectricTestRunner::class) class TimelineItemContentMessageFactoryTest { @Test @@ -641,6 +645,31 @@ class TimelineItemContentMessageFactoryTest { assertThat((result as TimelineItemEmoteContent).formattedBody).isEqualTo(SpannableString("* Bob formatted")) } + @Test + fun `a message with existing URLSpans keeps it after linkification`() = runTest { + val expectedSpanned = SpannableStringBuilder().apply { + append("Test ") + inSpans(URLSpan("https://www.example.org")) { + append("me@matrix.org") + } + } + val sut = createTimelineItemContentMessageFactory( + htmlConverterTransform = { expectedSpanned }, + permalinkParser = FakePermalinkParser { PermalinkData.FallbackLink(Uri.EMPTY) } + ) + val result = sut.create( + content = createMessageContent( + type = TextMessageType( + body = "Test [me@matrix.org](https://www.example.org)", + formatted = FormattedBody(MessageFormat.HTML, "Test me@matrix.org") + ) + ), + senderDisambiguatedDisplayName = "Bob", + eventId = AN_EVENT_ID, + ) + assertThat((result as TimelineItemTextContent).formattedBody).isEqualTo(expectedSpanned) + } + private fun createMessageContent( body: String = "Body", inReplyTo: InReplyTo? = null, @@ -660,12 +689,13 @@ class TimelineItemContentMessageFactoryTest { private fun createTimelineItemContentMessageFactory( featureFlagService: FeatureFlagService = FakeFeatureFlagService(), htmlConverterTransform: (String) -> CharSequence = { it }, + permalinkParser: FakePermalinkParser = FakePermalinkParser(), ) = TimelineItemContentMessageFactory( fileSizeFormatter = FakeFileSizeFormatter(), fileExtensionExtractor = FileExtensionExtractorWithoutValidation(), featureFlagService = featureFlagService, htmlConverterProvider = FakeHtmlConverterProvider(htmlConverterTransform), - permalinkParser = FakePermalinkParser(), + permalinkParser = permalinkParser, ) private fun createStickerContent(