From 16bd626352a20a84ac87c72afdff982c79922321 Mon Sep 17 00:00:00 2001 From: Simon Schiller Date: Wed, 9 Mar 2022 10:33:41 +0100 Subject: [PATCH] Add support for visualizing file-level ownership --- .../spotify/ruler/frontend/BreakdownTest.kt | 4 +- .../com/spotify/ruler/frontend/CommonTest.kt | 2 +- .../spotify/ruler/frontend/OwnershipTest.kt | 26 +++++++++-- .../ruler/frontend/robots/BaseRobot.kt | 6 +++ .../ruler/frontend/robots/BreakdownRobot.kt | 7 --- ruler-frontend/src/development/report.json | 2 +- .../ruler/frontend/components/Ownership.kt | 46 +++++++++++++++---- 7 files changed, 69 insertions(+), 24 deletions(-) diff --git a/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/BreakdownTest.kt b/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/BreakdownTest.kt index 9917cc48..f0dd5b7f 100644 --- a/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/BreakdownTest.kt +++ b/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/BreakdownTest.kt @@ -39,9 +39,9 @@ class BreakdownTest { fun `Toggling list items reveals and hides details`(driver: WebDriver) { start(driver) .assertNotVisible("com.spotify.MainActivity") - .toggleComponent(":app") + .click(":app") .assertVisible("com.spotify.MainActivity") - .toggleComponent(":app") + .click(":app") .assertNotVisible("com.spotify.MainActivity") } } diff --git a/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/CommonTest.kt b/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/CommonTest.kt index 760c0831..554b6876 100644 --- a/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/CommonTest.kt +++ b/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/CommonTest.kt @@ -45,6 +45,6 @@ class CommonTest { .assertVisible("Component type distribution (component count)") .navigateToOwnershipTab() .assertVisible("Ownership overview") - .assertVisible("Components grouped by owner") + .assertVisible("Components and files grouped by owner") } } diff --git a/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/OwnershipTest.kt b/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/OwnershipTest.kt index 5cd8b708..32f263e8 100644 --- a/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/OwnershipTest.kt +++ b/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/OwnershipTest.kt @@ -40,14 +40,32 @@ class OwnershipTest { fun `Switching the selected owner changes the displayed values`(driver: WebDriver) { start(driver) .navigateToOwnershipTab() - .assertVisible(":app") - .assertVisible("250.0 B") - .assertVisible("450.0 B") + .selectOwner("navigation-team") + .assertVisible(":sample:navigation") + .assertVisible("100.0 B") + .assertVisible("150.0 B") .assertNotVisible(":lib") .selectOwner("lib-team") .assertVisible(":lib") .assertVisible("500.0 B") .assertVisible("600.0 B") - .assertNotVisible(":app") + .assertNotVisible(":sample:navigation") + } + + @Test + fun `Files owned by different teams are filtered and attributed to the right team`(driver: WebDriver) { + start(driver) + .navigateToOwnershipTab() + .selectOwner("app-team") + .click(":app") + .assertVisible("com.spotify.MainActivity") + .assertVisible("100.0 B") + .assertNotVisible("/res/layout/activity_main.xml") + .assertNotVisible("Other owned files") + .selectOwner("app-resource-team") + .click("Other owned files") + .assertVisible("/res/layout/activity_main.xml") + .assertVisible("150.0 B") + .assertNotVisible("com.spotify.MainActivity") } } diff --git a/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/robots/BaseRobot.kt b/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/robots/BaseRobot.kt index 9eeeb2a3..eaa8c65a 100644 --- a/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/robots/BaseRobot.kt +++ b/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/robots/BaseRobot.kt @@ -50,6 +50,12 @@ abstract class BaseRobot>(protected val driver: WebDriver) { return OwnershipRobot(driver) } + /** Clicks on the element with the given [text]. */ + fun click(text: String): T { + driver.findElement(text(text)).click() + return self() + } + /** Selects download size in the global size type selection dropdown. */ fun selectDownloadSize(): T { val dropdown = Select(driver.findElement(id("size-type-dropdown"))) diff --git a/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/robots/BreakdownRobot.kt b/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/robots/BreakdownRobot.kt index f4c8bf7c..6d87c428 100644 --- a/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/robots/BreakdownRobot.kt +++ b/ruler-frontend-tests/src/test/kotlin/com/spotify/ruler/frontend/robots/BreakdownRobot.kt @@ -18,19 +18,12 @@ package com.spotify.ruler.frontend.robots import com.google.common.truth.Truth.assertThat import com.spotify.ruler.frontend.testutil.sibling -import com.spotify.ruler.frontend.testutil.text import org.openqa.selenium.WebDriver /** Testing robot specifically for the breakdown page. */ class BreakdownRobot(driver: WebDriver) : BaseRobot(driver) { override fun self() = this - /** Clicks and toggles (expands or hides) a given [component]. */ - fun toggleComponent(component: String) = apply { - val element = driver.findElement(text(component)) - element.click() - } - /** Assets that the size displayed for a given [component] matches the expected [size]. */ fun assertComponentSize(component: String, size: String) = apply { val element = driver.findElement(sibling(component)) diff --git a/ruler-frontend/src/development/report.json b/ruler-frontend/src/development/report.json index c8c15cca..3aa6f425 100644 --- a/ruler-frontend/src/development/report.json +++ b/ruler-frontend/src/development/report.json @@ -33,7 +33,7 @@ "type": "RESOURCE", "downloadSize": 150, "installSize": 250, - "owner": "app-team" + "owner": "app-resource-team" }, { "name": "com.spotify.MainActivity", diff --git a/ruler-frontend/src/main/kotlin/com/spotify/ruler/frontend/components/Ownership.kt b/ruler-frontend/src/main/kotlin/com/spotify/ruler/frontend/components/Ownership.kt index 969cc01d..296da791 100644 --- a/ruler-frontend/src/main/kotlin/com/spotify/ruler/frontend/components/Ownership.kt +++ b/ruler-frontend/src/main/kotlin/com/spotify/ruler/frontend/components/Ownership.kt @@ -22,6 +22,8 @@ import com.spotify.ruler.frontend.chart.BarChartConfig import com.spotify.ruler.frontend.chart.seriesOf import com.spotify.ruler.frontend.formatSize import com.spotify.ruler.models.AppComponent +import com.spotify.ruler.models.AppFile +import com.spotify.ruler.models.ComponentType import com.spotify.ruler.models.Measurable import react.RBuilder import react.dom.div @@ -40,11 +42,11 @@ fun RBuilder.ownership(components: List, sizeType: Measurable.Size @RFunction fun RBuilder.componentOwnershipOverview(components: List) { val sizes = mutableMapOf() - components.forEach { component -> - val owner = component.owner ?: return@forEach + components.flatMap(AppComponent::files).forEach { file -> + val owner = file.owner ?: return@forEach val current = sizes.getOrPut(owner) { Measurable.Mutable(0, 0) } - current.downloadSize += component.downloadSize - current.installSize += component.installSize + current.downloadSize += file.downloadSize + current.installSize += file.installSize } val sorted = sizes.entries.sortedByDescending { (_, measurable) -> measurable.downloadSize } @@ -73,19 +75,45 @@ fun RBuilder.componentOwnershipOverview(components: List) { @RFunction fun RBuilder.componentOwnershipPerTeam(components: List, sizeType: Measurable.SizeType) { - val owners = components.mapNotNull(AppComponent::owner).distinct().sorted() + val files = components.flatMap(AppComponent::files) + val owners = files.mapNotNull(AppFile::owner).distinct().sorted() var selectedOwner by useState(owners.first()) val ownedComponents = components.filter { component -> component.owner == selectedOwner } + val ownedFiles = files.filter { file -> file.owner == selectedOwner } - h4(classes = "mb-3 mt-4") { +"Components grouped by owner" } + val remainingOwnedFiles = ownedFiles.toMutableSet() + val processedComponents = ownedComponents.map { component -> + val ownedFilesFromComponent = component.files.filter { file -> file.owner == selectedOwner } + remainingOwnedFiles.removeAll(ownedFilesFromComponent) + component.copy( + downloadSize = ownedFilesFromComponent.sumOf(AppFile::downloadSize), + installSize = ownedFilesFromComponent.sumOf(AppFile::installSize), + files = ownedFilesFromComponent, + ) + }.toMutableList() + + // Group together all owned files which belong to components not owned by the currently selected owner + if (remainingOwnedFiles.isNotEmpty()) { + processedComponents += AppComponent( + name = "Other owned files", + type = ComponentType.INTERNAL, + downloadSize = remainingOwnedFiles.sumOf(AppFile::downloadSize), + installSize = remainingOwnedFiles.sumOf(AppFile::installSize), + files = remainingOwnedFiles.toList(), + owner = selectedOwner, + ) + } + + h4(classes = "mb-3 mt-4") { +"Components and files grouped by owner" } dropdown(owners, "owner-dropdown") { owner -> selectedOwner = owner } div(classes = "row mt-4 mb-4") { highlightedValue(ownedComponents.size, "Component(s)") - highlightedValue(ownedComponents.sumOf(AppComponent::downloadSize), "Download size", ::formatSize) - highlightedValue(ownedComponents.sumOf(AppComponent::installSize), "Install size", ::formatSize) + highlightedValue(ownedFiles.size, "File(s)") + highlightedValue(ownedFiles.sumOf(AppFile::downloadSize), "Download size", ::formatSize) + highlightedValue(ownedFiles.sumOf(AppFile::installSize), "Install size", ::formatSize) } - componentList(ownedComponents, sizeType) + componentList(processedComponents, sizeType) } @RFunction