Fix part of #2691: Migrate compute affected tests script to Kotlin (#…

…3374)

* Migrate compute affected tests script to Kotlin.

No tests for the script yet.

* Add BUILD.bazel docstrings.

* Enable linters for scripts code. Fix ktlint issues.

* Add tests for the compute affected tests script.

Fix some typos & realized bug in the script.

* Check out develop first so that exists in CI.

See comment in PR for context.

* Reorganize & refactor, and improve tests.

This splits up the test & script into multiple utility packages (for
common & testing utilities), reorganizes the test to be in a proper
javatests/ & java/ arrangement, and adds tests for checking the error
conditions of the test.

* Add tests for executeCommand().

This also refactors that function to be part of a class so that its
operating conditions are configurable (namely the process timeout).

* Add tests for BazelClient.

* Add tests for GitClient.

* Add tests for TestBazelWorkspace & documentation.

Also, fix a few small issues in the utility.

* Add tests git TestGitRepository.

* Add support for changing relative develop branch.

Also, fix some changed behaviors that broke the affected tests script.

* Fix linter error found in CI.

* Address reviewer comments.

* Remove unnecessary extra checkout from CI.

* Disable test that can't pass in CI.

See #2691 for context.

.github/workflows/unit_tests.yml

@@ -33,8 +33,8 @@ jobs:
         # https://unix.stackexchange.com/a/338124 for reference on creating a JSON-friendly
         # comma-separated list of test targets for the matrix.
         run: |
-          bash ./scripts/compute_affected_tests.sh ./affected_targets.txt
-          TEST_TARGET_LIST=$(cat ./affected_targets.txt | sed 's/^\|$/"/g' | paste -sd, -)
+          bazel run //scripts:compute_affected_tests -- $(pwd) $(pwd)/affected_targets.log origin/develop
+          TEST_TARGET_LIST=$(cat ./affected_targets.log | sed 's/^\|$/"/g' | paste -sd, -)
           echo "Affected tests (note that this might be all tests if on the develop branch): $TEST_TARGET_LIST"
           echo "::set-output name=matrix::{\"test-target\":[$TEST_TARGET_LIST]}"
           if [[ ! -z "$TEST_TARGET_LIST" ]]; then

scripts/BUILD.bazel

@@ -0,0 +1,44 @@
+"""
+Kotlin-based scripts to help developers or perform continuous integration tasks.
+"""
+
+load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_binary")
+
+# Visibility for libraries that should be accessible to script tests.
+package_group(
+    name = "oppia_script_test_visibility",
+    packages = [
+        "//scripts/src/javatests/...",
+    ],
+)
+
+# Visibility for libraries that have binaries defined at this package & tests.
+package_group(
+    name = "oppia_script_binary_visibility",
+    includes = [
+        ":oppia_script_test_visibility",
+    ],
+    packages = [
+        "//scripts",
+    ],
+)
+
+# Visibility for libraries that should be accessible to other script packages & tests.
+package_group(
+    name = "oppia_script_library_visibility",
+    includes = [
+        ":oppia_script_test_visibility",
+    ],
+    packages = [
+        "//scripts/src/java/...",
+    ],
+)
+
+kt_jvm_binary(
+    name = "compute_affected_tests",
+    testonly = True,
+    main_class = "org.oppia.android.scripts.ci.ComputeAffectedTestsKt",
+    runtime_deps = [
+        "//scripts/src/java/org/oppia/android/scripts/ci:compute_affected_tests_lib",
+    ],
+)

scripts/buildifier_lint_check.sh

@@ -14,7 +14,7 @@ else
     buildifier_file_path="$github_actions_path/oppia-android-tools/buildifier"
 fi
 
-$buildifier_file_path --lint=warn --mode=check --warnings=-native-android,+out-of-order-load,+unsorted-dict-items -r app data domain model testing utility third_party tools BUILD.bazel WORKSPACE oppia_android_test.bzl
+$buildifier_file_path --lint=warn --mode=check --warnings=-native-android,+out-of-order-load,+unsorted-dict-items -r app data domain model testing utility third_party tools scripts BUILD.bazel WORKSPACE oppia_android_test.bzl
 
 status=$?
 

scripts/checkstyle_lint_check.sh

@@ -14,7 +14,7 @@ else
     jar_file_path="$github_actions_path/oppia-android-tools/checkstyle-8.37-all.jar"
 fi
 
-lint_results=$(java -jar $jar_file_path -c /google_checks.xml app/src/ data/src/ domain/src/ utility/src/ testing/src/ 2>&1)
+lint_results=$(java -jar $jar_file_path -c /google_checks.xml app/src/ data/src/ domain/src/ utility/src/ testing/src/ scripts/src/ 2>&1)
 
 lint_command_result=$?
 

scripts/ktlint_lint_check.sh

@@ -14,7 +14,7 @@ else
     jar_file_path="$github_actions_path/oppia-android-tools/ktlint"
 fi
 
-java -jar $jar_file_path --android app/src/**/*.kt data/src/**/*.kt domain/src/**/*.kt testing/src/**/*.kt utility/src/**/*.kt
+java -jar $jar_file_path --android app/src/**/*.kt data/src/**/*.kt domain/src/**/*.kt testing/src/**/*.kt utility/src/**/*.kt scripts/src/**/*.kt
 
 status=$?
 
@@ -25,7 +25,7 @@ else
   echo "********************************"
   echo "Ktlint issue found."
   echo "Please fix the above issues.
-  You can also use the ktlint -F --android domain/src/**/*.kt utility/src/**/*.kt data/src/**/*.kt app/src/**/*.kt testing/src/**/*.kt
+  You can also use the java -jar $jar_file_path -F --android domain/src/**/*.kt utility/src/**/*.kt data/src/**/*.kt app/src/**/*.kt testing/src/**/*.kt scripts/src/**/*.kt
   command to fix the most common issues."
   echo "Please note, there might be a possibility where the above command will not fix the issue.
   In that case, you will have to fix it yourself."

scripts/src/java/org/oppia/android/scripts/ci/BUILD.bazel

@@ -0,0 +1,18 @@
+"""
+Libraries corresponding to developer scripts that help with continuous integration workflows.
+"""
+
+load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_library")
+
+kt_jvm_library(
+    name = "compute_affected_tests_lib",
+    testonly = True,
+    srcs = [
+        "ComputeAffectedTests.kt",
+    ],
+    visibility = ["//scripts:oppia_script_binary_visibility"],
+    deps = [
+        "//scripts/src/java/org/oppia/android/scripts/common:bazel_client",
+        "//scripts/src/java/org/oppia/android/scripts/common:git_client",
+    ],
+)

scripts/src/java/org/oppia/android/scripts/ci/ComputeAffectedTests.kt

@@ -0,0 +1,112 @@
+package org.oppia.android.scripts.ci
+
+import org.oppia.android.scripts.common.BazelClient
+import org.oppia.android.scripts.common.GitClient
+import java.io.File
+import java.util.Locale
+import kotlin.system.exitProcess
+
+/**
+ * The main entrypoint for computing the list of affected test targets based on changes in the local
+ * Oppia Android Git repository.
+ *
+ * Usage:
+ *   bazel run //scripts:compute_affected_tests -- \\
+ *     <path_to_directory_root> <path_to_output_file> <base_develop_branch_reference>
+ *
+ * Arguments:
+ * - path_to_directory_root: directory path to the root of the Oppia Android repository.
+ * - path_to_output_file: path to the file in which the affected test targets will be printed.
+ * - base_develop_branch_reference: the reference to the local develop branch that should be use.
+ *     Generally, this is 'origin/develop'.
+ *
+ * Example:
+ *   bazel run //scripts:compute_affected_tests -- $(pwd) /tmp/affected_tests.log
+ */
+fun main(args: Array<String>) {
+  if (args.size < 3) {
+    println(
+      "Usage: bazel run //scripts:compute_affected_tests --" +
+        " <path_to_directory_root> <path_to_output_file> <base_develop_branch_reference>"
+    )
+    exitProcess(1)
+  }
+
+  val pathToRoot = args[0]
+  val pathToOutputFile = args[1]
+  val baseDevelopBranchReference = args[2]
+  val rootDirectory = File(pathToRoot).absoluteFile
+  val outputFile = File(pathToOutputFile).absoluteFile
+
+  check(rootDirectory.isDirectory) { "Expected '$pathToRoot' to be a directory" }
+  check(rootDirectory.list().contains("WORKSPACE")) {
+    "Expected script to be run from the workspace's root directory"
+  }
+
+  println("Running from directory root: $rootDirectory")
+  println("Saving results to file: $outputFile")
+
+  val gitClient = GitClient(rootDirectory, baseDevelopBranchReference)
+  val bazelClient = BazelClient(rootDirectory)
+  println("Current branch: ${gitClient.currentBranch}")
+  println("Most recent common commit: ${gitClient.branchMergeBase}")
+  when (gitClient.currentBranch.toLowerCase(Locale.getDefault())) {
+    "develop" -> computeAffectedTargetsForDevelopBranch(bazelClient, outputFile)
+    else ->
+      computeAffectedTargetsForNonDevelopBranch(gitClient, bazelClient, rootDirectory, outputFile)
+  }
+}
+
+private fun computeAffectedTargetsForDevelopBranch(bazelClient: BazelClient, outputFile: File) {
+  // Compute & print all test targets since this is the develop branch.
+  println("Computing all test targets for the develop branch")
+
+  val allTestTargets = bazelClient.retrieveAllTestTargets()
+  println()
+  println(
+    "Affected test targets:" +
+      "\n${allTestTargets.joinToString(separator = "\n") { "- $it" }}"
+  )
+  outputFile.printWriter().use { writer -> allTestTargets.forEach { writer.println(it) } }
+}
+
+private fun computeAffectedTargetsForNonDevelopBranch(
+  gitClient: GitClient,
+  bazelClient: BazelClient,
+  rootDirectory: File,
+  outputFile: File
+) {
+  // Compute the list of changed files, but exclude files which no longer exist (since bazel query
+  // can't handle these well).
+  val changedFiles = gitClient.changedFiles.filter { filepath ->
+    File(rootDirectory, filepath).exists()
+  }
+  println("Changed files (per Git): $changedFiles")
+
+  val changedFileTargets = bazelClient.retrieveBazelTargets(changedFiles).toSet()
+  println("Changed Bazel file targets: $changedFileTargets")
+
+  val affectedTestTargets = bazelClient.retrieveRelatedTestTargets(changedFileTargets).toSet()
+  println("Affected Bazel test targets: $affectedTestTargets")
+
+  // Compute the list of Bazel files that were changed.
+  val changedBazelFiles = changedFiles.filter { file ->
+    file.endsWith(".bzl", ignoreCase = true) ||
+      file.endsWith(".bazel", ignoreCase = true) ||
+      file == "WORKSPACE"
+  }
+  println("Changed Bazel-specific support files: $changedBazelFiles")
+
+  // Compute the list of affected tests based on BUILD/Bazel/WORKSPACE files. These are generally
+  // framed as: if a BUILD file changes, run all tests transitively connected to it.
+  val transitiveTestTargets = bazelClient.retrieveTransitiveTestTargets(changedBazelFiles)
+  println("Affected test targets due to transitive build deps: $transitiveTestTargets")
+
+  val allAffectedTestTargets = (affectedTestTargets + transitiveTestTargets).toSet()
+  println()
+  println(
+    "Affected test targets:" +
+      "\n${allAffectedTestTargets.joinToString(separator = "\n") { "- $it" }}"
+  )
+  outputFile.printWriter().use { writer -> allAffectedTestTargets.forEach { writer.println(it) } }
+}

scripts/src/java/org/oppia/android/scripts/common/BUILD.bazel

@@ -0,0 +1,41 @@
+"""
+Package for common libraries that potentially support multiple scripts by performing common or
+generic operations.
+"""
+
+load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_library")
+
+kt_jvm_library(
+    name = "bazel_client",
+    testonly = True,
+    srcs = [
+        "BazelClient.kt",
+    ],
+    visibility = ["//scripts:oppia_script_library_visibility"],
+    deps = [
+        "//scripts/src/java/org/oppia/android/scripts/common:command_executor",
+    ],
+)
+
+kt_jvm_library(
+    name = "git_client",
+    testonly = True,
+    srcs = [
+        "GitClient.kt",
+    ],
+    visibility = ["//scripts:oppia_script_library_visibility"],
+    deps = [
+        "//scripts/src/java/org/oppia/android/scripts/common:command_executor",
+    ],
+)
+
+kt_jvm_library(
+    name = "command_executor",
+    testonly = True,
+    srcs = [
+        "CommandExecutor.kt",
+        "CommandExecutorImpl.kt",
+        "CommandResult.kt",
+    ],
+    visibility = ["//scripts:oppia_script_library_visibility"],
+)