From fd1946e7df99a8c2849f525c69db6bd4e1ab0403 Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 25 Jul 2024 12:09:49 +0530 Subject: [PATCH] Clean up of the coverage workflows and left over unused parts from retrieve changed files test targets utility --- .github/workflows/code_coverage.yml | 47 +------ scripts/BUILD.bazel | 8 -- .../org/oppia/android/scripts/ci/BUILD.bazel | 15 --- .../ci/RetrieveChangedFilesTestTargets.kt | 124 ------------------ 4 files changed, 1 insertion(+), 193 deletions(-) delete mode 100644 scripts/src/java/org/oppia/android/scripts/ci/RetrieveChangedFilesTestTargets.kt diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index a00e9037469..42f77c3e308 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -166,21 +166,6 @@ jobs: echo "CHANGED_FILES=$CHANGED_FILES" >> $GITHUB_ENV echo "BAZEL_TEST_TARGETS=$BAZEL_TEST_TARGETS" >> $GITHUB_ENV -# - name: Extract test caching bucket & changed files test targets -# env: -# CHANGED_FILESS_BUCKET_BASE64_ENCODED_SHARD: ${{ matrix.changed-files-bucket-base64-encoded-shard }} -# run: | -# # See https://stackoverflow.com/a/29903172 for cut logic. This is needed to remove the -# # user-friendly shard prefix from the matrix value. -# CHANGED_FILES_TEST_TARGETS_BUCKET_BASE64=$(echo "$CHANGED_FILESS_BUCKET_BASE64_ENCODED_SHARD" | cut -d ";" -f 2) -# bazel run //scripts:retrieve_changed_files_test_targets -- $(pwd) $CHANGED_FILES_TEST_TARGETS_BUCKET_BASE64 $(pwd)/test_bucket_name $(pwd)/bazel_test_targets -# TEST_CATEGORY=$(cat ./test_bucket_name) -# BAZEL_TEST_TARGETS=$(cat ./bazel_test_targets) -# echo "Test category: $TEST_CATEGORY" -# echo "Bazel test targets: $BAZEL_TEST_TARGETS" -# echo "TEST_CACHING_BUCKET=$TEST_CATEGORY" >> $GITHUB_ENV -# echo "BAZEL_TEST_TARGETS=$BAZEL_TEST_TARGETS" >> $GITHUB_ENV - # For reference on this & the later cache actions, see: # https://github.com/actions/cache/issues/239#issuecomment-606950711 & # https://github.com/actions/cache/issues/109#issuecomment-558771281. Note that these work @@ -307,44 +292,14 @@ jobs: BAZEL_REMOTE_CACHE_URL: ${{ secrets.BAZEL_REMOTE_CACHE_URL }} CHANGED_FILES: ${{ env.CHANGED_FILES }} run: | - # Attempt to build 5 times in case there are flaky builds. - # TODO(#3970): Remove this once there are no longer app test build failures. - # i=0 - # Disable exit-on-first-failure. - # set +e - # while [ $i -ne 5 ]; do - # i=$(( $i+1 )) - # echo "Attempt $i/5 to run test targets" bazel run //scripts:run_coverage -- $(pwd) $CHANGED_FILES --format=MARKDOWN - # done - # Capture the error code of the final command run (which should be a success if there isn't a real build failure). - # last_error_code=$? - # Reenable exit-on-first-failure. - # set -e - # Exit only if the most recent exit was a failure (by using a subshell). - #(exit $last_error_code) - name: Run Oppia Coverage (without caching, or on a fork) if: ${{ env.ENABLE_CACHING == 'false' || ((github.ref != 'refs/heads/develop' || github.event_name != 'push') && (github.event.pull_request.head.repo.full_name != 'oppia/oppia-android')) }} env: - BAZEL_TEST_TARGETS: ${{ env.BAZEL_TEST_TARGETS }} + CHANGED_FILES: ${{ env.CHANGED_FILES }} run: | - # Attempt to build 5 times in case there are flaky builds. - # TODO(#3970): Remove this once there are no longer app test build failures. - # i=0 - # Disable exit-on-first-failure. - # set +e - # while [ $i -ne 5 ]; do - # i=$(( $i+1 )) - # echo "Attempt $i/5 to run test targets" bazel run //scripts:run_coverage -- $(pwd) $CHANGED_FILES --format=MARKDOWN - # done - # Capture the error code of the final command run (which should be a success if there isn't a real build failure). - # last_error_code=$? - # Reenable exit-on-first-failure. - # set -e - # Exit only if the most recent exit was a failure (by using a subshell). - #(exit $last_error_code) # Reference: https://github.community/t/127354/7. check_coverage_results: diff --git a/scripts/BUILD.bazel b/scripts/BUILD.bazel index e0a54bbcade..f2911c1af66 100644 --- a/scripts/BUILD.bazel +++ b/scripts/BUILD.bazel @@ -262,14 +262,6 @@ kt_jvm_binary( runtime_deps = ["//scripts/src/java/org/oppia/android/scripts/ci:retrieve_changed_files_lib"], ) -#kt_jvm_binary( -# name = "retrieve_changed_files_test_targets", -# testonly = True, -# data = TEST_FILE_EXEMPTION_ASSETS, -# main_class = "org.oppia.android.scripts.ci.RetrieveChangedFilesTestTargetsKt", -# runtime_deps = ["//scripts/src/java/org/oppia/android/scripts/ci:retrieve_changed_files_test_targets_lib"], -#) - # Note that this is intentionally not test-only since it's used by the app build pipeline. Also, # this apparently needs to be a java_binary to set up runfiles correctly when executed within a # Starlark rule as a tool. diff --git a/scripts/src/java/org/oppia/android/scripts/ci/BUILD.bazel b/scripts/src/java/org/oppia/android/scripts/ci/BUILD.bazel index 570db861319..48c0c06fdd8 100644 --- a/scripts/src/java/org/oppia/android/scripts/ci/BUILD.bazel +++ b/scripts/src/java/org/oppia/android/scripts/ci/BUILD.bazel @@ -60,18 +60,3 @@ kt_jvm_library( "//scripts/src/java/org/oppia/android/scripts/proto:script_exemptions_java_proto", ], ) - -#kt_jvm_library( -# name = "retrieve_changed_files_test_targets_lib", -# testonly = True, -# srcs = [ -# "RetrieveChangedFilesTestTargets.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:proto_string_encoder", -# "//scripts/src/java/org/oppia/android/scripts/proto:changed_files_java_proto", -# "//scripts/src/java/org/oppia/android/scripts/proto:script_exemptions_java_proto", -# ], -#) diff --git a/scripts/src/java/org/oppia/android/scripts/ci/RetrieveChangedFilesTestTargets.kt b/scripts/src/java/org/oppia/android/scripts/ci/RetrieveChangedFilesTestTargets.kt deleted file mode 100644 index d22c3a97fb9..00000000000 --- a/scripts/src/java/org/oppia/android/scripts/ci/RetrieveChangedFilesTestTargets.kt +++ /dev/null @@ -1,124 +0,0 @@ -package org.oppia.android.scripts.ci - -import org.oppia.android.scripts.common.BazelClient -import org.oppia.android.scripts.common.CommandExecutor -import org.oppia.android.scripts.common.CommandExecutorImpl -import org.oppia.android.scripts.common.ProtoStringEncoder.Companion.mergeFromCompressedBase64 -import org.oppia.android.scripts.common.ScriptBackgroundCoroutineDispatcher -import org.oppia.android.scripts.proto.ChangedFilesBucket -import org.oppia.android.scripts.proto.TestFileExemptions -import java.io.File -import java.util.concurrent.TimeUnit - -/** - * The main entrypoint for retrieving the list of changed files from a particular encoded Base64 - * bucket. This is used to parse the output from compute_changed_files. - * - * Usage: - * bazel run //scripts:retrieve_changed_files -- \\ - * \\ - * - * - * Arguments: - * - encoded_proto_in_base64: the compressed & Base64-encoded [ChangedFilesBucket] proto computed - * by compute_changed_files. - * - path_to_bucket_name_output_file: path to the file where the file bucket name corresponding to - * this bucket should be printed. - * - path_to_file_list_output_file: path to the file where the list of changed files - * corresponding to this bucket should be printed. - * - * Example: - * bazel run //scripts:retrieve_changed_files -- $(pwd) $CHANGED_FILES_BUCKETS_BASE64_ENCODED_PROTO \\ - * $(pwd)/file_bucket_name $(pwd)/changed_files - */ -fun main(args: Array) { - /*if (args.size < 3) { - println( - "Usage: bazel run //scripts:retrieve_changed_files --" + - " " + - " " - ) - exitProcess(1) - }*/ - - val repoRoot = args[0] - val rootDirectory = File(repoRoot).absoluteFile - val protoBase64 = args[1] - val bucketNameOutputFile = File(args[2]) - val fileTestTargetsListOutputFile = File(args[3]) - - val testFileExemptionTextProto = "scripts/assets/test_file_exemptions" - val testFileExemptionList by lazy { - loadTestFileExemptionsProto(testFileExemptionTextProto) - .testFileExemptionList - .associateBy { it.exemptedFilePath } - } - - ScriptBackgroundCoroutineDispatcher().use { scriptBgDispatcher -> - val commandExecutor: CommandExecutor = - CommandExecutorImpl( - scriptBgDispatcher, processTimeout = 5, processTimeoutUnit = TimeUnit.MINUTES - ) - - val bazelClient = BazelClient(rootDirectory, commandExecutor) - - val changedFilesBucket = - ChangedFilesBucket.getDefaultInstance().mergeFromCompressedBase64(protoBase64) - bucketNameOutputFile.printWriter().use { writer -> - writer.println(changedFilesBucket.cacheBucketName) - } - - val changedFilesTestFiles = changedFilesBucket.changedFilesList.flatMap { changedFile -> - val exemption = testFileExemptionList[changedFile] - if (exemption != null && exemption.testFileNotRequired) { - emptyList() - } else { - findTestFile(rootDirectory, changedFile) - } - } - println("Changed Files Test Files: $changedFilesTestFiles") - - val changedFilesTestTargets = bazelClient.retrieveBazelTargets(changedFilesTestFiles) - println("Changed Files Test Targets: $changedFilesTestTargets") - - val changedFilesTestTargetWithoutSuffix = changedFilesTestTargets.map { it.removeSuffix(".kt") } - println("Changed Files Test Targets without suffix: $changedFilesTestTargetWithoutSuffix") - - fileTestTargetsListOutputFile.printWriter().use { writer -> - writer.println(changedFilesTestTargetWithoutSuffix.joinToString(separator = " ")) - } - } -} - -private fun findTestFile(rootDirectory: File, filePath: String): List { - val possibleTestFilePaths = when { - filePath.startsWith("scripts/") -> { - listOf(filePath.replace("/java/", "/javatests/").replace(".kt", "Test.kt")) - } - filePath.startsWith("app/") -> { - listOf( - filePath.replace("/main/", "/sharedTest/").replace(".kt", "Test.kt"), - filePath.replace("/main/", "/test/").replace(".kt", "Test.kt"), - filePath.replace("/main/", "/test/").replace(".kt", "LocalTest.kt") - ) - } - else -> { - listOf(filePath.replace("/main/", "/test/").replace(".kt", "Test.kt")) - } - } - - // val repoRootFile = File(repoRoot).absoluteFile - - return possibleTestFilePaths - .map { File(rootDirectory, it) } - .filter(File::exists) - .map { it.relativeTo(rootDirectory).path } -} - -private fun loadTestFileExemptionsProto(testFileExemptiontextProto: String): TestFileExemptions { - return File("$testFileExemptiontextProto.pb").inputStream().use { stream -> - TestFileExemptions.newBuilder().also { builder -> - builder.mergeFrom(stream) - }.build() - } -}