diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml new file mode 100644 index 00000000000..711151743b2 --- /dev/null +++ b/.github/workflows/code_coverage.yml @@ -0,0 +1,255 @@ +# Contains jobs corresponding to code coverage. + +name: Code Coverage + +# Controls when the action will run. Triggers the workflow on pull request +# events or push events in the develop branch. +on: + pull_request: + push: + branches: + # Push events on develop branch + - develop + +jobs: + check_unit_tests_completed: + name: Check unit test completed + runs-on: ubuntu-latest + steps: + - name: Wait for unit tests to checks + uses: ArcticLampyrid/action-wait-for-workflow@v1.2.0 + with: + workflow: unit_tests.yml + sha: auto + + compute_changed_files: + name: Compute changed files + needs: check_unit_tests_completed + runs-on: ubuntu-20.04 + outputs: + matrix: ${{ steps.compute-file-matrix.outputs.matrix }} + can_skip_files: ${{ steps.compute-file-matrix.outputs.can_skip_files }} + env: + CACHE_DIRECTORY: ~/.bazel_cache + steps: + - uses: actions/checkout@v2 + with: + fetch-depth: 0 + + - name: Set up Bazel + uses: abhinavsingh/setup-bazel@v3 + with: + version: 6.5.0 + + - uses: actions/cache@v2 + id: scripts_cache + with: + path: ${{ env.CACHE_DIRECTORY }} + key: ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel-scripts-${{ github.sha }} + restore-keys: | + ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel-scripts- + ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel- + + # This check is needed to ensure that Bazel's unbounded cache growth doesn't result in a + # situation where the cache never updates (e.g. due to exceeding GitHub's cache size limit) + # thereby only ever using the last successful cache version. This solution will result in a + # few slower CI actions around the time cache is detected to be too large, but it should + # incrementally improve thereafter. + - name: Ensure cache size + env: + BAZEL_CACHE_DIR: ${{ env.CACHE_DIRECTORY }} + run: | + # See https://stackoverflow.com/a/27485157 for reference. + EXPANDED_BAZEL_CACHE_PATH="${BAZEL_CACHE_DIR/#\~/$HOME}" + CACHE_SIZE_MB=$(du -smc $EXPANDED_BAZEL_CACHE_PATH | grep total | cut -f1) + echo "Total size of Bazel cache (rounded up to MBs): $CACHE_SIZE_MB" + # Use a 4.5GB threshold since actions/cache compresses the results, and Bazel caches seem + # to only increase by a few hundred megabytes across changes for unrelated branches. This + # is also a reasonable upper-bound (local tests as of 2021-03-31 suggest that a full build + # of the codebase (e.g. //...) from scratch only requires a ~2.1GB uncompressed/~900MB + # compressed cache). + if [[ "$CACHE_SIZE_MB" -gt 4500 ]]; then + echo "Cache exceeds cut-off; resetting it (will result in a slow build)" + rm -rf $EXPANDED_BAZEL_CACHE_PATH + fi + + - name: Configure Bazel to use a local cache + env: + BAZEL_CACHE_DIR: ${{ env.CACHE_DIRECTORY }} + run: | + EXPANDED_BAZEL_CACHE_PATH="${BAZEL_CACHE_DIR/#\~/$HOME}" + echo "Using $EXPANDED_BAZEL_CACHE_PATH as Bazel's cache path" + echo "build --disk_cache=$EXPANDED_BAZEL_CACHE_PATH" >> $HOME/.bazelrc + shell: bash + + - name: Compute file matrix + id: compute-file-matrix + env: + compute_all_files: ${{ contains(github.event.pull_request.title, '[RunAllTests]') }} + # See: https://docs.github.com/en/webhooks-and-events/webhooks/webhook-events-and-payloads#pull_request. Defer to origin/develop outside a PR (such as develop's main CI runs). + base_commit_hash: ${{ github.event.pull_request.base.sha || 'origin/develop' }} + # https://unix.stackexchange.com/a/338124 for reference on creating a JSON-friendly + # comma-separated list of test targets for the matrix. + run: | + bazel run //scripts:compute_changed_files -- $(pwd) $(pwd)/changed_files.log $base_commit_hash compute_all_files=$compute_all_files + FILE_BUCKET_LIST=$(cat ./changed_files.log | sed 's/^\|$/"/g' | paste -sd, -) + echo "Changed files (note that this might be all files if configured to run all or on the develop branch): $FILE_BUCKET_LIST" + echo "::set-output name=matrix::{\"changed-files-bucket-base64-encoded-shard\":[$FILE_BUCKET_LIST]}" + if [[ ! -z "$FILE_BUCKET_LIST" ]]; then + echo "::set-output name=can_skip_files::false" + else + echo "::set-output name=can_skip_files::true" + echo "No files are affected by this change. If this is wrong, you can add '[RunAllTests]' to the PR title to force a run." + fi + + code_coverage_run: + name: Run Code Coverage + needs: compute_changed_files + if: ${{ needs.compute_changed_files.outputs.can_skip_files != 'true' }} + runs-on: ubuntu-20.04 + strategy: + fail-fast: false + max-parallel: 10 + matrix: ${{ fromJson(needs.compute_changed_files.outputs.matrix) }} + env: + CACHE_DIRECTORY: ~/.bazel_cache + steps: + - uses: actions/checkout@v2 + + - name: Set up JDK 11 + uses: actions/setup-java@v1 + with: + java-version: 11 + + - name: Set up Bazel + uses: abhinavsingh/setup-bazel@v3 + with: + version: 6.5.0 + + - uses: actions/cache@v2 + id: scripts_cache + with: + path: ${{ env.CACHE_DIRECTORY }} + key: ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel-scripts-${{ github.sha }} + restore-keys: | + ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel-scripts- + ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel- + + - name: Set up build environment + uses: ./.github/actions/set-up-android-bazel-build-environment + + - name: Configure Bazel to use a local cache (for scripts) + env: + BAZEL_CACHE_DIR: ${{ env.CACHE_DIRECTORY }} + run: | + EXPANDED_BAZEL_CACHE_PATH="${BAZEL_CACHE_DIR/#\~/$HOME}" + echo "Using $EXPANDED_BAZEL_CACHE_PATH as Bazel's cache path" + echo "build --disk_cache=$EXPANDED_BAZEL_CACHE_PATH" >> $HOME/.bazelrc + shell: bash + + - name: Extract caching bucket + 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_BUCKET_BASE64=$(echo "$CHANGED_FILESS_BUCKET_BASE64_ENCODED_SHARD" | cut -d ";" -f 2) + bazel run //scripts:retrieve_changed_files -- $(pwd) $CHANGED_FILES_BUCKET_BASE64 $(pwd)/file_bucket_name $(pwd)/changed_files $(pwd)/bazel_test_targets + FILE_CATEGORY=$(cat ./file_bucket_name) + CHANGED_FILES=$(cat ./changed_files) + BAZEL_TEST_TARGETS=$(cat ./bazel_test_targets) + echo "File category: $FILE_CATEGORY" + echo "Changed Files: $CHANGED_FILES" + echo "Bazel test targets: $BAZEL_TEST_TARGETS" + echo "FILE_CACHING_BUCKET=$FILE_CATEGORY" >> $GITHUB_ENV + echo "CHANGED_FILES=$CHANGED_FILES" >> $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 + # with Bazel since Bazel can share the most recent cache from an unrelated build and still + # benefit from incremental build performance (assuming that actions/cache aggressively removes + # older caches due to the 5GB cache limit size & Bazel's large cache size). + - uses: actions/cache@v2 + id: test_cache + with: + path: ${{ env.CACHE_DIRECTORY }} + key: ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel-tests-${{ env.FILE_CACHING_BUCKET }}-${{ github.sha }} + restore-keys: | + ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel-tests-${{ env.FILE_CACHING_BUCKET }}- + ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel-tests- + ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel-binary- + ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel- + + # This check is needed to ensure that Bazel's unbounded cache growth doesn't result in a + # situation where the cache never updates (e.g. due to exceeding GitHub's cache size limit) + # thereby only ever using the last successful cache version. This solution will result in a + # few slower CI actions around the time cache is detected to be too large, but it should + # incrementally improve thereafter. + - name: Ensure cache size + env: + BAZEL_CACHE_DIR: ${{ env.CACHE_DIRECTORY }} + run: | + # See https://stackoverflow.com/a/27485157 for reference. + EXPANDED_BAZEL_CACHE_PATH="${BAZEL_CACHE_DIR/#\~/$HOME}" + CACHE_SIZE_MB=$(du -smc $EXPANDED_BAZEL_CACHE_PATH | grep total | cut -f1) + echo "Total size of Bazel cache (rounded up to MBs): $CACHE_SIZE_MB" + # Use a 4.5GB threshold since actions/cache compresses the results, and Bazel caches seem + # to only increase by a few hundred megabytes across changes for unrelated branches. This + # is also a reasonable upper-bound (local tests as of 2021-03-31 suggest that a full build + # of the codebase (e.g. //...) from scratch only requires a ~2.1GB uncompressed/~900MB + # compressed cache). + if [[ "$CACHE_SIZE_MB" -gt 4500 ]]; then + echo "Cache exceeds cut-off; resetting it (will result in a slow build)" + rm -rf $EXPANDED_BAZEL_CACHE_PATH + fi + + - name: Configure Bazel to use a local cache + env: + BAZEL_CACHE_DIR: ${{ env.CACHE_DIRECTORY }} + run: | + EXPANDED_BAZEL_CACHE_PATH="${BAZEL_CACHE_DIR/#\~/$HOME}" + echo "Using $EXPANDED_BAZEL_CACHE_PATH as Bazel's cache path" + echo "build --disk_cache=$EXPANDED_BAZEL_CACHE_PATH" >> $HOME/.bazelrc + shell: bash + + - name: Build Oppia Tests + env: + BAZEL_TEST_TARGETS: ${{ env.BAZEL_TEST_TARGETS }} + run: | + # Attempt to build 5 times in case there are flaky builds. + # TODO(#3759): 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 build test targets" + bazel build --keep_going -- $BAZEL_TEST_TARGETS + 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 + env: + CHANGED_FILES: ${{ env.CHANGED_FILES }} + run: | + bazel run //scripts:run_coverage -- $(pwd) $CHANGED_FILES --format=MARKDOWN --processTimeout=15 + + # Reference: https://github.community/t/127354/7. + check_coverage_results: + name: Check Code Coverage Results + needs: [ compute_changed_files, code_coverage_run ] + # The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations, + # serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows. + if: ${{ !cancelled() }} + runs-on: ubuntu-20.04 + steps: + - name: Check coverages passed + if: ${{ needs.compute_changed_files.outputs.can_skip_files != 'true' && needs.code_coverage_run.result != 'success' }} + run: exit 1 diff --git a/scripts/BUILD.bazel b/scripts/BUILD.bazel index 2fba670d34e..d92196994e5 100644 --- a/scripts/BUILD.bazel +++ b/scripts/BUILD.bazel @@ -247,6 +247,21 @@ kt_jvm_binary( ], ) +kt_jvm_binary( + name = "retrieve_changed_files", + testonly = True, + data = TEST_FILE_EXEMPTION_ASSETS, + main_class = "org.oppia.android.scripts.ci.RetrieveChangedFilesKt", + runtime_deps = ["//scripts/src/java/org/oppia/android/scripts/ci:retrieve_changed_files_lib"], +) + +kt_jvm_binary( + name = "compute_changed_files", + testonly = True, + main_class = "org.oppia.android.scripts.ci.ComputeChangedFilesKt", + runtime_deps = ["//scripts/src/java/org/oppia/android/scripts/ci:compute_changed_files_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 ff9990b866d..413e3a8ca1e 100644 --- a/scripts/src/java/org/oppia/android/scripts/ci/BUILD.bazel +++ b/scripts/src/java/org/oppia/android/scripts/ci/BUILD.bazel @@ -31,3 +31,33 @@ kt_jvm_library( "//scripts/src/java/org/oppia/android/scripts/proto:affected_tests_java_proto", ], ) + +kt_jvm_library( + name = "compute_changed_files_lib", + testonly = True, + srcs = [ + "ComputeChangedFiles.kt", + ], + visibility = ["//scripts:oppia_script_binary_visibility"], + deps = [ + "//scripts/src/java/org/oppia/android/scripts/common:git_client", + "//scripts/src/java/org/oppia/android/scripts/common:proto_string_encoder", + "//scripts/src/java/org/oppia/android/scripts/common:repository_file", + "//scripts/src/java/org/oppia/android/scripts/proto:changed_files_java_proto", + ], +) + +kt_jvm_library( + name = "retrieve_changed_files_lib", + testonly = True, + srcs = [ + "RetrieveChangedFiles.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/ComputeChangedFiles.kt b/scripts/src/java/org/oppia/android/scripts/ci/ComputeChangedFiles.kt new file mode 100644 index 00000000000..93748288c92 --- /dev/null +++ b/scripts/src/java/org/oppia/android/scripts/ci/ComputeChangedFiles.kt @@ -0,0 +1,370 @@ +package org.oppia.android.scripts.ci + +import org.oppia.android.scripts.common.CommandExecutor +import org.oppia.android.scripts.common.CommandExecutorImpl +import org.oppia.android.scripts.common.GitClient +import org.oppia.android.scripts.common.ProtoStringEncoder.Companion.toCompressedBase64 +import org.oppia.android.scripts.common.RepositoryFile +import org.oppia.android.scripts.common.ScriptBackgroundCoroutineDispatcher +import org.oppia.android.scripts.proto.ChangedFilesBucket +import java.io.File +import java.util.Locale +import java.util.concurrent.TimeUnit +import kotlin.system.exitProcess + +private const val COMPUTE_ALL_FILES_PREFIX = "compute_all_files=" +private const val MAX_FILE_COUNT_PER_LARGE_SHARD = 50 +private const val MAX_FILE_COUNT_PER_MEDIUM_SHARD = 25 +private const val MAX_FILE_COUNT_PER_SMALL_SHARD = 15 + +/** + * The main entrypoint for computing the list of changed files based on changes in the local + * Oppia Android Git repository. + * + * Usage: + * bazel run //scripts:compute_changed_files -- \\ + * \\ + * + * + * 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 changed files will be printed. + * - merge_base_commit: the base commit against which the local changes will be compared when + * determining which files to run. When running outside of CI you can use the result of running: + * 'git merge-base develop HEAD' + * - compute_all_files: whether to compute a list of all files to run. + * + * Example: + * bazel run //scripts:compute_changed_files -- $(pwd) /tmp/changed_file_buckets.proto64 \\ + * abcdef0123456789 compute_all_files=false + */ +fun main(args: Array) { + if (args.size < 4) { + println( + "Usage: bazel run //scripts:compute_changed_files --" + + " " + + " " + ) + exitProcess(1) + } + + val pathToRoot = args[0] + val pathToOutputFile = args[1] + val baseCommit = args[2] + val computeAllFilesSetting = args[3].let { + check(it.startsWith(COMPUTE_ALL_FILES_PREFIX)) { + "Expected last argument to start with '$COMPUTE_ALL_FILES_PREFIX'" + } + val computeAllFilesValue = it.removePrefix(COMPUTE_ALL_FILES_PREFIX) + return@let computeAllFilesValue.toBooleanStrictOrNull() + ?: error( + "Expected last argument to have 'true' or 'false' passed to it, not:" + + " '$computeAllFilesValue'" + ) + } + println("Compute All Files Setting set to: $computeAllFilesSetting") + ScriptBackgroundCoroutineDispatcher().use { scriptBgDispatcher -> + ComputeChangedFiles(scriptBgDispatcher) + .compute(pathToRoot, pathToOutputFile, baseCommit, computeAllFilesSetting) + } +} + +/** Utility used to compute changed files. */ +class ComputeChangedFiles( + private val scriptBgDispatcher: ScriptBackgroundCoroutineDispatcher, + val maxFileCountPerLargeShard: Int = MAX_FILE_COUNT_PER_LARGE_SHARD, + val maxFileCountPerMediumShard: Int = MAX_FILE_COUNT_PER_MEDIUM_SHARD, + val maxFileCountPerSmallShard: Int = MAX_FILE_COUNT_PER_SMALL_SHARD, + val commandExecutor: CommandExecutor = + CommandExecutorImpl( + scriptBgDispatcher, processTimeout = 5, processTimeoutUnit = TimeUnit.MINUTES + ) +) { + private companion object { + private const val GENERIC_FILE_BUCKET_NAME = "generic" + } + + /** + * Computes a list of files to run. + * + * @param pathToRoot the absolute path to the working root directory + * @param pathToOutputFile the absolute path to the file in which the encoded Base64 file bucket + * protos should be printed + * @param baseCommit see [GitClient] + * @param computeAllFilesSetting whether all files should be outputted versus only the ones which + * are changed by local changes in the repository + */ + fun compute( + pathToRoot: String, + pathToOutputFile: String, + baseCommit: String, + computeAllFilesSetting: Boolean + ) { + val rootDirectory = File(pathToRoot).absoluteFile + check(rootDirectory.isDirectory) { "Expected '$pathToRoot' to be a directory" } + check(rootDirectory.list()?.contains("WORKSPACE") == true) { + "Expected script to be run from the workspace's root directory" + } + + println("Running from directory root: $rootDirectory.") + + val gitClient = GitClient(rootDirectory, baseCommit, commandExecutor) + println("Current branch: ${gitClient.currentBranch}.") + println("Most recent common commit: ${gitClient.branchMergeBase}.") + + val currentBranch = gitClient.currentBranch.lowercase(Locale.US) + val changedFiles = if (computeAllFilesSetting || currentBranch == "develop") { + computeAllFiles(rootDirectory, pathToRoot) + } else computeChangedFilesForNonDevelopBranch(gitClient, rootDirectory, pathToRoot) + + val filteredFiles = filterFiles(changedFiles) + println() + println("Changed Files:") + println(filteredFiles.joinToString(separator = "\n") { "- $it" }) + + val changedFileBuckets = bucketFiles(filteredFiles) + val encodedFileBucketEntries = changedFileBuckets + .associateBy { it.toCompressedBase64() } + .entries.shuffled() + + File(pathToOutputFile).printWriter().use { writer -> + encodedFileBucketEntries.forEachIndexed { index, (encoded, bucket) -> + writer.println("${bucket.cacheBucketName}-shard$index;$encoded") + } + } + } + + private fun computeAllFiles( + rootDirectory: File, + pathToRoot: String + ): List { + val searchFiles = RepositoryFile.collectSearchFiles( + repoPath = pathToRoot, + expectedExtension = ".kt", + ) + + return searchFiles + .filter { it.extension == "kt" && !it.nameWithoutExtension.endsWith("Test") } + .map { it.toRelativeString(rootDirectory) } + } + + private fun computeChangedFilesForNonDevelopBranch( + gitClient: GitClient, + rootDirectory: File, + pathToRoot: String + ): List { + val changedKtFiles = gitClient.committedFiles + .map { File(rootDirectory, it) } + .filter { it.exists() } + .map { it.toRelativeString(rootDirectory) } + .filter { it.endsWith(".kt") } + + return changedKtFiles + .map { changedKtFile -> + when { + changedKtFile.endsWith("Test.kt") -> { + mapTestFileToSourceFile(rootDirectory, pathToRoot, changedKtFile) + } + changedKtFile.endsWith(".kt") -> changedKtFile + else -> null + } + } + .filterNotNull() + .distinct() + } + + private fun filterFiles(files: List): List { + // Filter out instrumentation files since code coverage only runs on unit tests. + return files.filter { file -> + !file + .startsWith( + "instrumentation/src/javatests/org/oppia/android/instrumentation/player", + ignoreCase = true + ) + } + } + + private fun mapTestFileToSourceFile( + rootDirectory: File, + repoRoot: String, + filePath: String + ): String? { + val repoRootFile = File(repoRoot).absoluteFile + + val possibleSourceFilePaths = when { + filePath.startsWith("scripts/") -> { + listOf(filePath.replace("/javatests/", "/java/").replace("Test.kt", ".kt")) + } + filePath.startsWith("app/") -> { + when { + filePath.contains("/sharedTest/") -> { + listOf(filePath.replace("/sharedTest/", "/main/").replace("Test.kt", ".kt")) + } + filePath.contains("/test/") -> { + listOf( + filePath.replace("/test/", "/main/").replace("Test.kt", ".kt"), + filePath.replace("/test/", "/main/").replace("LocalTest.kt", ".kt") + ) + } + else -> emptyList() + } + } + else -> { + listOf(filePath.replace("/test/", "/main/").replace("Test.kt", ".kt")) + } + } + + return possibleSourceFilePaths + .mapNotNull { path -> + val file = File(repoRootFile, path) + file.takeIf { it.exists() }?.toRelativeString(rootDirectory) + } + .firstOrNull() + } + + private fun bucketFiles(filteredFiles: List): List { + val groupedBuckets = filteredFiles.groupBy { FileBucket.retrieveCorrespondingFileBucket(it) } + .entries.groupBy( + keySelector = { checkNotNull(it.key).groupingStrategy }, + valueTransform = { checkNotNull(it.key) to it.value } + ).mapValues { (_, fileLists) -> fileLists.toMap() } + + val partitionedBuckets = groupedBuckets.flatMap { (strategy, buckets) -> + when (strategy) { + GroupingStrategy.BUCKET_SEPARATELY -> + buckets.map { (fileBucket, targets) -> + fileBucket.cacheBucketName to mapOf(fileBucket to targets) + } + GroupingStrategy.BUCKET_GENERICALLY -> listOf(GENERIC_FILE_BUCKET_NAME to buckets) + } + }.toMap() + + val shardedBuckets: Map>> = + partitionedBuckets.mapValues { (_, bucketMap) -> + val shardingStrategies = bucketMap.keys.map { it.shardingStrategy }.toSet() + check(shardingStrategies.size == 1) { + "Error: expected all buckets in the same partition to share a sharding strategy:" + + " ${bucketMap.keys} (strategies: $shardingStrategies)" + } + val maxFileCountPerShard = when (shardingStrategies.first()) { + ShardingStrategy.LARGE_PARTITIONS -> maxFileCountPerLargeShard + ShardingStrategy.MEDIUM_PARTITIONS -> maxFileCountPerMediumShard + ShardingStrategy.SMALL_PARTITIONS -> maxFileCountPerSmallShard + } + val allPartitionFiles = bucketMap.values.flatten() + + // Use randomization to encourage cache breadth & potentially improve workflow performance. + allPartitionFiles.shuffled().chunked(maxFileCountPerShard) + } + + return shardedBuckets.entries.flatMap { (bucketName, shardedFiles) -> + shardedFiles.map { files -> + ChangedFilesBucket.newBuilder().apply { + cacheBucketName = bucketName + addAllChangedFiles(files) + }.build() + } + } + } + + private enum class FileBucket( + val cacheBucketName: String, + val groupingStrategy: GroupingStrategy, + val shardingStrategy: ShardingStrategy + ) { + /** Corresponds to app layer files. */ + APP( + cacheBucketName = "app", + groupingStrategy = GroupingStrategy.BUCKET_SEPARATELY, + shardingStrategy = ShardingStrategy.SMALL_PARTITIONS + ), + + /** Corresponds to data layer files. */ + DATA( + cacheBucketName = "data", + groupingStrategy = GroupingStrategy.BUCKET_GENERICALLY, + shardingStrategy = ShardingStrategy.LARGE_PARTITIONS + ), + + /** Corresponds to domain layer files. */ + DOMAIN( + cacheBucketName = "domain", + groupingStrategy = GroupingStrategy.BUCKET_SEPARATELY, + shardingStrategy = ShardingStrategy.LARGE_PARTITIONS + ), + + /** Corresponds to instrumentation files. */ + INSTRUMENTATION( + cacheBucketName = "instrumentation", + groupingStrategy = GroupingStrategy.BUCKET_GENERICALLY, + shardingStrategy = ShardingStrategy.LARGE_PARTITIONS + ), + + /** Corresponds to scripts files. */ + SCRIPTS( + cacheBucketName = "scripts", + groupingStrategy = GroupingStrategy.BUCKET_SEPARATELY, + shardingStrategy = ShardingStrategy.MEDIUM_PARTITIONS + ), + + /** Corresponds to testing utility files. */ + TESTING( + cacheBucketName = "testing", + groupingStrategy = GroupingStrategy.BUCKET_GENERICALLY, + shardingStrategy = ShardingStrategy.LARGE_PARTITIONS + ), + + /** Corresponds to production utility files. */ + UTILITY( + cacheBucketName = "utility", + groupingStrategy = GroupingStrategy.BUCKET_GENERICALLY, + shardingStrategy = ShardingStrategy.LARGE_PARTITIONS + ); + + companion object { + private val EXTRACT_BUCKET_REGEX = "^([^/]+)".toRegex() + + /** Returns the [FileBucket] that corresponds to the specific [changedFiles]. */ + fun retrieveCorrespondingFileBucket(filePath: String): FileBucket { + return EXTRACT_BUCKET_REGEX.find(filePath) + ?.groupValues + ?.get(1) + ?.let { bucket -> + values().find { it.cacheBucketName == bucket } + ?: error("Invalid bucket name: $bucket") + } ?: error("Invalid file path: $filePath") + } + } + } + + private enum class GroupingStrategy { + /** Indicates that a particular file bucket should be sharded by itself. */ + BUCKET_SEPARATELY, + + /** + * Indicates that a particular file bucket should be combined with all other generically grouped + * buckets. + */ + BUCKET_GENERICALLY + } + + private enum class ShardingStrategy { + /** + * Indicates that the file bucket don't need as much + * parallelization. + */ + LARGE_PARTITIONS, + + /** + * Indicates that the file bucket are somewhere between [LARGE_PARTITIONS] and + * [SMALL_PARTITIONS]. + */ + MEDIUM_PARTITIONS, + + /** + * Indicates that the file bucket require more parallelization for + * faster CI runs. + */ + SMALL_PARTITIONS + } +} diff --git a/scripts/src/java/org/oppia/android/scripts/ci/RetrieveChangedFiles.kt b/scripts/src/java/org/oppia/android/scripts/ci/RetrieveChangedFiles.kt new file mode 100644 index 00000000000..86cf8cd22bc --- /dev/null +++ b/scripts/src/java/org/oppia/android/scripts/ci/RetrieveChangedFiles.kt @@ -0,0 +1,127 @@ +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 +import kotlin.system.exitProcess + +/** + * 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: + * - path_to_directory_root: directory path to the root of the Oppia Android repository. + * - 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. + * - path_to_test_target_list_output_file: path to the file where the list of changed file's test targets + * 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 $(pwd)/bazel_test_targets + */ +fun main(args: Array) { + if (args.size < 5) { + 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 fileListOutputFile = File(args[3]) + val fileTestTargetsListOutputFile = File(args[4]) + + 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) + + val changedFilesTestFiles = changedFilesBucket.changedFilesList.flatMap { changedFile -> + val exemption = testFileExemptionList[changedFile] + if (exemption != null && exemption.testFileNotRequired) { + emptyList() + } else { + findTestFile(rootDirectory, changedFile) + } + } + val changedFilesTestTargets = bazelClient.retrieveBazelTargets(changedFilesTestFiles) + val changedFilesTestTargetWithoutSuffix = changedFilesTestTargets.map { it.removeSuffix(".kt") } + + bucketNameOutputFile.printWriter().use { writer -> + writer.println(changedFilesBucket.cacheBucketName) + } + + fileListOutputFile.printWriter().use { writer -> + writer.println(changedFilesBucket.changedFilesList.joinToString(separator = " ")) + } + + 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")) + } + } + + return possibleTestFilePaths + .map { File(rootDirectory, it) } + .filter(File::exists) + .map { it.toRelativeString(rootDirectory) } +} + +private fun loadTestFileExemptionsProto(testFileExemptiontextProto: String): TestFileExemptions { + return File("$testFileExemptiontextProto.pb").inputStream().use { stream -> + TestFileExemptions.newBuilder().also { builder -> + builder.mergeFrom(stream) + }.build() + } +} diff --git a/scripts/src/java/org/oppia/android/scripts/common/GitClient.kt b/scripts/src/java/org/oppia/android/scripts/common/GitClient.kt index 159401bb81c..54cc56e2dda 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/GitClient.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/GitClient.kt @@ -27,6 +27,9 @@ class GitClient( */ val changedFiles: Set by lazy { retrieveChangedFilesWithPotentialDuplicates().toSet() } + /** The list of files that have been committed in the local branch. */ + val committedFiles: List by lazy { retrieveChangedCommittedFiles() } + private fun retrieveCurrentCommit(): String { return executeGitCommandWithOneLineOutput("rev-parse HEAD") } diff --git a/scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt b/scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt index a0d34d4f8c5..81f230ebf6b 100644 --- a/scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt +++ b/scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt @@ -63,15 +63,17 @@ fun main(vararg args: String) { val filePathList = args.drop(1) .takeWhile { !it.startsWith("--") } .map { it.trim(',', '[', ']') } - .flatMap { filePath -> + .map { filePath -> when { filePath.endsWith("Test.kt") -> { - findSourceFile(repoRoot, filePath) + findSourceFile(File(repoRoot).absoluteFile, repoRoot, filePath) } - filePath.endsWith(".kt") -> listOf(filePath) - else -> emptyList() + filePath.endsWith(".kt") -> filePath + else -> null } } + .filterNotNull() + .distinct() println("Running coverage analysis for the files: $filePathList") @@ -181,7 +183,6 @@ class RunCoverage( } private fun runCoverageForFile(filePath: String): CoverageReport { - val exemption = testFileExemptionList[filePath] return when { exemption?.testFileNotRequired == true -> { @@ -209,7 +210,7 @@ class RunCoverage( ).build() } else -> { - val testFilePaths = findTestFiles(repoRoot, filePath) + val testFilePaths = findTestFiles(rootDirectory, repoRoot, filePath) when { testFilePaths.isEmpty() -> { return CoverageReport.newBuilder() @@ -303,7 +304,13 @@ class RunCoverage( } } -private fun findTestFiles(repoRoot: String, filePath: String): List { +private fun findTestFiles( + rootDirectory: File, + repoRoot: String, + filePath: String +): List { + val repoRootFile = File(repoRoot).absoluteFile + val possibleTestFilePaths = when { filePath.startsWith("scripts/") -> { listOf(filePath.replace("/java/", "/javatests/").replace(".kt", "Test.kt")) @@ -320,15 +327,18 @@ private fun findTestFiles(repoRoot: String, filePath: String): List { } } - val repoRootFile = File(repoRoot).absoluteFile - return possibleTestFilePaths .map { File(repoRootFile, it) } .filter(File::exists) - .map { it.relativeTo(repoRootFile).path } + .map { it.toRelativeString(rootDirectory) } } -private fun findSourceFile(repoRoot: String, filePath: String): List { +private fun findSourceFile( + rootDirectory: File, + repoRoot: String, + filePath: String +): String? { + val repoRootFile = File(repoRoot).absoluteFile val possibleSourceFilePaths = when { filePath.startsWith("scripts/") -> { listOf(filePath.replace("/javatests/", "/java/").replace("Test.kt", ".kt")) @@ -344,9 +354,7 @@ private fun findSourceFile(repoRoot: String, filePath: String): List { filePath.replace("/test/", "/main/").replace("LocalTest.kt", ".kt") ) } - else -> { - emptyList() - } + else -> emptyList() } } else -> { @@ -354,12 +362,12 @@ private fun findSourceFile(repoRoot: String, filePath: String): List { } } - val repoRootFile = File(repoRoot).absoluteFile - return possibleSourceFilePaths - .map { File(repoRootFile, it) } - .filter(File::exists) - .map { it.relativeTo(repoRootFile).path } + .mapNotNull { path -> + val file = File(repoRootFile, path) + file.takeIf { it.exists() }?.toRelativeString(rootDirectory) + } + .firstOrNull() } private fun loadTestFileExemptionsProto(testFileExemptionProtoPath: String): TestFileExemptions { diff --git a/scripts/src/java/org/oppia/android/scripts/proto/BUILD.bazel b/scripts/src/java/org/oppia/android/scripts/proto/BUILD.bazel index 72956d786a5..5d710ca35ea 100644 --- a/scripts/src/java/org/oppia/android/scripts/proto/BUILD.bazel +++ b/scripts/src/java/org/oppia/android/scripts/proto/BUILD.bazel @@ -21,6 +21,17 @@ java_proto_library( deps = [":affected_tests_proto"], ) +oppia_proto_library( + name = "changed_files_proto", + srcs = ["changed_files.proto"], +) + +java_proto_library( + name = "changed_files_java_proto", + visibility = ["//scripts:oppia_script_library_visibility"], + deps = [":changed_files_proto"], +) + oppia_proto_library( name = "coverage_proto", srcs = ["coverage.proto"], diff --git a/scripts/src/java/org/oppia/android/scripts/proto/changed_files.proto b/scripts/src/java/org/oppia/android/scripts/proto/changed_files.proto new file mode 100644 index 00000000000..c65d3db0d5e --- /dev/null +++ b/scripts/src/java/org/oppia/android/scripts/proto/changed_files.proto @@ -0,0 +1,15 @@ +syntax = "proto3"; + +package proto; + +option java_package = "org.oppia.android.scripts.proto"; +option java_multiple_files = true; + +message ChangedFilesBucket { + // The name of the GitHub Actions cache that should be downloaded prior to running any targets in + // this bucket. + string cache_bucket_name = 1; + + // The list of chnaged files that belong to this bucket and should be run. + repeated string changed_files = 2; +} diff --git a/scripts/src/javatests/org/oppia/android/scripts/ci/BUILD.bazel b/scripts/src/javatests/org/oppia/android/scripts/ci/BUILD.bazel index fa1a3c2798a..6f4001a90a5 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/ci/BUILD.bazel +++ b/scripts/src/javatests/org/oppia/android/scripts/ci/BUILD.bazel @@ -31,3 +31,31 @@ kt_jvm_test( "//third_party:org_jetbrains_kotlin_kotlin-test-junit", ], ) + +kt_jvm_test( + name = "ComputeChangedFilesTest", + srcs = ["ComputeChangedFilesTest.kt"], + deps = [ + "//scripts/src/java/org/oppia/android/scripts/ci:compute_changed_files_lib", + "//scripts/src/java/org/oppia/android/scripts/common:proto_string_encoder", + "//scripts/src/java/org/oppia/android/scripts/testing:test_bazel_workspace", + "//scripts/src/java/org/oppia/android/scripts/testing:test_git_repository", + "//testing:assertion_helpers", + "//third_party:com_google_truth_truth", + "//third_party:org_jetbrains_kotlin_kotlin-test-junit", + ], +) + +kt_jvm_test( + name = "RetrieveChangedFilesTest", + srcs = ["RetrieveChangedFilesTest.kt"], + deps = [ + "//scripts:test_file_check_assets", + "//scripts/src/java/org/oppia/android/scripts/ci:retrieve_changed_files_lib", + "//scripts/src/java/org/oppia/android/scripts/common:proto_string_encoder", + "//scripts/src/java/org/oppia/android/scripts/testing:test_bazel_workspace", + "//testing:assertion_helpers", + "//third_party:com_google_truth_truth", + "//third_party:org_jetbrains_kotlin_kotlin-test-junit", + ], +) diff --git a/scripts/src/javatests/org/oppia/android/scripts/ci/ComputeChangedFilesTest.kt b/scripts/src/javatests/org/oppia/android/scripts/ci/ComputeChangedFilesTest.kt new file mode 100644 index 00000000000..b66667901ee --- /dev/null +++ b/scripts/src/javatests/org/oppia/android/scripts/ci/ComputeChangedFilesTest.kt @@ -0,0 +1,1054 @@ +package org.oppia.android.scripts.ci + +import com.google.common.truth.Truth.assertThat +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.oppia.android.scripts.common.CommandExecutor +import org.oppia.android.scripts.common.CommandExecutorImpl +import org.oppia.android.scripts.common.GitClient +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.testing.TestBazelWorkspace +import org.oppia.android.scripts.testing.TestGitRepository +import org.oppia.android.testing.assertThrows +import java.io.ByteArrayOutputStream +import java.io.File +import java.io.OutputStream +import java.io.PrintStream +import java.util.concurrent.TimeUnit + +/** + * Tests for the compute_changed_files utility. + */ +class ComputeChangedFilesTest { + @field:[Rule JvmField] val tempFolder = TemporaryFolder() + + private val scriptBgDispatcher by lazy { ScriptBackgroundCoroutineDispatcher() } + + private lateinit var commandExecutor: CommandExecutor + private lateinit var testBazelWorkspace: TestBazelWorkspace + private lateinit var testGitRepository: TestGitRepository + private lateinit var pendingOutputStream: ByteArrayOutputStream + private lateinit var originalStandardOutputStream: OutputStream + + @Before + fun setUp() { + commandExecutor = initializeCommandExecutorWithLongProcessWaitTime() + testBazelWorkspace = TestBazelWorkspace(tempFolder) + testGitRepository = TestGitRepository(tempFolder, commandExecutor) + + // Redirect script output for testing purposes. + pendingOutputStream = ByteArrayOutputStream() + originalStandardOutputStream = System.out + System.setOut(PrintStream(pendingOutputStream)) + } + + @After + fun tearDown() { + // Reinstate test output redirection. + System.setOut(PrintStream(pendingOutputStream)) + scriptBgDispatcher.close() + } + + @Test + fun testUtility_noArguments_printsUsageStringAndExits() { + val exception = assertThrows() { main(arrayOf()) } + + // Bazel catches the System.exit() call and throws a SecurityException. This is a bit hacky way + // to verify that System.exit() is called, but it's helpful. + assertThat(exception).hasMessageThat().contains("System.exit()") + assertThat(pendingOutputStream.toString()).contains("Usage:") + } + + @Test + fun testUtility_oneArgument_printsUsageStringAndExits() { + val exception = assertThrows() { main(arrayOf("first")) } + + // Bazel catches the System.exit() call and throws a SecurityException. This is a bit hacky way + // to verify that System.exit() is called, but it's helpful. + assertThat(exception).hasMessageThat().contains("System.exit()") + assertThat(pendingOutputStream.toString()).contains("Usage:") + } + + @Test + fun testUtility_twoArguments_printsUsageStringAndExits() { + val exception = assertThrows() { main(arrayOf("first", "second")) } + + // Bazel catches the System.exit() call and throws a SecurityException. This is a bit hacky way + // to verify that System.exit() is called, but it's helpful. + assertThat(exception).hasMessageThat().contains("System.exit()") + assertThat(pendingOutputStream.toString()).contains("Usage:") + } + + @Test + fun testUtility_threeArguments_printsUsageStringAndExits() { + val exception = assertThrows() { main(arrayOf("first", "second", "three")) } + + // Bazel catches the System.exit() call and throws a SecurityException. This is a bit hacky way + // to verify that System.exit() is called, but it's helpful. + assertThat(exception).hasMessageThat().contains("System.exit()") + assertThat(pendingOutputStream.toString()).contains("Usage:") + } + + @Test + fun testUtility_directoryRootDoesNotExist_throwsException() { + val exception = assertThrows() { + main(arrayOf("fake", "alsofake", "andstillfake", "compute_all_files=false")) + } + + assertThat(exception).hasMessageThat().contains("Expected 'fake' to be a directory") + } + + @Test + fun testUtility_invalid_lastArgument_throwsException() { + val exception = assertThrows() { + main(arrayOf("fake", "alsofake", "andstillfake", "compute_all_filess=false")) + } + + assertThat(exception).hasMessageThat() + .contains("Expected last argument to start with 'compute_all_files='") + } + + @Test + fun testUtility_invalid_lastArgumentValue_throwsException() { + val exception = assertThrows() { + main(arrayOf("fake", "alsofake", "andstillfake", "compute_all_files=blah")) + } + + assertThat(exception).hasMessageThat() + .contains("Expected last argument to have 'true' or 'false' passed to it, not: 'blah'") + } + + @Test + fun testUtility_emptyDirectory_throwsException() { + val exception = assertThrows() { runScript(currentHeadHash = "ad") } + + assertThat(exception).hasMessageThat().contains("run from the workspace's root directory") + } + + @Test + fun testUtility_emptyWorkspace_returnsNoTargets() { + // Need to be on a feature branch since the develop branch expects there to be files. + initializeEmptyGitRepository() + switchToFeatureBranch() + createEmptyWorkspace() + + val reportedFiles = runScript() + + // An empty workspace should yield no files. + assertThat(reportedFiles).isEmpty() + } + + @Test + fun testUtility_developBranch_returnsAllFiles() { + initializeEmptyGitRepository() + createEmptyWorkspace() + createAndCommitFiles("First", "Second", "Third", subPackage = "app") + + val reportedFiles = runScript() + + // Since the develop branch is checked out, all files should be returned. + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly("app/First.kt", "app/Second.kt", "app/Third.kt") + } + + @Test + fun testUtility_featureBranch_noChanges_returnsNoFiles() { + initializeEmptyGitRepository() + createAndCommitFiles("First", "Second", "Third", subPackage = "app") + switchToFeatureBranch() + + val reportedFiles = runScript() + + assertThat(reportedFiles).isEmpty() + } + + @Test + fun testUtility_featureBranch_noChanges_computeAllFiles_returnsAllFiles() { + initializeEmptyGitRepository() + createAndCommitFiles("First", "Second", "Third", subPackage = "app") + switchToFeatureBranch() + + val reportedFiles = runScript(computeAllFiles = true) + + // Even though there are no changes, all files should be returned since that was requested via + // a command argument. + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly("app/First.kt", "app/Second.kt", "app/Third.kt") + } + + @Test + fun testUtility_featureBranch_fileChange_committed_returnsChangedFile() { + initializeEmptyGitRepository() + createAndCommitFiles("First", "Second", "Third", subPackage = "app") + switchToFeatureBranch() + changeAndCommitFile("First", subPackage = "app") + + val reportedFiles = runScript() + + // Only the first file should be reported since the file itself was changed & committed. + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList).containsExactly("app/First.kt") + } + + @Test + fun testUtility_featureBranch_fileChangeStaged_shouldNotBeComputed() { + initializeEmptyGitRepository() + createAndCommitFiles("First", "Second", "Third", subPackage = "app") + switchToFeatureBranch() + changeAndStageFile("First", subPackage = "app") + + val reportedFiles = runScript() + + // No files should be reported since the file was only staged and + // not fully committed in the feature branch. + assertThat(reportedFiles).isEmpty() + } + + @Test + fun testUtility_featureBranch_fileChangeUnstaged_shouldNotBeComputed() { + initializeEmptyGitRepository() + createAndCommitFiles("First", "Second", "Third", subPackage = "app") + switchToFeatureBranch() + changeAndCommitFile("First", subPackage = "app") + + val reportedFiles = runScript() + + // Only the first file should be reported because it is the only one + // that was changed and committed in the feature branch. + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList).containsExactly("app/First.kt") + } + + @Test + fun testUtility_featureBranch_newFile_untracked_shouldNotBeComputed() { + initializeEmptyGitRepository() + createAndCommitFiles("First", "Second", "Third", subPackage = "app") + switchToFeatureBranch() + createFiles("NewUntrackedFile", subPackage = "data") + + val reportedFiles = runScript() + + // No files should be reported since the file was untracked and + // not fully committed in the feature branch. + assertThat(reportedFiles).isEmpty() + } + + @Test + fun testUtility_featureBranch_deletedFile_committed_returnsNoFiles() { + initializeEmptyGitRepository() + createAndCommitFiles("First", subPackage = "app") + switchToFeatureBranch() + removeAndCommitFile("First", subPackage = "app") + + val reportedFiles = runScript() + + // Removing the file should result in no files being returned (since the file is gone). + assertThat(reportedFiles).isEmpty() + } + + @Test + fun testUtility_featureBranch_movedFile_stagedAndCommitted_returnsNewFile() { + initializeEmptyGitRepository() + createAndCommitFiles("First", subPackage = "app") + switchToFeatureBranch() + moveFile( + oldFileName = "First", + oldSubPackage = "app", + newFileName = "RenamedFile", + newSubPackage = "domain" + ) + + val reportedFiles = runScript() + + // The file should show up under its new name since moving it is the same as changing it. + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList).containsExactly("domain/RenamedFile.kt") + } + + @Test + fun testUtility_featureBranch_multipleFilesChanged_committed_returnsChangedFiles() { + initializeEmptyGitRepository() + createAndCommitFiles("First", "Second", "Third", subPackage = "app") + switchToFeatureBranch() + changeAndCommitFile("First", subPackage = "app") + changeAndCommitFile("Third", subPackage = "app") + + val reportedFiles = runScript() + + // Changing multiple files independently should be reflected in the script's results. + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly("app/First.kt", "app/Third.kt") + } + + @Test + fun testUtility_developBranch_instrumentationModuleChanged_filteredFilesAreIgnored() { + initializeEmptyGitRepository() + createAndCommitFiles( + "InstrumentationFile", + subPackage = "instrumentation/src/javatests/org/oppia/android/instrumentation/player" + ) + + val reportedFiles = runScript() + + assertThat(reportedFiles).isEmpty() + } + + @Test + fun testUtility_developBranch_instrumentationModuleChanged_unfilteredFilesAreComputed() { + initializeEmptyGitRepository() + createAndCommitFiles( + "Robolectric", + subPackage = "instrumentation/src/javatests/org/oppia/android/instrumentation/app" + ) + createAndCommitFiles( + "Third", + subPackage = "instrumentation" + ) + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList).contains( + "instrumentation/src/javatests/org/oppia/android/instrumentation/app/Robolectric.kt" + ) + assertThat(reportedFiles.first().changedFilesList).contains( + "instrumentation/Third.kt" + ) + } + + @Test + fun testUtility_featureBranch_instrumentationModuleChanged_filteredFilesAreIgnored() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles( + "InstrumentationFile", + subPackage = "instrumentation/src/javatests/org/oppia/android/instrumentation/player" + ) + + val reportedFiles = runScript() + + assertThat(reportedFiles).isEmpty() + } + + @Test + fun testUtility_featureBranch_instrumentationModuleChanged_unfilteredFilesAreComputed() { + initializeEmptyGitRepository() + createAndCommitFiles("First", "Second", subPackage = "app") + switchToFeatureBranch() + createAndCommitFiles( + "Robolectric", + subPackage = "instrumentation/src/javatests/org/oppia/android/instrumentation/app" + ) + createAndCommitFiles( + "Third", + subPackage = "instrumentation" + ) + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList).contains( + "instrumentation/src/javatests/org/oppia/android/instrumentation/app/Robolectric.kt" + ) + assertThat(reportedFiles.first().changedFilesList).contains( + "instrumentation/Third.kt" + ) + } + + @Test + fun testUtility_appFile_usesAppCacheName() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles("Example", subPackage = "app") + + val reportedTargets = runScript() + + assertThat(reportedTargets).hasSize(1) + assertThat(reportedTargets.first().cacheBucketName).isEqualTo("app") + } + + @Test + fun testUtility_dataFile_usesGenericCacheName() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles("Example", subPackage = "data") + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().cacheBucketName).isEqualTo("generic") + } + + @Test + fun testUtility_domainFile_usesDomainCacheName() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles("Example", subPackage = "domain") + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().cacheBucketName).isEqualTo("domain") + } + + @Test + fun testUtility_instrumentationFile_usesGenericCacheName() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles("Example", subPackage = "instrumentation") + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().cacheBucketName).isEqualTo("generic") + } + + @Test + fun testUtility_scriptsFile_usesScriptsCacheName() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles("Example", subPackage = "scripts") + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().cacheBucketName).isEqualTo("scripts") + } + + @Test + fun testUtility_testingFile_usesGenericCacheName() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles("Example", subPackage = "testing") + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().cacheBucketName).isEqualTo("generic") + } + + @Test + fun testUtility_utilityFile_usesGenericCacheName() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles("Example", subPackage = "utility") + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().cacheBucketName).isEqualTo("generic") + } + + @Test + fun testUtility_testsForMultipleBuckets_correctlyGroupTogether() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles("AppFile", subPackage = "app") + createAndCommitFiles("DataFile", subPackage = "data") + createAndCommitFiles("DomainFile", subPackage = "domain") + createAndCommitFiles("InstrumentationFile", subPackage = "instrumentation") + createAndCommitFiles("ScriptsFile", subPackage = "scripts") + createAndCommitFiles("TestingFile", subPackage = "testing") + createAndCommitFiles("UtilityFile", subPackage = "utility") + + val reportedFiles = runScript() + + // Turn the files into a map by cache name in order to guard against potential randomness from + // the script. + val fileMap = reportedFiles.associateBy { it.cacheBucketName } + assertThat(reportedFiles).hasSize(4) + assertThat(fileMap).hasSize(4) + assertThat(fileMap).containsKey("app") + assertThat(fileMap).containsKey("domain") + assertThat(fileMap).containsKey("generic") + assertThat(fileMap).containsKey("scripts") + // Verify that dedicated groups only have their relevant files & the generic group includes all + // other files. + assertThat(fileMap["app"]?.changedFilesList).containsExactly("app/AppFile.kt") + assertThat(fileMap["domain"]?.changedFilesList).containsExactly("domain/DomainFile.kt") + assertThat(fileMap["generic"]?.changedFilesList) + .containsExactly( + "data/DataFile.kt", "instrumentation/InstrumentationFile.kt", "testing/TestingFile.kt", + "utility/UtilityFile.kt" + ) + assertThat(fileMap["scripts"]?.changedFilesList) + .containsExactly("scripts/ScriptsFile.kt") + } + + @Test + fun testUtility_appFiles_shardWithSmallPartitions() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles("AppFile1", "AppFile2", "AppFile3", subPackage = "app") + + val reportedFiles = runScriptWithShardLimits( + maxFileCountPerLargeShard = 3, + maxFileCountPerMediumShard = 2, + maxFileCountPerSmallShard = 1 + ) + + // App module files partition eagerly, so there should be 3 groups. Also, the code below + // verifies duplicates by ensuring no shards are empty and there are no duplicate files. Note + // that it's done in this way to be resilient against potential randomness from the script. + val allFiles = reportedFiles.flatMap { it.changedFilesList } + assertThat(reportedFiles).hasSize(3) + assertThat(reportedFiles[0].changedFilesList).isNotEmpty() + assertThat(reportedFiles[1].changedFilesList).isNotEmpty() + assertThat(reportedFiles[2].changedFilesList).isNotEmpty() + assertThat(allFiles).containsExactly("app/AppFile1.kt", "app/AppFile2.kt", "app/AppFile3.kt") + } + + @Test + fun testUtility_dataFiles_shardWithLargePartitions() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles("DataFile1", "DataFile2", "DataFile3", subPackage = "data") + + val reportedFiles = runScriptWithShardLimits( + maxFileCountPerLargeShard = 3, + maxFileCountPerMediumShard = 2, + maxFileCountPerSmallShard = 1 + ) + + // Data files are partitioned such that they are combined into one partition. + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly("data/DataFile1.kt", "data/DataFile2.kt", "data/DataFile3.kt") + } + + @Test + fun testUtility_domainFiles_shardWithLargePartitions() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles("DomainFile1", "DomainFile2", "DomainFile3", subPackage = "domain") + + val reportedFiles = runScriptWithShardLimits( + maxFileCountPerLargeShard = 3, + maxFileCountPerMediumShard = 2, + maxFileCountPerSmallShard = 1 + ) + + // Domain files are partitioned such that they are combined into one partition. + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly("domain/DomainFile1.kt", "domain/DomainFile2.kt", "domain/DomainFile3.kt") + } + + @Test + fun testUtility_instrumentationFiles_shardWithLargePartitions() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles( + "InstrumentationFile1", "InstrumentationFile2", "InstrumentationFile3", + subPackage = "instrumentation" + ) + + val reportedFiles = runScriptWithShardLimits( + maxFileCountPerLargeShard = 3, + maxFileCountPerMediumShard = 2, + maxFileCountPerSmallShard = 1 + ) + + // Instrumentation files are partitioned such that they are combined into one partition. + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly( + "instrumentation/InstrumentationFile1.kt", "instrumentation/InstrumentationFile2.kt", + "instrumentation/InstrumentationFile3.kt" + ) + } + + @Test + fun testUtility_scriptsFiles_shardWithMediumPartitions() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles("ScriptsFile1", "ScriptsFile2", "ScriptsFile3", subPackage = "scripts") + + val reportedFiles = runScriptWithShardLimits( + maxFileCountPerLargeShard = 3, + maxFileCountPerMediumShard = 2, + maxFileCountPerSmallShard = 1 + ) + + // See app module file above for specifics. Scripts files are medium partitioned which means 3 + // files will be split into two partitions. + val allFiles = reportedFiles.flatMap { it.changedFilesList } + assertThat(reportedFiles).hasSize(2) + assertThat(reportedFiles[0].changedFilesList).isNotEmpty() + assertThat(reportedFiles[1].changedFilesList).isNotEmpty() + assertThat(allFiles) + .containsExactly( + "scripts/ScriptsFile1.kt", + "scripts/ScriptsFile2.kt", + "scripts/ScriptsFile3.kt" + ) + } + + @Test + fun testUtility_testingFiles_shardWithLargePartitions() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles("TestingFile1", "TestingFile2", "TestingFile3", subPackage = "testing") + + val reportedFiles = runScriptWithShardLimits( + maxFileCountPerLargeShard = 3, + maxFileCountPerMediumShard = 2, + maxFileCountPerSmallShard = 1 + ) + + // Testing files are partitioned such that they are combined into one partition. + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly( + "testing/TestingFile1.kt", + "testing/TestingFile2.kt", + "testing/TestingFile3.kt" + ) + } + + @Test + fun testUtility_utilityFiles_shardWithLargePartitions() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles("UtilityFile1", "UtilityFile2", "UtilityFile3", subPackage = "utility") + + val reportedFiles = runScriptWithShardLimits( + maxFileCountPerLargeShard = 3, + maxFileCountPerMediumShard = 2, + maxFileCountPerSmallShard = 1 + ) + + // Utility tests are partitioned such that they are combined into one partition. + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly( + "utility/UtilityFile1.kt", + "utility/UtilityFile2.kt", + "utility/UtilityFile3.kt" + ) + } + + @Test + fun testUtility_singleShard_fileOutputIncludesHumanReadablePrefix() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles("ExampleFile", subPackage = "app") + + val generatedLines = runScriptWithTextOutput() + + assertThat(generatedLines).hasSize(1) + assertThat(generatedLines.first()).startsWith("app-shard0") + } + + @Test + fun testUtility_twoShards_computesFilesForBothShards() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles("AppFile1", "AppFile2", "AppFile3", subPackage = "app") + createAndCommitFiles("ScriptsFile1", "ScriptsFile2", subPackage = "scripts") + + val reportedFiles = runScript() + + // Turn the files into a map by cache name in order to guard against potential randomness from + // the script. + val filetMap = reportedFiles.associateBy { it.cacheBucketName } + assertThat(reportedFiles).hasSize(2) + assertThat(filetMap).hasSize(2) + assertThat(filetMap).containsKey("app") + assertThat(filetMap).containsKey("scripts") + assertThat(filetMap["app"]?.changedFilesList) + .containsExactly("app/AppFile1.kt", "app/AppFile2.kt", "app/AppFile3.kt") + assertThat(filetMap["scripts"]?.changedFilesList) + .containsExactly("scripts/ScriptsFile1.kt", "scripts/ScriptsFile2.kt") + } + + @Test + fun testUtility_multipleShards_fileOutputIncludesHumanReadablePrefixForEachShard() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles("AppFile", subPackage = "app") + createAndCommitFiles("ScriptsFile", subPackage = "scripts") + + // The sorting here counteracts the intentional randomness from the script. + val generatedLines = runScriptWithTextOutput().sorted() + + assertThat(generatedLines).hasSize(2) + assertThat(generatedLines[0]).matches("^app-shard[0-3];.+?$") + assertThat(generatedLines[1]).matches("^scripts-shard[0-3];.+?\$") + } + + @Test + fun testUtility_featureBranch_computeAllFiles_filtersOutNonKotlinFiles() { + initializeEmptyGitRepository() + createAndCommitFiles("First", "Second", "Third", subPackage = "app") + createNonKotlinFiles("Fourth", subPackage = "app") + switchToFeatureBranch() + + val reportedFiles = runScript(computeAllFiles = true) + + // Even though there are no changes, all files should be returned since that was requested + // via a command argument. + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly("app/First.kt", "app/Second.kt", "app/Third.kt") + assertThat(reportedFiles.first().changedFilesList).doesNotContain("app/Fourth.xml") + } + + @Test + fun testUtility_featureBranch_computeAllFiles_filtersOutTestFiles() { + initializeEmptyGitRepository() + createAndCommitFiles("First", "SecondTest", "Third", subPackage = "app") + switchToFeatureBranch() + + val reportedFiles = runScript(computeAllFiles = true) + + // Even though there are no changes, all files should be returned since that was requested + // via a command argument. + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly("app/First.kt", "app/Third.kt") + assertThat(reportedFiles.first().changedFilesList).doesNotContain("app/SecondTest.kt") + } + + @Test + fun testUtility_featureBranch_filtersOutNonKotlinFiles() { + initializeEmptyGitRepository() + switchToFeatureBranch() + createAndCommitFiles("First", "Second", "Third", subPackage = "app") + createNonKotlinFiles("Fourth", subPackage = "app") + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly("app/First.kt", "app/Second.kt", "app/Third.kt") + assertThat(reportedFiles.first().changedFilesList).doesNotContain("app/Fourth.xml") + } + + @Test + fun testUtility_featureBranch_filtersOutTestFiles() { + initializeEmptyGitRepository() + createAndCommitFiles("First", "SecondTest", "Third", subPackage = "app") + switchToFeatureBranch() + changeAndCommitFile("First", subPackage = "app") + changeAndCommitFile("SecondTest", subPackage = "app") + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly("app/First.kt") + assertThat(reportedFiles.first().changedFilesList).doesNotContain("app/SecondTest.kt") + } + + @Test + fun testUtility_featureBranch_withChangesToTestFile_mapsTestFileToSourceFile() { + initializeEmptyGitRepository() + createAndCommitFiles("First", subPackage = "app/main/java/com/src") + createAndCommitFiles("FirstTest", subPackage = "app/test/java/com/src") + switchToFeatureBranch() + changeAndCommitFile("FirstTest", subPackage = "app/test/java/com/src") + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly("app/main/java/com/src/First.kt") + } + + @Test + fun testUtility_featureBranch_appTestChanges_mapsToRespectiveSourceFile() { + initializeEmptyGitRepository() + createAndCommitFiles("First", subPackage = "app/main/java/com/src") + createAndCommitFiles("FirstTest", subPackage = "app/test/java/com/src") + switchToFeatureBranch() + changeAndCommitFile("FirstTest", subPackage = "app/test/java/com/src") + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly("app/main/java/com/src/First.kt") + } + + @Test + fun testUtility_featureBranch_appLocalTestChanges_mapsToRespectiveSourceFile() { + initializeEmptyGitRepository() + createAndCommitFiles("First", subPackage = "app/main/java/com/src") + createAndCommitFiles("FirstLocalTest", subPackage = "app/test/java/com/src") + switchToFeatureBranch() + changeAndCommitFile("FirstLocalTest", subPackage = "app/test/java/com/src") + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly("app/main/java/com/src/First.kt") + } + + @Test + fun testUtility_featureBranch_appSharedTestChanges_mapsToRespectiveSourceFile() { + initializeEmptyGitRepository() + createAndCommitFiles("First", subPackage = "app/main/java/com/src") + createAndCommitFiles("FirstTest", subPackage = "app/sharedTest/java/com/src") + switchToFeatureBranch() + changeAndCommitFile("FirstTest", subPackage = "app/sharedTest/java/com/src") + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly("app/main/java/com/src/First.kt") + } + + @Test + fun testUtility_featureBranch_scriptTestChanges_mapsToRespectiveSourceFile() { + initializeEmptyGitRepository() + createAndCommitFiles("First", subPackage = "scripts/java/com/src") + createAndCommitFiles("FirstTest", subPackage = "scripts/javatests/com/src") + switchToFeatureBranch() + changeAndCommitFile("FirstTest", subPackage = "scripts/javatests/com/src") + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly("scripts/java/com/src/First.kt") + } + + @Test + fun testUtility_featureBranch_domainModuleTestChanges_mapsToRespectiveSourceFile() { + initializeEmptyGitRepository() + createAndCommitFiles("First", subPackage = "domain/main/java/com/src") + createAndCommitFiles("FirstTest", subPackage = "domain/test/java/com/src") + switchToFeatureBranch() + changeAndCommitFile("FirstTest", subPackage = "domain/test/java/com/src") + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly("domain/main/java/com/src/First.kt") + } + + @Test + fun testUtility_featureBranch_dataModuleTestChanges_mapsToRespectiveSourceFile() { + initializeEmptyGitRepository() + createAndCommitFiles("First", subPackage = "data/main/java/com/src") + createAndCommitFiles("FirstTest", subPackage = "data/test/java/com/src") + switchToFeatureBranch() + changeAndCommitFile("FirstTest", subPackage = "data/test/java/com/src") + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly("data/main/java/com/src/First.kt") + } + + @Test + fun testUtility_featureBranch_utilityModuleTestChanges_mapsToRespectiveSourceFile() { + initializeEmptyGitRepository() + createAndCommitFiles("First", subPackage = "utility/main/java/com/src") + createAndCommitFiles("FirstTest", subPackage = "utility/test/java/com/src") + switchToFeatureBranch() + changeAndCommitFile("FirstTest", subPackage = "utility/test/java/com/src") + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly("utility/main/java/com/src/First.kt") + } + + @Test + fun testUtility_featureBranch_testingModuleTestChanges_mapsToRespectiveSourceFile() { + initializeEmptyGitRepository() + createAndCommitFiles("First", subPackage = "testing/main/java/com/src") + createAndCommitFiles("FirstTest", subPackage = "testing/test/java/com/src") + switchToFeatureBranch() + changeAndCommitFile("FirstTest", subPackage = "testing/test/java/com/src") + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly("testing/main/java/com/src/First.kt") + } + + @Test + fun testUtility_featureBranch_withChangesToSourceAndTestFile_computesSingleSourceFile() { + initializeEmptyGitRepository() + createAndCommitFiles("First", subPackage = "app/main/java/com/src") + createAndCommitFiles("FirstTest", subPackage = "app/test/java/com/src") + switchToFeatureBranch() + changeAndCommitFile("First", subPackage = "app/main/java/com/src") + changeAndCommitFile("FirstTest", subPackage = "app/test/java/com/src") + + val reportedFiles = runScript() + + assertThat(reportedFiles).hasSize(1) + assertThat(reportedFiles.first().changedFilesList) + .containsExactly("app/main/java/com/src/First.kt") + } + + private fun runScriptWithTextOutput( + currentHeadHash: String = computeMergeBase("develop"), + computeAllFiles: Boolean = false + ): List { + val outputLog = tempFolder.newFile("output.log") + main( + arrayOf( + tempFolder.root.absolutePath, + outputLog.absolutePath, + currentHeadHash, + "compute_all_files=$computeAllFiles" + ) + ) + return outputLog.readLines() + } + + /** + * Runs the compute_affected_files utility & returns all of the output lines. + * Note that the output here is that which is saved directly to the output file, + * not debug lines printed to the console. + */ + private fun runScript( + currentHeadHash: String = computeMergeBase("develop"), + computeAllFiles: Boolean = false + ): List { + return parseOutputLogLines(runScriptWithTextOutput(currentHeadHash, computeAllFiles)) + } + + private fun runScriptWithShardLimits( + baseBranch: String = "develop", + maxFileCountPerLargeShard: Int, + maxFileCountPerMediumShard: Int, + maxFileCountPerSmallShard: Int + ): List { + val outputLog = tempFolder.newFile("output.log") + val currentHeadHash = computeMergeBase(baseBranch) + + // Note that main() can't be used since the shard counts need to be overwritten. Dagger would + // be a nicer means to do this, but it's not set up currently for scripts. + ComputeChangedFiles( + scriptBgDispatcher, + maxFileCountPerLargeShard = maxFileCountPerLargeShard, + maxFileCountPerMediumShard = maxFileCountPerMediumShard, + maxFileCountPerSmallShard = maxFileCountPerSmallShard, + commandExecutor = commandExecutor + ).compute( + pathToRoot = tempFolder.root.absolutePath, + pathToOutputFile = outputLog.absolutePath, + baseCommit = currentHeadHash, + computeAllFilesSetting = false + ) + + return parseOutputLogLines(outputLog.readLines()) + } + + private fun parseOutputLogLines(outputLogLines: List): List { + return outputLogLines.map { + ChangedFilesBucket.getDefaultInstance().mergeFromCompressedBase64(it.split(";")[1]) + } + } + + private fun createEmptyWorkspace() { + testBazelWorkspace.initEmptyWorkspace() + } + + private fun initializeEmptyGitRepository() { + // Initialize the git repository with a base 'develop' branch & an initial empty commit + // (so that there's a HEAD commit). + testGitRepository.init() + testGitRepository.setUser(email = "test@oppia.org", name = "Test User") + testGitRepository.checkoutNewBranch("develop") + testGitRepository.commit(message = "Initial commit.", allowEmpty = true) + } + + private fun createFiles(vararg fileNames: String, subPackage: String): List { + createEmptyWorkspace() + if (!File(tempFolder.root, subPackage).exists()) { + tempFolder.newFolder(subPackage) + } + return fileNames.map { fileName -> + tempFolder.newFile("$subPackage/$fileName.kt") + } + } + + private fun createNonKotlinFiles(vararg fileNames: String, subPackage: String): List { + createEmptyWorkspace() + if (!File(tempFolder.root, subPackage).exists()) { + tempFolder.newFolder(subPackage) + } + return fileNames.map { fileName -> + tempFolder.newFile("$subPackage/$fileName.xml") + } + } + + private fun createAndCommitFiles(vararg fileNames: String, subPackage: String) { + val createdFiles = createFiles(fileNames = fileNames, subPackage = subPackage) + + testGitRepository.stageFilesForCommit(createdFiles) + testGitRepository.commit(message = "Introduce files.") + } + + private fun changeFile(fileName: String, subPackage: String): File { + val file = File(tempFolder.root, "$subPackage/$fileName.kt") + file.appendText(";") // Add a character to change the file. + return file + } + + private fun changeAndStageFile(fileName: String, subPackage: String) { + val file = changeFile(fileName, subPackage) + testGitRepository.stageFileForCommit(file) + } + + private fun changeAndCommitFile(fileName: String, subPackage: String) { + changeAndStageFile(fileName, subPackage) + testGitRepository.commit(message = "Modified file $fileName") + } + + private fun moveFile( + oldFileName: String, + oldSubPackage: String, + newFileName: String, + newSubPackage: String + ) { + val oldFilePath = File(tempFolder.root, "$oldSubPackage/$oldFileName.kt") + val newFilePath = File(tempFolder.root, "$newSubPackage/$newFileName.kt") + + oldFilePath.copyTo(newFilePath) + oldFilePath.delete() + + testGitRepository.stageFileForCommit(newFilePath) + testGitRepository.commit(message = "Move file from $oldFilePath to $newFilePath") + } + + private fun removeAndCommitFile(fileName: String, subPackage: String) { + val file = File(tempFolder.root, "$subPackage/$fileName.kt") + testGitRepository.removeFileForCommit(file) + testGitRepository.commit(message = "Remove file $fileName") + } + + private fun switchToFeatureBranch() { + testGitRepository.checkoutNewBranch("introduce-feature") + } + + private fun computeMergeBase(referenceBranch: String): String = + GitClient(tempFolder.root, referenceBranch, commandExecutor).branchMergeBase + + private fun initializeCommandExecutorWithLongProcessWaitTime(): CommandExecutorImpl { + return CommandExecutorImpl( + scriptBgDispatcher, processTimeout = 5, processTimeoutUnit = TimeUnit.MINUTES + ) + } +} diff --git a/scripts/src/javatests/org/oppia/android/scripts/ci/RetrieveChangedFilesTest.kt b/scripts/src/javatests/org/oppia/android/scripts/ci/RetrieveChangedFilesTest.kt new file mode 100644 index 00000000000..0fa6b8c283a --- /dev/null +++ b/scripts/src/javatests/org/oppia/android/scripts/ci/RetrieveChangedFilesTest.kt @@ -0,0 +1,295 @@ +package org.oppia.android.scripts.ci + +import com.google.common.truth.Truth.assertThat +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.oppia.android.scripts.common.CommandExecutor +import org.oppia.android.scripts.common.CommandExecutorImpl +import org.oppia.android.scripts.common.ProtoStringEncoder.Companion.toCompressedBase64 +import org.oppia.android.scripts.common.ScriptBackgroundCoroutineDispatcher +import org.oppia.android.scripts.proto.ChangedFilesBucket +import org.oppia.android.scripts.testing.TestBazelWorkspace +import org.oppia.android.testing.assertThrows +import java.io.ByteArrayOutputStream +import java.io.File +import java.io.OutputStream +import java.io.PrintStream +import java.util.concurrent.TimeUnit + +/** Tests for the retrieve_changed_files utility. */ +class RetrieveChangedFilesTest { + @field:[Rule JvmField] val tempFolder = TemporaryFolder() + + private val scriptBgDispatcher by lazy { ScriptBackgroundCoroutineDispatcher() } + private lateinit var commandExecutor: CommandExecutor + private lateinit var testBazelWorkspace: TestBazelWorkspace + private lateinit var pendingOutputStream: ByteArrayOutputStream + private lateinit var originalStandardOutputStream: OutputStream + + @Before + fun setUp() { + commandExecutor = initializeCommandExecutorWithLongProcessWaitTime() + testBazelWorkspace = TestBazelWorkspace(tempFolder) + + // Redirect script output for testing purposes. + pendingOutputStream = ByteArrayOutputStream() + originalStandardOutputStream = System.out + System.setOut(PrintStream(pendingOutputStream)) + } + + @After + fun tearDown() { + // Reinstate test output redirection. + System.setOut(PrintStream(pendingOutputStream)) + + // Print the status of the git repository to help with debugging in the cases of test failures + // and to help manually verify the expect git state at the end of each test. + println("git status (at end of test):") + + scriptBgDispatcher.close() + } + + @Test + fun testUtility_noArguments_printsUsageStringAndExits() { + val exception = assertThrows() { runScript() } + + // Bazel catches the System.exit() call and throws a SecurityException. This is a bit hacky way + // to verify that System.exit() is called, but it's helpful. + assertThat(exception).hasMessageThat().contains("System.exit()") + assertThat(pendingOutputStream.toString()).contains("Usage:") + } + + @Test + fun testUtility_oneArgument_printsUsageStringAndExits() { + val exception = assertThrows() { runScript("arg1") } + + // Bazel catches the System.exit() call and throws a SecurityException. This is a bit hacky way + // to verify that System.exit() is called, but it's helpful. + assertThat(exception).hasMessageThat().contains("System.exit()") + assertThat(pendingOutputStream.toString()).contains("Usage:") + } + + @Test + fun testUtility_twoArguments_printsUsageStringAndExits() { + val exception = assertThrows() { runScript("arg1", "arg2") } + + // Bazel catches the System.exit() call and throws a SecurityException. This is a bit hacky way + // to verify that System.exit() is called, but it's helpful. + assertThat(exception).hasMessageThat().contains("System.exit()") + assertThat(pendingOutputStream.toString()).contains("Usage:") + } + + @Test + fun testUtility_threeArguments_printsUsageStringAndExits() { + val exception = assertThrows() { runScript("arg1", "arg2", "arg3") } + + // Bazel catches the System.exit() call and throws a SecurityException. This is a bit hacky way + // to verify that System.exit() is called, but it's helpful. + assertThat(exception).hasMessageThat().contains("System.exit()") + assertThat(pendingOutputStream.toString()).contains("Usage:") + } + + @Test + fun testUtility_fourArguments_printsUsageStringAndExits() { + val exception = assertThrows() { runScript("arg1", "arg2", "arg3", "arg4") } + + // Bazel catches the System.exit() call and throws a SecurityException. This is a bit hacky way + // to verify that System.exit() is called, but it's helpful. + assertThat(exception).hasMessageThat().contains("System.exit()") + assertThat(pendingOutputStream.toString()).contains("Usage:") + } + + @Test + fun testUtility_invalidBase64_throwsException() { + val exception = assertThrows() { + runScript( + "${tempFolder.root}", + "badbase64", + "file1", + "file2", + "file3" + ) + } + + // Indicates the Proto data is improperly formatted or corrupted. + assertThat(exception).hasMessageThat().contains( + "Last unit does not have enough valid bits" + ) + } + + @Test + fun testUtility_validBase64_oneFile_writesCacheNameFile() { + val cacheNameFilePath = tempFolder.getNewTempFilePath("cache_name") + val changedFilePath = tempFolder.getNewTempFilePath("changed_file_list") + val testTargetFilePath = tempFolder.getNewTempFilePath("test_target_list") + val base64String = computeBase64String( + ChangedFilesBucket.newBuilder().apply { + cacheBucketName = "example" + addChangedFiles("//example/to/a/file/Demonstration.kt") + }.build() + ) + + runScript( + tempFolder.root.absolutePath, + base64String, + cacheNameFilePath, + changedFilePath, + testTargetFilePath + ) + + assertThat(File(cacheNameFilePath).readText().trim()).isEqualTo("example") + } + + @Test + fun testUtility_validBase64_oneFile_writesChangedFilePathWithCorrectFile() { + val cacheNameFilePath = tempFolder.getNewTempFilePath("cache_name") + val changedFilePath = tempFolder.getNewTempFilePath("changed_file_list") + val testTargetFilePath = tempFolder.getNewTempFilePath("test_target_list") + val base64String = computeBase64String( + ChangedFilesBucket.newBuilder().apply { + cacheBucketName = "example" + addChangedFiles("//example/to/a/file/Demonstration.kt") + }.build() + ) + + runScript( + tempFolder.root.absolutePath, + base64String, + cacheNameFilePath, + changedFilePath, + testTargetFilePath + ) + + assertThat(File(changedFilePath).readText().trim()).isEqualTo( + "//example/to/a/file/Demonstration.kt" + ) + } + + @Test + fun testUtility_validBase64_multipleFiles_writesChangedFilePathWithCorrectFile() { + val cacheNameFilePath = tempFolder.getNewTempFilePath("cache_name") + val changedFilePath = tempFolder.getNewTempFilePath("changed_file_list") + val testTargetFilePath = tempFolder.getNewTempFilePath("test_target_list") + val base64String = computeBase64String( + ChangedFilesBucket.newBuilder().apply { + cacheBucketName = "example" + addChangedFiles("//example/to/a/file/FirstDemonstration.kt") + addChangedFiles("//example/to/a/file/SecondDemonstration.kt") + }.build() + ) + + runScript( + tempFolder.root.absolutePath, + base64String, + cacheNameFilePath, + changedFilePath, + testTargetFilePath + ) + + assertThat(File(changedFilePath).readText().trim()).isEqualTo( + "//example/to/a/file/FirstDemonstration.kt //example/to/a/file/SecondDemonstration.kt" + ) + } + + @Test + fun testUtility_validBase64_oneFile_writesCorrectTestTargetForFile() { + testBazelWorkspace.initEmptyWorkspace() + testBazelWorkspace.addSourceAndTestFileWithContent( + filename = "Source", + testFilename = "SourceTest", + sourceContent = "class Source()", + testContent = "class SourceTest()", + sourceSubpackage = "coverage/main/java/com/example", + testSubpackage = "coverage/test/java/com/example" + ) + + val cacheNameFilePath = tempFolder.getNewTempFilePath("cache_name") + val changedFilePath = tempFolder.getNewTempFilePath("changed_file_list") + val testTargetFilePath = tempFolder.getNewTempFilePath("test_target_list") + val base64String = computeBase64String( + ChangedFilesBucket.newBuilder().apply { + cacheBucketName = "example" + addChangedFiles("coverage/main/java/com/example/Source.kt") + }.build() + ) + + runScript( + tempFolder.root.absolutePath, + base64String, + cacheNameFilePath, + changedFilePath, + testTargetFilePath + ) + + assertThat(File(testTargetFilePath).readText().trim()).isEqualTo( + "//coverage/test/java/com/example:SourceTest" + ) + } + + @Test + fun testUtility_validBase64_multipleFiles_writesCorrectTestTargetsForFiles() { + testBazelWorkspace.initEmptyWorkspace() + testBazelWorkspace.addSourceAndTestFileWithContent( + filename = "Source1", + testFilename = "Source1Test", + sourceContent = "class Source1()", + testContent = "class Source1Test()", + sourceSubpackage = "coverage/main/java/com/example", + testSubpackage = "coverage/test/java/com/example" + ) + testBazelWorkspace.addSourceAndTestFileWithContent( + filename = "Source2", + testFilename = "Source2Test", + sourceContent = "class Source2()", + testContent = "class Source2Test()", + sourceSubpackage = "coverage/main/java/com/example", + testSubpackage = "coverage/test/java/com/example" + ) + + val cacheNameFilePath = tempFolder.getNewTempFilePath("cache_name") + val changedFilePath = tempFolder.getNewTempFilePath("changed_file_list") + val testTargetFilePath = tempFolder.getNewTempFilePath("test_target_list") + val base64String = computeBase64String( + ChangedFilesBucket.newBuilder().apply { + cacheBucketName = "example" + addChangedFiles("coverage/main/java/com/example/Source1.kt") + addChangedFiles("coverage/main/java/com/example/Source2.kt") + }.build() + ) + + runScript( + tempFolder.root.absolutePath, + base64String, + cacheNameFilePath, + changedFilePath, + testTargetFilePath + ) + + assertThat(File(testTargetFilePath).readText().trim()).isEqualTo( + "//coverage/test/java/com/example:Source1Test //coverage/test/java/com/example:Source2Test" + ) + } + + private fun runScript(vararg args: String) { + main(args.toList().toTypedArray()) + } + + private fun computeBase64String(changedFilesBucket: ChangedFilesBucket): String = + changedFilesBucket.toCompressedBase64() + + /** + * Returns the absolute file path of a new file that can be written under this [TemporaryFolder] + * (but does not create the file). + */ + private fun TemporaryFolder.getNewTempFilePath(name: String) = + File(tempFolder.root, name).absolutePath + + private fun initializeCommandExecutorWithLongProcessWaitTime(): CommandExecutorImpl { + return CommandExecutorImpl( + scriptBgDispatcher, processTimeout = 5, processTimeoutUnit = TimeUnit.MINUTES + ) + } +}