Skip to content

Commit

Permalink
Add support for visualizing file-level ownership
Browse files Browse the repository at this point in the history
  • Loading branch information
simonschiller committed Mar 14, 2022
1 parent 4adfcac commit 16bd626
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ abstract class BaseRobot<T : BaseRobot<T>>(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")))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<BreakdownRobot>(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))
Expand Down
2 changes: 1 addition & 1 deletion ruler-frontend/src/development/report.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"type": "RESOURCE",
"downloadSize": 150,
"installSize": 250,
"owner": "app-team"
"owner": "app-resource-team"
},
{
"name": "com.spotify.MainActivity",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -40,11 +42,11 @@ fun RBuilder.ownership(components: List<AppComponent>, sizeType: Measurable.Size
@RFunction
fun RBuilder.componentOwnershipOverview(components: List<AppComponent>) {
val sizes = mutableMapOf<String, Measurable.Mutable>()
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 }
Expand Down Expand Up @@ -73,19 +75,45 @@ fun RBuilder.componentOwnershipOverview(components: List<AppComponent>) {

@RFunction
fun RBuilder.componentOwnershipPerTeam(components: List<AppComponent>, 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
Expand Down

0 comments on commit 16bd626

Please sign in to comment.