Merge pull request #17 from buildkite/tech/code-quality-enhancements

Introduce Static Code Analysis and Refine Encapsulation in SDK

.github/workflows/build-and-test-sdk.yml

@@ -2,13 +2,13 @@ name: Build and Test SDK
 
 on:
   push:
-    branches: [ main ]
+    branches: [main]
   pull_request:
-    branches: [ main ]
+    branches: [main]
 
 jobs:
   build-and-test-sdk:
-    name: Build and Test SDK Project
+    name: Build and Test SDK
     runs-on: ubuntu-latest
 
     env:
@@ -25,5 +25,8 @@ jobs:
           distribution: zulu
           java-version: 17
 
+      - name: Lint Project
+        run: "support/scripts/lint"
+
       - name: Run SDK Unit Tests
-        run: "support/scripts/sdk-unit-tests"
+        run: "support/scripts/sdk-unit-tests"

.github/workflows/publish-sdk-snapshot.yml

@@ -2,7 +2,7 @@ name: Publish SDK SNAPSHOT to Maven Central
 
 on:
   push:
-    branches: [ main ]
+    branches: [main]
 
 jobs:
   publish-sdk-snapshot:

.github/workflows/release-sdk.yml

@@ -2,7 +2,7 @@ name: Release SDK to Maven Central
 
 on:
   release:
-    types: [ published ]
+    types: [published]
 
 jobs:
   publish-sdk:

.github/workflows/test-example-project.yml

@@ -4,7 +4,7 @@ name: Test Example Project To Verify Test Collector Integration
 
 on:
   pull_request:
-    branches: [ main ]
+    branches: [main]
 
 jobs:
   example-project-unit-tests:
@@ -57,4 +57,4 @@ jobs:
         with:
           api-level: 27
           arch: x86_64
-          script: "support/scripts/example-instrumented-tests"
+          script: "support/scripts/example-instrumented-tests"

.github/workflows/yaml-lint.yml

@@ -0,0 +1,18 @@
+name: Yaml Lint
+on:
+  pull_request:
+    branches: [main]
+    paths:
+      - '**.yml'
+
+jobs:
+  yamllint:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/[email protected]
+      - name: yaml-lint
+        uses: reviewdog/[email protected]
+        with:
+          level: error
+          reporter: github-pr-review
+          yamllint_flags: '.'

.yamllint.yml

@@ -0,0 +1,8 @@
+---
+extends: default
+ignore: |
+  /build/
+rules:
+  document-start: disable
+  line-length: disable
+  truthy: disable

CONTRIBUTION.md → CONTRIBUTING.md

@@ -42,4 +42,13 @@ Another approach would be to use code generation to automatically generate some
 this library. It could try to read the library's version and expose this to the library to use. Frankly,
 this approach seems to be ideal but is very difficult to achieve.
 
-Therefore we went with the simple but manual approach to update the version ourselves.
+Therefore we went with the simple but manual approach to update the version ourselves.
+
+### Lint Checks
+
+To maintain high code quality standards, we encourage contributors to utilise our linting scripts before submitting changes. You can run the following commands locally:
+
+To identify linting issues: `.support/scripts/lint`
+For automatic fixes to some common issues: `.support/scripts/lint -F`
+
+Note that while this can resolve many issues automatically, some linting warnings or errors may require manual intervention to correct.

README.md

@@ -149,7 +149,7 @@ the [Buildkite Test Analytics docs](https://buildkite.com/docs/test-analytics).
 
 ## 👩‍💻 Contributing
 
-Please refer the contribution guide [here](CONTRIBUTION.md). 
+Please refer the contribution guide [here](CONTRIBUTING.md). 
 Bug reports and pull requests are welcome on GitHub at https://github.com/buildkite/test-collector-android
 
 ## 📜 License

build-logic/convention/.gitignore

@@ -0,0 +1 @@
+/build

build-logic/convention/build.gradle.kts

@@ -0,0 +1,26 @@
+plugins {
+    `kotlin-dsl`
+}
+group = "com.buildkite.test.collector.buildlogic"
+
+java {
+    sourceCompatibility = JavaVersion.VERSION_17
+    targetCompatibility = JavaVersion.VERSION_17
+}
+
+dependencies {
+    implementation(libs.detekt.gradlePlugin)
+}
+
+gradlePlugin {
+    plugins {
+        register("staticCodeAnalysis") {
+            id = "buildkite.static-code-analysis"
+            implementationClass = "StaticCodeAnalysisConventionPlugin"
+        }
+        register("gitHooks") {
+            id = "buildkite.git-hooks"
+            implementationClass = "GitHooksConventionPlugin"
+        }
+    }
+}

build-logic/convention/src/main/kotlin/GitHooksConventionPlugin.kt

@@ -0,0 +1,31 @@
+import org.gradle.api.Plugin
+import org.gradle.api.Project
+import org.gradle.api.tasks.Copy
+import org.gradle.api.tasks.Exec
+
+class GitHooksConventionPlugin : Plugin<Project> {
+    override fun apply(target: Project) {
+        with(target) {
+            tasks.register(TASK_COPY_HOOKS, Copy::class.java) {
+                from("$rootDir/support/scripts/git") {
+                    include("pre-push")
+                }
+                into("$rootDir/.git/hooks")
+            }
+            tasks.register(TASK_INSTALL_HOOKS, Exec::class.java) {
+                workingDir(rootDir)
+                outputs.dir("$rootDir/.git/hooks")
+                commandLine("chmod")
+                args("-R", "+x", ".git/hooks")
+                dependsOn(tasks.named(TASK_COPY_HOOKS))
+            }
+
+            tasks.getByPath(":example:preBuild").dependsOn(tasks.named(TASK_INSTALL_HOOKS))
+        }
+    }
+
+    private companion object {
+        const val TASK_COPY_HOOKS = "copyGitHooks"
+        const val TASK_INSTALL_HOOKS = "installGitHooks"
+    }
+}

build-logic/convention/src/main/kotlin/StaticCodeAnalysisConventionPlugin.kt

@@ -0,0 +1,56 @@
+import io.gitlab.arturbosch.detekt.Detekt
+import org.gradle.api.Plugin
+import org.gradle.api.Project
+import org.gradle.api.artifacts.VersionCatalogsExtension
+import org.gradle.kotlin.dsl.dependencies
+import org.gradle.kotlin.dsl.getByType
+
+class StaticCodeAnalysisConventionPlugin : Plugin<Project> {
+    override fun apply(target: Project) {
+        with(target) {
+            with(pluginManager) {
+                apply("io.gitlab.arturbosch.detekt")
+            }
+
+            fun Detekt.commonDetektConfiguration() {
+                parallel = true
+                buildUponDefaultConfig = true
+                setSource(file(projectDir))
+                config.setFrom(file("$rootDir/support/vendor/detekt/detekt.yml"))
+                include("**/*.kt")
+                include("**/*.kts")
+                exclude("**/resources/**")
+                exclude("**/build/**")
+                reports {
+                    xml.required.set(false)
+                    html.required.set(true)
+                }
+            }
+
+            tasks.register(TASK_DETEKT_ALL, Detekt::class.java) {
+                description = "Run lint check for whole project"
+                commonDetektConfiguration()
+            }
+
+            tasks.register(TASK_DETEKT_FORMAT, Detekt::class.java) {
+                description = "Format lint errors for whole project"
+                autoCorrect = true
+                commonDetektConfiguration()
+            }
+
+            val libs = extensions.getByType<VersionCatalogsExtension>().named("libs")
+
+            dependencies {
+                add(
+                    configurationName = "detektPlugins",
+                    dependencyNotation = libs.findLibrary("detekt.formatting").get()
+                )
+            }
+        }
+    }
+
+    private companion object {
+        const val TASK_DETEKT_ALL = "detektAll"
+        const val TASK_DETEKT_FORMAT = "detektFormat"
+    }
+}

build-logic/settings.gradle.kts

@@ -0,0 +1,16 @@
+@file:Suppress("UnstableApiUsage")
+
+dependencyResolutionManagement {
+    repositories {
+        google()
+        mavenCentral()
+    }
+    versionCatalogs {
+        create("libs") {
+            from(files("../gradle/libs.versions.toml"))
+        }
+    }
+}
+
+rootProject.name = "build_logic"
+include(":convention")

build.gradle.kts

@@ -3,4 +3,6 @@ plugins {
     alias(libs.plugins.android.library) apply false
     alias(libs.plugins.kotlin.android) apply false
     alias(libs.plugins.kotlin.jvm) apply false
-}
+    id("buildkite.static-code-analysis")
+    id("buildkite.git-hooks")
+}

collector/instrumented-test-collector/src/main/kotlin/com/buildkite/test/collector/android/InstrumentedTestCollector.kt

@@ -17,7 +17,7 @@ abstract class InstrumentedTestCollector(
     private val testObserver = TestObserver()
     private val testUploader = configureInstrumentedTestUploader(apiToken, isDebugEnabled)
 
-    override fun testSuiteStarted(testDescription: Description?) {}
+    override fun testSuiteStarted(testDescription: Description?) { /* Nothing to do */ }
 
     override fun testSuiteFinished(description: Description?) {
         description?.let { testSuite ->

collector/test-data-uploader/src/main/kotlin/com/buildkite/test/collector/android/TestDataUploader.kt

@@ -1,37 +1,40 @@
 package com.buildkite.test.collector.android
 
-import com.buildkite.test.collector.android.network.RetrofitInstance
-import com.buildkite.test.collector.android.network.api.TestUploaderApi
 import com.buildkite.test.collector.android.models.RunEnvironment
 import com.buildkite.test.collector.android.models.TestData
 import com.buildkite.test.collector.android.models.TestDetails
 import com.buildkite.test.collector.android.models.TestResponse
-import com.buildkite.test.collector.android.util.Constants.Collector
+import com.buildkite.test.collector.android.network.RetrofitInstance
+import com.buildkite.test.collector.android.network.api.TestUploaderApi
+import com.buildkite.test.collector.android.util.CollectorUtils.Uploader
 import retrofit2.Response
 
 class TestDataUploader(
-    val testSuiteApiToken: String?,
-    val isDebugEnabled: Boolean
+    private val testSuiteApiToken: String?,
+    private val isDebugEnabled: Boolean
 ) {
     fun configureUploadData(testCollection: List<TestDetails>) {
         val runEnvironment = RunEnvironment().getEnvironmentValues()
 
         val testData = TestData(
             format = "json",
             runEnvironment = runEnvironment,
-            data = testCollection.take(Collector.TEST_DATA_UPLOAD_LIMIT)
+            data = testCollection.take(Uploader.TEST_DATA_UPLOAD_LIMIT)
         )
 
         uploadTestData(testData = testData)
     }
 
     private fun uploadTestData(testData: TestData) {
         if (testSuiteApiToken == null) {
-            println("Buildkite test suite API token is missing. Please set up your API token environment variable to upload the analytics data. Follow [README] for further information.")
+            println(
+                "Buildkite test suite API token is missing. " +
+                    "Please set up your API token environment variable to upload the analytics data. Follow [README] for further information."
+            )
         } else {
-            val retroService = RetrofitInstance.getRetrofitInstance(testSuiteApiToken = testSuiteApiToken)
+            val testUploaderService = RetrofitInstance.getRetrofitInstance(testSuiteApiToken = testSuiteApiToken)
                 .create(TestUploaderApi::class.java)
-            val uploadTestDataApiCall = retroService.uploadTestData(testData = testData)
+            val uploadTestDataApiCall = testUploaderService.uploadTestData(testData = testData)
 
             val executeApiCall = uploadTestDataApiCall.execute()
 
@@ -43,8 +46,12 @@ class TestDataUploader(
 
     private fun logApiResponse(executeApiCall: Response<TestResponse>) {
         when (val apiResponseCode = executeApiCall.raw().code) {
-            202 -> println("\nTest analytics data successfully uploaded to the BuildKite Test Suite. - ${executeApiCall.body()?.runUrl}")
-            else -> println("\nError uploading test analytics data to the BuildKite Test Suite. Error code: $apiResponseCode! Ensure that the test suite API Token is correct.")
+            202 -> println(
+                "\nTest analytics data successfully uploaded to the BuildKite Test Suite. - ${executeApiCall.body()?.runUrl}"
+            )
+            else -> println(
+                "\nError uploading test analytics data to the BuildKite Test Suite. Error code: $apiResponseCode! Ensure that the test suite API Token is correct."
+            )
         }
     }
 }

collector/test-data-uploader/src/main/kotlin/com/buildkite/test/collector/android/models/RunEnvironment.kt

@@ -1,11 +1,11 @@
 package com.buildkite.test.collector.android.models
 
-import com.buildkite.test.collector.android.util.Helpers.generateUUID
+import com.buildkite.test.collector.android.util.CollectorUtils.generateUUIDString
 import com.google.gson.annotations.SerializedName
 
-data class RunEnvironment(
+internal data class RunEnvironment(
     @SerializedName("CI") val ci: String? = null,
-    @SerializedName("key") val key: String = generateUUID(),
+    @SerializedName("key") val key: String = generateUUIDString(),
     @SerializedName("url") val url: String? = null,
     @SerializedName("branch") val branch: String? = null,
     @SerializedName("commit_sha") val commitSha: String? = null,
@@ -46,7 +46,7 @@ data class RunEnvironment(
     companion object {
         // When bumping version, update VERSION_NAME to match new version
         // Used for uploading correct library version
-        val VERSION_NAME = "0.2.0"
-        val COLLECTOR_NAME = "android-buildkite-test-collector"
+        const val VERSION_NAME = "0.2.0"
+        const val COLLECTOR_NAME = "android-buildkite-test-collector"
     }
 }

collector/test-data-uploader/src/main/kotlin/com/buildkite/test/collector/android/models/SpanSection.kt

@@ -3,9 +3,18 @@ package com.buildkite.test.collector.android.models
 import com.google.gson.annotations.SerializedName
 
 enum class SpanSection {
-    @SerializedName(value = "top") Top,
-    @SerializedName(value = "sql") Sql,
-    @SerializedName(value = "http") Http,
-    @SerializedName(value = "sleep") Sleep,
-    @SerializedName(value = "annotation") Annotation
+    @SerializedName(value = "top")
+    Top,
+
+    @SerializedName(value = "sql")
+    Sql,
+
+    @SerializedName(value = "http")
+    Http,
+
+    @SerializedName(value = "sleep")
+    Sleep,
+
+    @SerializedName(value = "annotation")
+    Annotation
 }

collector/test-data-uploader/src/main/kotlin/com/buildkite/test/collector/android/models/TestData.kt

@@ -2,7 +2,7 @@ package com.buildkite.test.collector.android.models
 
 import com.google.gson.annotations.SerializedName
 
-data class TestData(
+internal data class TestData(
     @SerializedName("format") val format: String = "json",
     @SerializedName("run_env") val runEnvironment: RunEnvironment,
     @SerializedName("data") val data: List<TestDetails>