Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report commit lead time #474

Merged
merged 4 commits into from
Apr 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,6 @@ interface GitHubApi {
fun listPrsWithoutMilestone(): List<PullRequest>

fun createRelease(version: String)

fun prCommits(prNumber: Int): List<Commit>
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,23 @@
package com.jraska.github.client.release

import okhttp3.HttpUrl
import java.time.Instant

class Release(
val releaseName: String,
val releaseUrl: HttpUrl
val releaseUrl: HttpUrl,
val timestamp: Instant
)

class PullRequest(
val number: Int,
val title: String
)

class Commit(
val sha: String,
val time: Instant,
val author: String,
val message: String,
val prNumber: Int
)
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package com.jraska.github.client.release

import com.jraska.github.client.release.data.LeadTimeReporter

class ReleaseMarker(
private val gitHubApi: GitHubApi,
private val notesComposer: NotesComposer
private val notesComposer: NotesComposer,
private val leadTimeReporter: LeadTimeReporter
) {
fun markPrsWithMilestone(release: Release) {
val pullRequests = gitHubApi.listPrsWithoutMilestone()
Expand All @@ -19,5 +22,7 @@ class ReleaseMarker(
val releaseNotes = notesComposer.releaseNotes(pullRequests)
gitHubApi.setMilestoneBody(milestoneNumber, releaseNotes)
gitHubApi.setReleaseBody(release.releaseName, releaseNotes)

leadTimeReporter.reportLeadTime(pullRequests)
}
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
package com.jraska.github.client.release

import com.jraska.analytics.AnalyticsReporter
import com.jraska.github.client.release.data.GitHubApiFactory
import com.jraska.github.client.release.data.LeadTimeReporter
import okhttp3.HttpUrl.Companion.toHttpUrl
import java.time.Instant

object ReleaseMarksPRs {
fun execute(tag: String) {
val environment = Environment.create()
val release = Release(tag, "https://github.com/jraska/github-client/releases/tag/$tag".toHttpUrl())
val release = Release(tag, "https://github.com/jraska/github-client/releases/tag/$tag".toHttpUrl(), Instant.now())

val api = GitHubApiFactory.create(environment)
val releaseMarker = ReleaseMarker(api, NotesComposer())

val leadTimeReporter = LeadTimeReporter(api, release, AnalyticsReporter.create("Lead Time Metrics"))
val releaseMarker = ReleaseMarker(api, NotesComposer(), leadTimeReporter)

releaseMarker.markPrsWithMilestone(release)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package com.jraska.github.client.release.data

import com.jraska.github.client.release.Commit
import com.jraska.github.client.release.GitHubApi
import com.jraska.github.client.release.PullRequest
import java.time.Instant

class GitHubApiImpl(
private val api: RetrofitGitHubApi
Expand Down Expand Up @@ -51,4 +53,16 @@ class GitHubApiImpl(
override fun createRelease(version: String) {
api.createRelease(CreateReleaseDto(version)).execute()
}

override fun prCommits(prNumber: Int): List<Commit> {
return api.commits(prNumber).execute().body()!!.map {
Commit(
it.sha,
Instant.parse(it.commit.author.dateString),
it.author.login,
it.commit.message,
prNumber
)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package com.jraska.github.client.release.data

import com.jraska.analytics.AnalyticsEvent
import com.jraska.analytics.AnalyticsReporter
import com.jraska.github.client.release.Commit
import com.jraska.github.client.release.GitHubApi
import com.jraska.github.client.release.PullRequest
import com.jraska.github.client.release.Release
import java.time.Duration

class LeadTimeReporter(
private val api: GitHubApi,
private val release: Release,
private val reporter: AnalyticsReporter
) {
fun reportLeadTime(pulls: List<PullRequest>) {
val events = pulls.flatMap { api.prCommits(it.number) }
.map { toEvent(it) }

reporter.report(*events.toTypedArray())
}

private fun toEvent(commit: Commit): AnalyticsEvent {
return AnalyticsEvent(
"Commit Released",
mapOf(
"leadTimeSec" to Duration.between(commit.time, release.timestamp).seconds,
"gitCommit" to commit.sha,
"author" to commit.author,
"prNumber" to commit.prNumber,
"releaseName" to release.releaseName,
"message" to commit.message
)
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ interface RetrofitGitHubApi {

@POST("releases")
fun createRelease(@Body dto: CreateReleaseDto): Call<ResponseBody>

@GET("pulls/{pr_number}/commits")
fun commits(@Path("pr_number") prNumber: Int): Call<List<CommitItemDto>>
}

class PullRequestDto {
Expand Down Expand Up @@ -92,3 +95,32 @@ class CreateMilestoneResponseDto {
}

class CommentRequestDto(val body: String)

class CommitItemDto {
@SerializedName("sha")
lateinit var sha: String

@SerializedName("commit")
lateinit var commit: CommitDto

@SerializedName("author")
lateinit var author: UserDto
}

class CommitDto {
@SerializedName("author")
lateinit var author: AuthorDto

@SerializedName("message")
lateinit var message: String
}

class AuthorDto {
@SerializedName("date")
lateinit var dateString: String
}

class UserDto {
@SerializedName("login")
lateinit var login: String
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,28 @@ import com.jraska.github.client.release.data.GitHubApiFactory
import okhttp3.mockwebserver.MockResponse
import okhttp3.mockwebserver.MockWebServer
import org.assertj.core.api.Assertions.assertThat
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import java.io.File
import java.time.Instant

class GitHubApiImplTest {

private lateinit var gitHubApi: GitHubApi

@get:Rule
val mockWebServer = MockWebServer()

@Test
fun testPrMarkedProperly() {
@Before
fun setUp() {
val environment = Environment("fakeToken", mockWebServer.url("/"))
val gitHubApi = GitHubApiFactory.create(environment)
gitHubApi = GitHubApiFactory.create(environment)
}

mockWebServer.enqueue(MockResponse().setResponseCode(200).setBody("{\"url\":\"https://api.github.com/repos/jraska/github-client/releases/40105170\",\"assets_url\":\"https://api.github.com/repos/jraska/github-client/releases/40105170/assets\",\"upload_url\":\"https://uploads.github.com/repos/jraska/github-client/releases/40105170/assets{?name,label}\",\"html_url\":\"https://github.com/jraska/github-client/releases/tag/0.23.0\",\"id\":40105170,\"author\":{\"login\":\"jraska\",\"id\":6277721,\"node_id\":\"MDQ6VXNlcjYyNzc3MjE=\",\"avatar_url\":\"https://avatars.githubusercontent.com/u/6277721?v=4\",\"gravatar_id\":\"\",\"url\":\"https://api.github.com/users/jraska\",\"html_url\":\"https://github.com/jraska\",\"followers_url\":\"https://api.github.com/users/jraska/followers\",\"following_url\":\"https://api.github.com/users/jraska/following{/other_user}\",\"gists_url\":\"https://api.github.com/users/jraska/gists{/gist_id}\",\"starred_url\":\"https://api.github.com/users/jraska/starred{/owner}{/repo}\",\"subscriptions_url\":\"https://api.github.com/users/jraska/subscriptions\",\"organizations_url\":\"https://api.github.com/users/jraska/orgs\",\"repos_url\":\"https://api.github.com/users/jraska/repos\",\"events_url\":\"https://api.github.com/users/jraska/events{/privacy}\",\"received_events_url\":\"https://api.github.com/users/jraska/received_events\",\"type\":\"User\",\"site_admin\":false},\"node_id\":\"MDc6UmVsZWFzZTQwMTA1MTcw\",\"tag_name\":\"0.23.0\",\"target_commitish\":\"master\",\"name\":\"0.23.0\",\"draft\":false,\"prerelease\":false,\"created_at\":\"2021-03-20T16:30:43Z\",\"published_at\":\"2021-03-20T16:31:04Z\",\"assets\":[],\"tarball_url\":\"https://api.github.com/repos/jraska/github-client/tarball/0.23.0\",\"zipball_url\":\"https://api.github.com/repos/jraska/github-client/zipball/0.23.0\",\"body\":\"Hey hey\"}\n"))
@Test
fun testPrMarkedProperly() {
mockWebServer.enqueue("response/release.json")
mockWebServer.enqueue(MockResponse().setResponseCode(200))

gitHubApi.setReleaseBody("0.23.0", "Hey hallo")
Expand All @@ -28,4 +36,29 @@ class GitHubApiImplTest {
assertThat(secondRequest.requestUrl!!.encodedPath.endsWith("releases/40105170"))
assertThat(secondRequest.body.toString()).contains("{\"body\":\"Hey hallo\"}")
}

@Test
fun testGetsCommits() {
mockWebServer.enqueue("response/commits_pr472.json")

val prCommits = gitHubApi.prCommits(472)

assertThat(mockWebServer.takeRequest().requestUrl!!.encodedPath).endsWith("/pulls/472/commits")

val expectedCommit = Commit(
"65910afb3de84bb52283fbc8cb0c4be0988d4343", Instant.parse("2021-04-09T22:37:33Z"),
"jraska", "Delete test which isn't needed anymore",472
)
assertThat(prCommits[1]).usingRecursiveComparison().isEqualTo(expectedCommit)
}
}

fun MockWebServer.enqueue(path: String) {
enqueue(MockResponse().setResponseCode(200).setBody(json(path)))
}

fun json(path: String): String {
val uri = GitHubApiImplTest::class.java.classLoader.getResource(path)
val file = File(uri?.path!!)
return String(file.readBytes())
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package com.jraska.github.client.release.data

import com.jraska.analytics.AnalyticsEvent
import com.jraska.analytics.AnalyticsReporter
import com.jraska.github.client.release.Environment
import com.jraska.github.client.release.PullRequest
import com.jraska.github.client.release.Release
import com.jraska.github.client.release.enqueue
import okhttp3.HttpUrl.Companion.toHttpUrl
import okhttp3.mockwebserver.MockWebServer
import org.assertj.core.api.Assertions.assertThat
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import java.time.Instant

class LeadTimeReporterTest {
private lateinit var leadTimeReporter: LeadTimeReporter
lateinit var analyticsReporter: RecordingAnalyticsReporter

@get:Rule
val mockWebServer = MockWebServer()

@Before
fun setUp() {
analyticsReporter = RecordingAnalyticsReporter()
val environment = Environment("fakeToken", mockWebServer.url("/"))
val gitHubApi = GitHubApiFactory.create(environment)

val release = Release("testRelease", "https://jrasks.com".toHttpUrl(), Instant.parse("2021-04-10T12:00:00Z"))
leadTimeReporter = LeadTimeReporter(gitHubApi, release, analyticsReporter)
}

@Test
fun testReportsLeadTimeProperly() {
mockWebServer.enqueue("response/commits_pr472.json")
mockWebServer.enqueue("response/commits_pr460.json")

leadTimeReporter.reportLeadTime(listOf(PullRequest(472, "First PR"), PullRequest(460, "Second PR")))

assertThat(mockWebServer.requestCount).isEqualTo(2)

val reportedEvents = analyticsReporter.events()
assertThat(reportedEvents).hasSize(3)

assertThat(reportedEvents).allMatch { it.name == "Commit Released" }
assertThat(reportedEvents).allMatch { it.properties["releaseName"] == "testRelease" }

val first = reportedEvents.first()
assertThat(first.properties["leadTimeSec"]).isEqualTo(13 * 3600 + 24 * 60 + 53L)
assertThat(first.properties["author"]).isEqualTo("jraska")
assertThat(first.properties["gitCommit"]).isEqualTo("859ffe735dc185336cbcad09e692d45dcf8c3361")
assertThat(first.properties["message"]).isEqualTo("un composite build tests on CI")

val second = reportedEvents[1]
assertThat(second.properties["leadTimeSec"]).isEqualTo(13 * 3600 + 22 * 60 + 27L)
assertThat(second.properties["author"]).isEqualTo("jraska")
assertThat(second.properties["gitCommit"]).isEqualTo("65910afb3de84bb52283fbc8cb0c4be0988d4343")
assertThat(second.properties["message"]).isEqualTo("Delete test which isn't needed anymore")

val third = reportedEvents[2]
assertThat(third.properties["leadTimeSec"]).isEqualTo(11 * 24 * 3600 + 23 * 3600 + 24 * 60 + 0L)
assertThat(third.properties["author"]).isEqualTo("dependabot[bot]")
assertThat(third.properties["gitCommit"]).isEqualTo("70e59ee5f812506651b10effbb00657d8e7ca4b2")
}
}

class RecordingAnalyticsReporter : AnalyticsReporter {
private val recorder: MutableList<AnalyticsEvent> = mutableListOf()

fun events(): List<AnalyticsEvent> = recorder

override fun report(vararg events: AnalyticsEvent) {
recorder.addAll(events)
}
}
Loading