diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 34884d696a2..ecc0ffa5804 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -76,10 +76,12 @@ jobs: id: compute-test-matrix env: compute_all_targets: ${{ contains(github.event.pull_request.title, '[RunAllTests]') }} + # See: https://docs.github.com/en/webhooks-and-events/webhooks/webhook-events-and-payloads#pull_request. + base_commit_hash: ${{ github.event.pull_request.base.sha }} # 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_affected_tests -- $(pwd) $(pwd)/affected_targets.log origin/develop compute_all_tests=$compute_all_targets + bazel run //scripts:compute_affected_tests -- $(pwd) $(pwd)/affected_targets.log $base_commit_hash compute_all_tests=$compute_all_targets TEST_BUCKET_LIST=$(cat ./affected_targets.log | sed 's/^\|$/"/g' | paste -sd, -) echo "Affected tests (note that this might be all tests if configured to run all or on the develop branch): $TEST_BUCKET_LIST" echo "::set-output name=matrix::{\"affected-tests-bucket-base64-encoded-shard\":[$TEST_BUCKET_LIST]}" diff --git a/scripts/assets/todo_open_exemptions.textproto b/scripts/assets/todo_open_exemptions.textproto index f7dbbc0cdb9..715fdf0bf32 100644 --- a/scripts/assets/todo_open_exemptions.textproto +++ b/scripts/assets/todo_open_exemptions.textproto @@ -232,24 +232,24 @@ todo_open_exemption { } todo_open_exemption { exempted_file_path: "scripts/src/javatests/org/oppia/android/scripts/todo/TodoIssueResolvedCheckTest.kt" + line_number: 69 line_number: 71 - line_number: 73 - line_number: 79 + line_number: 77 + line_number: 93 line_number: 95 - line_number: 97 - line_number: 103 - line_number: 131 - line_number: 133 - line_number: 139 + line_number: 101 + line_number: 128 + line_number: 130 + line_number: 136 + line_number: 140 + line_number: 142 line_number: 143 - line_number: 145 - line_number: 146 - line_number: 177 - line_number: 179 - line_number: 185 - line_number: 189 - line_number: 191 - line_number: 192 + line_number: 172 + line_number: 174 + line_number: 180 + line_number: 184 + line_number: 186 + line_number: 187 } todo_open_exemption { exempted_file_path: "scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt" @@ -281,58 +281,58 @@ todo_open_exemption { line_number: 287 line_number: 288 line_number: 292 - line_number: 336 line_number: 337 line_number: 338 - line_number: 342 - line_number: 411 + line_number: 339 + line_number: 343 line_number: 412 - line_number: 418 - line_number: 420 - line_number: 438 + line_number: 413 + line_number: 419 + line_number: 421 line_number: 439 line_number: 440 line_number: 441 line_number: 442 - line_number: 457 + line_number: 443 line_number: 458 - line_number: 461 - line_number: 476 + line_number: 459 + line_number: 462 line_number: 477 line_number: 478 line_number: 479 line_number: 480 line_number: 481 line_number: 482 - line_number: 485 + line_number: 483 line_number: 486 line_number: 487 - line_number: 505 - line_number: 509 - line_number: 569 + line_number: 488 + line_number: 506 + line_number: 510 line_number: 570 - line_number: 576 - line_number: 578 - line_number: 599 + line_number: 571 + line_number: 577 + line_number: 579 line_number: 600 line_number: 601 line_number: 602 line_number: 603 - line_number: 640 + line_number: 604 line_number: 641 - line_number: 644 - line_number: 677 + line_number: 642 + line_number: 645 line_number: 678 line_number: 679 line_number: 680 line_number: 681 line_number: 682 line_number: 683 - line_number: 686 + line_number: 684 line_number: 687 line_number: 688 - line_number: 736 - line_number: 740 + line_number: 689 + line_number: 737 + line_number: 741 } todo_open_exemption { exempted_file_path: "scripts/static_checks.sh" diff --git a/scripts/src/java/org/oppia/android/scripts/ci/ComputeAffectedTests.kt b/scripts/src/java/org/oppia/android/scripts/ci/ComputeAffectedTests.kt index ec82cb18e8b..915b506ce0f 100644 --- a/scripts/src/java/org/oppia/android/scripts/ci/ComputeAffectedTests.kt +++ b/scripts/src/java/org/oppia/android/scripts/ci/ComputeAffectedTests.kt @@ -9,6 +9,7 @@ import org.oppia.android.scripts.common.ScriptBackgroundCoroutineDispatcher import org.oppia.android.scripts.proto.AffectedTestsBucket import java.io.File import java.util.Locale +import java.util.concurrent.TimeUnit import kotlin.system.exitProcess private const val COMPUTE_ALL_TESTS_PREFIX = "compute_all_tests=" @@ -28,19 +29,20 @@ private const val MAX_TEST_COUNT_PER_SMALL_SHARD = 15 * Arguments: * - path_to_directory_root: directory path to the root of the Oppia Android repository. * - path_to_output_file: path to the file in which the affected test targets will be printed. - * - base_develop_branch_reference: the reference to the local develop branch that should be use. - * Generally, this is 'origin/develop'. + * - merge_base_commit: the base commit against which the local changes will be compared when + * determining which tests to run. When running outside of CI you can use the result of running: + * 'git merge-base develop HEAD' * - compute_all_tests: whether to compute a list of all tests to run. * * Example: * bazel run //scripts:compute_affected_tests -- $(pwd) /tmp/affected_test_buckets.proto64 \\ - * origin/develop compute_all_tests=false + * abcdef0123456789 compute_all_tests=false */ fun main(args: Array) { if (args.size < 4) { println( "Usage: bazel run //scripts:compute_affected_tests --" + - " " + + " " + " " ) exitProcess(1) @@ -48,7 +50,7 @@ fun main(args: Array) { val pathToRoot = args[0] val pathToOutputFile = args[1] - val baseDevelopBranchReference = args[2] + val baseCommit = args[2] val computeAllTestsSetting = args[3].let { check(it.startsWith(COMPUTE_ALL_TESTS_PREFIX)) { "Expected last argument to start with '$COMPUTE_ALL_TESTS_PREFIX'" @@ -62,7 +64,7 @@ fun main(args: Array) { } ScriptBackgroundCoroutineDispatcher().use { scriptBgDispatcher -> ComputeAffectedTests(scriptBgDispatcher) - .compute(pathToRoot, pathToOutputFile, baseDevelopBranchReference, computeAllTestsSetting) + .compute(pathToRoot, pathToOutputFile, baseCommit, computeAllTestsSetting) } } @@ -81,7 +83,10 @@ class ComputeAffectedTests( val maxTestCountPerLargeShard: Int = MAX_TEST_COUNT_PER_LARGE_SHARD, val maxTestCountPerMediumShard: Int = MAX_TEST_COUNT_PER_MEDIUM_SHARD, val maxTestCountPerSmallShard: Int = MAX_TEST_COUNT_PER_SMALL_SHARD, - val commandExecutor: CommandExecutor = CommandExecutorImpl(scriptBgDispatcher) + val commandExecutor: CommandExecutor = + CommandExecutorImpl( + scriptBgDispatcher, processTimeout = 5, processTimeoutUnit = TimeUnit.MINUTES + ) ) { private companion object { private const val GENERIC_TEST_BUCKET_NAME = "generic" @@ -93,28 +98,28 @@ class ComputeAffectedTests( * @param pathToRoot the absolute path to the working root directory * @param pathToOutputFile the absolute path to the file in which the encoded Base64 test bucket * protos should be printed - * @param baseDevelopBranchReference see [GitClient] + * @param baseCommit see [GitClient] * @param computeAllTestsSetting whether all tests should be outputted versus only the ones which * are affected by local changes in the repository */ fun compute( pathToRoot: String, pathToOutputFile: String, - baseDevelopBranchReference: String, + baseCommit: String, computeAllTestsSetting: Boolean ) { val rootDirectory = File(pathToRoot).absoluteFile check(rootDirectory.isDirectory) { "Expected '$pathToRoot' to be a directory" } - check(rootDirectory.list().contains("WORKSPACE")) { + check(rootDirectory.list()?.contains("WORKSPACE") == true) { "Expected script to be run from the workspace's root directory" } - println("Running from directory root: $rootDirectory") + println("Running from directory root: $rootDirectory.") - val gitClient = GitClient(rootDirectory, baseDevelopBranchReference, commandExecutor) + val gitClient = GitClient(rootDirectory, baseCommit, commandExecutor) val bazelClient = BazelClient(rootDirectory, commandExecutor) - println("Current branch: ${gitClient.currentBranch}") - println("Most recent common commit: ${gitClient.branchMergeBase}") + println("Current branch: ${gitClient.currentBranch}.") + println("Most recent common commit: ${gitClient.branchMergeBase}.") val currentBranch = gitClient.currentBranch.toLowerCase(Locale.US) val affectedTestTargets = if (computeAllTestsSetting || currentBranch == "develop") { @@ -139,7 +144,7 @@ class ComputeAffectedTests( } private fun computeAllTestTargets(bazelClient: BazelClient): List { - println("Computing all test targets") + println("Computing all test targets...") return bazelClient.retrieveAllTestTargets() } @@ -153,7 +158,7 @@ class ComputeAffectedTests( val changedFiles = gitClient.changedFiles.filter { filepath -> File(rootDirectory, filepath).exists() }.toSet() - println("Changed files (per Git): $changedFiles") + println("Changed files (per Git, ${changedFiles.size} total): $changedFiles.") // Compute the changed targets 100 files at a time to avoid unnecessarily long-running Bazel // commands. @@ -161,7 +166,7 @@ class ComputeAffectedTests( changedFiles.chunked(size = 100).fold(initial = setOf()) { allTargets, filesChunk -> allTargets + bazelClient.retrieveBazelTargets(filesChunk).toSet() } - println("Changed Bazel file targets: $changedFileTargets") + println("Changed Bazel file targets (${changedFileTargets.size} total): $changedFileTargets.") // Similarly, compute the affect test targets list 100 file targets at a time. val affectedTestTargets = @@ -169,7 +174,9 @@ class ComputeAffectedTests( .fold(initial = setOf()) { allTargets, targetChunk -> allTargets + bazelClient.retrieveRelatedTestTargets(targetChunk).toSet() } - println("Affected Bazel test targets: $affectedTestTargets") + println( + "Affected Bazel test targets (${affectedTestTargets.size} total): $affectedTestTargets." + ) // Compute the list of Bazel files that were changed. val changedBazelFiles = changedFiles.filter { file -> @@ -177,14 +184,19 @@ class ComputeAffectedTests( file.endsWith(".bazel", ignoreCase = true) || file == "WORKSPACE" } - println("Changed Bazel-specific support files: $changedBazelFiles") + println( + "Changed Bazel-specific support files (${changedBazelFiles.size} total): $changedBazelFiles." + ) // Compute the list of affected tests based on BUILD/Bazel/WORKSPACE files. These are generally // framed as: if a BUILD file changes, run all tests transitively connected to it. val transitiveTestTargets = bazelClient.retrieveTransitiveTestTargets(changedBazelFiles) - println("Affected test targets due to transitive build deps: $transitiveTestTargets") + println( + "Affected test targets due to transitive build deps (${transitiveTestTargets.size} total):" + + " $transitiveTestTargets." + ) - return (affectedTestTargets + transitiveTestTargets).toSet().toList() + return (affectedTestTargets + transitiveTestTargets).distinct() } private fun filterTargets(testTargets: List): List { @@ -321,7 +333,7 @@ class ComputeAffectedTests( private val EXTRACT_BUCKET_REGEX = "^//([^(/|:)]+?)[/:].+?\$".toRegex() /** Returns the [TestBucket] that corresponds to the specific [testTarget]. */ - fun retrieveCorrespondingTestBucket(testTarget: String): TestBucket? { + fun retrieveCorrespondingTestBucket(testTarget: String): TestBucket { return EXTRACT_BUCKET_REGEX.matchEntire(testTarget) ?.groupValues ?.maybeSecond() diff --git a/scripts/src/java/org/oppia/android/scripts/common/BazelClient.kt b/scripts/src/java/org/oppia/android/scripts/common/BazelClient.kt index 7d5806a5869..01d59a6f505 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/BazelClient.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/BazelClient.kt @@ -8,10 +8,7 @@ import java.util.Locale * Utility class to query & interact with a Bazel workspace on the local filesystem (residing within * the specified root directory). */ -class BazelClient( - private val rootDirectory: File, - private val commandExecutor: CommandExecutor -) { +class BazelClient(private val rootDirectory: File, private val commandExecutor: CommandExecutor) { /** Returns all Bazel test targets in the workspace. */ fun retrieveAllTestTargets(): List { return correctPotentiallyBrokenTargetNames( @@ -57,16 +54,19 @@ class BazelClient( // Note that this check is needed since rbuildfiles() doesn't like taking an empty list. return if (buildFileList.isNotEmpty()) { val referencingBuildFiles = - executeBazelCommand( - "query", + runPotentiallyShardedQueryCommand( + "filter('^[^@]', rbuildfiles(%s))", // Use a filter to limit the search space. + buildFiles, "--noshow_progress", "--universe_scope=//...", "--order_output=no", - "rbuildfiles($buildFileList)" + delimiter = "," ) // Compute only test & library siblings for each individual build file. While this is both // much slower than a fully combined query & can potentially miss targets, it runs - // substantially faster per query and helps to avoid potential hanging in CI. + // substantially faster per query and helps to avoid potential hanging in CI. Note also that + // this is more correct than a combined query since it ensures that siblings checks are + // properly unique for each file being considered (vs. searching for common siblings). val relevantSiblings = referencingBuildFiles.flatMap { buildFileTarget -> retrieveFilteredSiblings(filterRuleType = "test", buildFileTarget) + retrieveFilteredSiblings(filterRuleType = "android_library", buildFileTarget) @@ -77,7 +77,7 @@ class BazelClient( relevantSiblings, "--noshow_progress", "--universe_scope=//...", - "--order_output=no", + "--order_output=no" ) ) } else listOf() @@ -142,6 +142,7 @@ class BazelClient( queryFormatStr: String, values: Iterable, vararg prefixArgs: String, + delimiter: String = " ", allowPartialFailures: Boolean = false ): List { // Split up values into partitions to ensure that the argument calls don't over-run the limit. @@ -154,7 +155,7 @@ class BazelClient( // Fragment the query across the partitions to ensure all values can be considered. return partitions.flatMap { partition -> - val lastArgument = queryFormatStr.format(Locale.US, partition.joinToString(" ")) + val lastArgument = queryFormatStr.format(Locale.US, partition.joinToString(delimiter)) val allArguments = prefixArgs.toList() + lastArgument executeBazelCommand( "query", *allArguments.toTypedArray(), allowPartialFailures = allowPartialFailures diff --git a/scripts/src/java/org/oppia/android/scripts/common/CommandExecutorImpl.kt b/scripts/src/java/org/oppia/android/scripts/common/CommandExecutorImpl.kt index 8431c534496..01476cbf3cd 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/CommandExecutorImpl.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/CommandExecutorImpl.kt @@ -1,9 +1,19 @@ package org.oppia.android.scripts.common import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Deferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.TimeoutCancellationException import kotlinx.coroutines.async +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.flow.consumeAsFlow import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeout import java.io.File +import java.io.InputStream import java.util.concurrent.TimeUnit /** @@ -33,17 +43,57 @@ class CommandExecutorImpl( .directory(workingDir) .redirectErrorStream(includeErrorOutput) .start() - val finished = runBlocking { - CoroutineScope(scriptBgDispatcher).async { - process.waitFor(processTimeout, processTimeoutUnit) - }.await() + + // Consume the input & error streams individually, and separately from waiting for the process + // to complete (since consuming the output channels may be required for the process to actually + // finish executing properly). + val stdoutLinesDeferred = process.inputStream.readAllLinesAsync() + val stderrLinesDeferred = process.errorStream.readAllLinesAsync() + + val finished = process.waitFor(processTimeout, processTimeoutUnit) + val (standardOutputLines, standardErrorLines) = try { + runBlocking { + withTimeout(processTimeoutUnit.toMillis(processTimeout)) { + stdoutLinesDeferred.await() to stderrLinesDeferred.await() + } + } + } catch (e: TimeoutCancellationException) { + throw IllegalStateException("Process did not finish within the expected timeout", e) } check(finished) { "Process did not finish within the expected timeout" } return CommandResult( - process.exitValue(), - process.inputStream.bufferedReader().readLines(), - if (!includeErrorOutput) process.errorStream.bufferedReader().readLines() else listOf(), - assembledCommand, + process.exitValue(), standardOutputLines, standardErrorLines, assembledCommand ) } + + private fun InputStream.readAllLinesAsync(): Deferred> { + return CoroutineScope(scriptBgDispatcher).async { + mutableListOf().also { lines -> convertToAsyncLineFlow().collect { lines += it } } + } + } + + private fun InputStream.convertToAsyncLineFlow(): Flow { + return Channel().also { inputChannel -> + @Suppress("DeferredResultUnused") // Can be ignored since the channel result is watched. + CoroutineScope(scriptBgDispatcher).async { + this@convertToAsyncLineFlow.writeTo(inputChannel) + } + }.consumeAsFlow() + } + + private suspend fun InputStream.writeTo(channel: Channel) { + // Use I/O dispatchers for blocking I/O operations to avoid potentially running out of threads + // in the background dispatcher (that may be doing other things elsewhere in scripts). + withContext(Dispatchers.IO) { + // See https://stackoverflow.com/a/3285479 for context. Some processes require stdout/stderr + // to be consumed to progress. + try { + for (line in this@writeTo.bufferedReader().lineSequence()) { + channel.send(line) + } + } finally { + channel.close() + } + } + } } 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 dfda3889b8f..159401bb81c 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/GitClient.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/GitClient.kt @@ -4,12 +4,12 @@ import java.io.File /** * General utility for interfacing with a Git repository located at the specified working directory - * and using the specified relative branch reference that should be used when computing changes from - * the develop branch. + * and using the specified base commit hash reference that should be used when computing changes + * from the local branch. */ class GitClient( private val workingDirectory: File, - private val baseDevelopBranchReference: String, + private val baseCommit: String, private val commandExecutor: CommandExecutor ) { /** The commit hash of the HEAD of the local Git repository. */ @@ -36,7 +36,11 @@ class GitClient( } private fun retrieveBranchMergeBase(): String { - return executeGitCommandWithOneLineOutput("merge-base $baseDevelopBranchReference HEAD") + return executeGitCommandWithOneLineOutput("merge-base $baseCommit HEAD").also { + if (baseCommit != it) { + println("WARNING: Provided base commit $baseCommit doesn't match merge-base: $it.") + } + } } private fun retrieveChangedFilesWithPotentialDuplicates(): List = diff --git a/scripts/src/java/org/oppia/android/scripts/common/RepositoryFile.kt b/scripts/src/java/org/oppia/android/scripts/common/RepositoryFile.kt index 14962f3ee0b..09f437629da 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/RepositoryFile.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/RepositoryFile.kt @@ -6,10 +6,10 @@ import java.nio.file.Path import kotlin.streams.asSequence /** Helper class for managing & accessing files within the project repository. */ -class RepositoryFile() { +class RepositoryFile { companion object { /** A list of directories which should be excluded for every script check. */ - private val alwaysExcludeDirectoryList = listOf( + private val alwaysExcludeDirectoryList = listOf( ".git", ".gitsecret", ".idea", @@ -19,12 +19,12 @@ class RepositoryFile() { "bazel-oppia-android", "bazel-out", "bazel-testlogs", - "app/build/", - "data/build/", - "domain/build/", - "model/build/", - "testing/build/", - "utility/build/", + "app/build", + "data/build", + "domain/build", + "model/build", + "testing/build", + "utility/build", ) /** @@ -42,33 +42,25 @@ class RepositoryFile() { fun collectSearchFiles( repoPath: String, expectedExtension: String = "", - exemptionsList: List = listOf() + exemptionsList: List = listOf() ): List { // Note that Files.walk() is used instead of Kotlin's walk() function since the latter follows // symbolic links which is almost 10x slower than not following them (due to very deep Bazel // build directories), and it's not necessary to follow the symlinks. - return Files.walk(File(repoPath).toPath()).asSequence().map(Path::toFile).filter { file -> - val isProhibited = checkIfProhibitedFile(retrieveRelativeFilePath(file, repoPath)) - !isProhibited && + val repoRoot = File(repoPath).absoluteFile.normalize() + return Files.walk(repoRoot.toPath()).asSequence().map(Path::toFile).map { file -> + file.absoluteFile.normalize() + }.filter { file -> + !checkIfProhibitedFile(repoRoot, file) && file.isFile && file.name.endsWith(expectedExtension) && - retrieveRelativeFilePath(file, repoPath) !in exemptionsList + file.toRelativeString(repoRoot) !in exemptionsList }.toList() } - /** - * Checks if a file/directory is prohibited to be analyzed for the check. - * - * @param pathString the path of the repo - * @return whether the specified path should be analyzed per allow rules - */ - private fun checkIfProhibitedFile(pathString: String): Boolean { - return alwaysExcludeDirectoryList.any { - if (it.endsWith("/")) { - pathString.startsWith("$it") - } else { - pathString.startsWith("$it/") - } + private fun checkIfProhibitedFile(repoRoot: File, consideredFile: File): Boolean { + return alwaysExcludeDirectoryList.any { dirPath -> + consideredFile.hasBase(File(repoRoot, dirPath)) } } @@ -82,5 +74,8 @@ class RepositoryFile() { fun retrieveRelativeFilePath(file: File, repoPath: String): String { return file.toString().removePrefix(repoPath) } + + private fun File.hasBase(possibleBase: File): Boolean = + relativeToOrNull(possibleBase)?.path?.let { ".." !in it } ?: false } } diff --git a/scripts/src/javatests/org/oppia/android/scripts/build/FilterPerLanguageResourcesTest.kt b/scripts/src/javatests/org/oppia/android/scripts/build/FilterPerLanguageResourcesTest.kt index 511b07f37e9..51ca0726345 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/build/FilterPerLanguageResourcesTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/build/FilterPerLanguageResourcesTest.kt @@ -69,7 +69,7 @@ class FilterPerLanguageResourcesTest { createSupportedLanguages(ENGLISH, BRAZILIAN_PORTUGUESE, SWAHILI, NIGERIAN_PIDGIN) private val SUPPORTED_LANGUAGES_EN_AR = createSupportedLanguages(ENGLISH, ARABIC) - @field:[Rule JvmField] var tempFolder = TemporaryFolder() + @field:[Rule JvmField] val tempFolder = TemporaryFolder() private val outContent: ByteArrayOutputStream = ByteArrayOutputStream() private val originalOut: PrintStream = System.out diff --git a/scripts/src/javatests/org/oppia/android/scripts/ci/ComputeAffectedTestsTest.kt b/scripts/src/javatests/org/oppia/android/scripts/ci/ComputeAffectedTestsTest.kt index 4d8542e1e5b..e8634ba11f6 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/ci/ComputeAffectedTestsTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/ci/ComputeAffectedTestsTest.kt @@ -8,6 +8,7 @@ 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.AffectedTestsBucket @@ -139,7 +140,7 @@ class ComputeAffectedTestsTest { @Test fun testUtility_emptyDirectory_throwsException() { - val exception = assertThrows() { runScript() } + val exception = assertThrows() { runScript(currentHeadHash = "ad") } assertThat(exception).hasMessageThat().contains("run from the workspace's root directory") } @@ -529,7 +530,9 @@ class ComputeAffectedTestsTest { createBasicTests("AppTest1", "AppTest2", "AppTest3", subpackage = "app") val reportedTargets = runScriptWithShardLimits( - maxTestCountPerLargeShard = 3, maxTestCountPerMediumShard = 2, maxTestCountPerSmallShard = 1 + maxTestCountPerLargeShard = 3, + maxTestCountPerMediumShard = 2, + maxTestCountPerSmallShard = 1 ) // App module tests partition eagerly, so there should be 3 groups. Also, the code below @@ -550,7 +553,9 @@ class ComputeAffectedTestsTest { createBasicTests("DataTest1", "DataTest2", "DataTest3", subpackage = "data") val reportedTargets = runScriptWithShardLimits( - maxTestCountPerLargeShard = 3, maxTestCountPerMediumShard = 2, maxTestCountPerSmallShard = 1 + maxTestCountPerLargeShard = 3, + maxTestCountPerMediumShard = 2, + maxTestCountPerSmallShard = 1 ) // Data tests are partitioned such that they are combined into one partition. @@ -566,7 +571,9 @@ class ComputeAffectedTestsTest { createBasicTests("DomainTest1", "DomainTest2", "DomainTest3", subpackage = "domain") val reportedTargets = runScriptWithShardLimits( - maxTestCountPerLargeShard = 3, maxTestCountPerMediumShard = 2, maxTestCountPerSmallShard = 1 + maxTestCountPerLargeShard = 3, + maxTestCountPerMediumShard = 2, + maxTestCountPerSmallShard = 1 ) // Domain tests are partitioned such that they are combined into one partition. @@ -585,7 +592,9 @@ class ComputeAffectedTestsTest { ) val reportedTargets = runScriptWithShardLimits( - maxTestCountPerLargeShard = 3, maxTestCountPerMediumShard = 2, maxTestCountPerSmallShard = 1 + maxTestCountPerLargeShard = 3, + maxTestCountPerMediumShard = 2, + maxTestCountPerSmallShard = 1 ) // Instrumentation tests are partitioned such that they are combined into one partition. @@ -604,7 +613,9 @@ class ComputeAffectedTestsTest { createBasicTests("ScriptsTest1", "ScriptsTest2", "ScriptsTest3", subpackage = "scripts") val reportedTargets = runScriptWithShardLimits( - maxTestCountPerLargeShard = 3, maxTestCountPerMediumShard = 2, maxTestCountPerSmallShard = 1 + maxTestCountPerLargeShard = 3, + maxTestCountPerMediumShard = 2, + maxTestCountPerSmallShard = 1 ) // See app module test above for specifics. Scripts tests are medium partitioned which means 3 @@ -624,7 +635,9 @@ class ComputeAffectedTestsTest { createBasicTests("TestingTest1", "TestingTest2", "TestingTest3", subpackage = "testing") val reportedTargets = runScriptWithShardLimits( - maxTestCountPerLargeShard = 3, maxTestCountPerMediumShard = 2, maxTestCountPerSmallShard = 1 + maxTestCountPerLargeShard = 3, + maxTestCountPerMediumShard = 2, + maxTestCountPerSmallShard = 1 ) // Testing tests are partitioned such that they are combined into one partition. @@ -640,7 +653,9 @@ class ComputeAffectedTestsTest { createBasicTests("UtilityTest1", "UtilityTest2", "UtilityTest3", subpackage = "utility") val reportedTargets = runScriptWithShardLimits( - maxTestCountPerLargeShard = 3, maxTestCountPerMediumShard = 2, maxTestCountPerSmallShard = 1 + maxTestCountPerLargeShard = 3, + maxTestCountPerMediumShard = 2, + maxTestCountPerSmallShard = 1 ) // Utility tests are partitioned such that they are combined into one partition. @@ -698,14 +713,20 @@ class ComputeAffectedTestsTest { .containsExactly("//scripts:ScriptsTest1", "//scripts:ScriptsTest2") } - private fun runScriptWithTextOutput(computeAllTargets: Boolean = false): List { + private fun runScriptWithTextOutput( + currentHeadHash: String = computeMergeBase("develop"), + computeAllTargets: Boolean = false + ): List { val outputLog = tempFolder.newFile("output.log") - main( - arrayOf( - tempFolder.root.absolutePath, outputLog.absolutePath, "develop", - "compute_all_tests=$computeAllTargets" + val currentHeadHash = + main( + arrayOf( + tempFolder.root.absolutePath, + outputLog.absolutePath, + currentHeadHash, + "compute_all_tests=$computeAllTargets" + ) ) - ) return outputLog.readLines() } @@ -714,16 +735,21 @@ class ComputeAffectedTestsTest { * here is that which is saved directly to the output file, not debug lines printed to the * console. */ - private fun runScript(computeAllTargets: Boolean = false): List { - return parseOutputLogLines(runScriptWithTextOutput(computeAllTargets = computeAllTargets)) + private fun runScript( + currentHeadHash: String = computeMergeBase("develop"), + computeAllTargets: Boolean = false + ): List { + return parseOutputLogLines(runScriptWithTextOutput(currentHeadHash, computeAllTargets)) } private fun runScriptWithShardLimits( + baseBranch: String = "develop", maxTestCountPerLargeShard: Int, maxTestCountPerMediumShard: Int, maxTestCountPerSmallShard: 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. @@ -736,7 +762,7 @@ class ComputeAffectedTestsTest { ).compute( pathToRoot = tempFolder.root.absolutePath, pathToOutputFile = outputLog.absolutePath, - baseDevelopBranchReference = "develop", + baseCommit = currentHeadHash, computeAllTestsSetting = false ) @@ -871,6 +897,9 @@ class ComputeAffectedTestsTest { testGitRepository.commit(message = "Modified library $name") } + 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/RetrieveAffectedTestsTest.kt b/scripts/src/javatests/org/oppia/android/scripts/ci/RetrieveAffectedTestsTest.kt index e449b856a6f..c3fa8de0808 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/ci/RetrieveAffectedTestsTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/ci/RetrieveAffectedTestsTest.kt @@ -18,9 +18,7 @@ import java.io.PrintStream // FunctionName: test names are conventionally named with underscores. @Suppress("FunctionName") class RetrieveAffectedTestsTest { - @Rule - @JvmField - var tempFolder = TemporaryFolder() + @field:[Rule JvmField] val tempFolder = TemporaryFolder() private lateinit var pendingOutputStream: ByteArrayOutputStream private lateinit var originalStandardOutputStream: OutputStream diff --git a/scripts/src/javatests/org/oppia/android/scripts/common/ProtoStringEncoderTest.kt b/scripts/src/javatests/org/oppia/android/scripts/common/ProtoStringEncoderTest.kt index bed9db19a1b..fa1e4d6525a 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/common/ProtoStringEncoderTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/common/ProtoStringEncoderTest.kt @@ -16,9 +16,7 @@ import java.util.zip.ZipException // Function name: test names are conventionally named with underscores. @Suppress("FunctionName") class ProtoStringEncoderTest { - @Rule - @JvmField - var tempFolder = TemporaryFolder() + @field:[Rule JvmField] val tempFolder = TemporaryFolder() @Test fun testEncode_defaultProto_producesString() { diff --git a/scripts/src/javatests/org/oppia/android/scripts/common/RepositoryFileTest.kt b/scripts/src/javatests/org/oppia/android/scripts/common/RepositoryFileTest.kt index 99049bd675d..4aa35d33e2e 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/common/RepositoryFileTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/common/RepositoryFileTest.kt @@ -9,9 +9,7 @@ import java.io.File /** Tests for [RepositoryFile]. */ class RepositoryFileTest { - @Rule - @JvmField - var tempFolder = TemporaryFolder() + @field:[Rule JvmField] val tempFolder = TemporaryFolder() @Before fun setUp() { diff --git a/scripts/src/javatests/org/oppia/android/scripts/docs/KdocValidityCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/docs/KdocValidityCheckTest.kt index 6850c951041..9fb44afa3fd 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/docs/KdocValidityCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/docs/KdocValidityCheckTest.kt @@ -24,9 +24,7 @@ class KdocValidityCheckTest { "Refer to https://github.com/oppia/oppia-android/wiki/Static-Analysis-Checks" + "#kdoc-validity-check for more details on how to fix this." - @Rule - @JvmField - var tempFolder = TemporaryFolder() + @field:[Rule JvmField] val tempFolder = TemporaryFolder() @Before fun setUp() { diff --git a/scripts/src/javatests/org/oppia/android/scripts/label/AccessibilityLabelCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/label/AccessibilityLabelCheckTest.kt index 900140e93f7..db8d35a5f2f 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/label/AccessibilityLabelCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/label/AccessibilityLabelCheckTest.kt @@ -23,9 +23,7 @@ class AccessibilityLabelCheckTest { "Refer to https://github.com/oppia/oppia-android/wiki/Static-Analysis-Checks" + "#accessibility-label-check for more details on how to fix this." - @Rule - @JvmField - var tempFolder = TemporaryFolder() + @field:[Rule JvmField] val tempFolder = TemporaryFolder() @Before fun setUp() { diff --git a/scripts/src/javatests/org/oppia/android/scripts/license/LicenseTextsCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/license/LicenseTextsCheckTest.kt index baf034a9095..8aabf135e1e 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/license/LicenseTextsCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/license/LicenseTextsCheckTest.kt @@ -21,9 +21,7 @@ class LicenseTextsCheckTest { private val outContent: ByteArrayOutputStream = ByteArrayOutputStream() private val originalOut: PrintStream = System.out - @Rule - @JvmField - var tempFolder = TemporaryFolder() + @field:[Rule JvmField] val tempFolder = TemporaryFolder() @Before fun setUp() { diff --git a/scripts/src/javatests/org/oppia/android/scripts/maven/RetrieveLicenseTextsTest.kt b/scripts/src/javatests/org/oppia/android/scripts/maven/RetrieveLicenseTextsTest.kt index d889d6ede2b..74bbb260e66 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/maven/RetrieveLicenseTextsTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/maven/RetrieveLicenseTextsTest.kt @@ -45,9 +45,7 @@ class RetrieveLicenseTextsTest { private val outContent: ByteArrayOutputStream = ByteArrayOutputStream() private val originalOut: PrintStream = System.out - @Rule - @JvmField - var tempFolder = TemporaryFolder() + @field:[Rule JvmField] val tempFolder = TemporaryFolder() @Before fun setUp() { diff --git a/scripts/src/javatests/org/oppia/android/scripts/regex/RegexPatternValidationCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/regex/RegexPatternValidationCheckTest.kt index d3075be5f4f..29cf36ac6bc 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/regex/RegexPatternValidationCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/regex/RegexPatternValidationCheckTest.kt @@ -227,9 +227,7 @@ class RegexPatternValidationCheckTest { "Refer to https://github.com/oppia/oppia-android/wiki/Static-Analysis-Checks" + "#regexpatternvalidation-check for more details on how to fix this." - @Rule - @JvmField - var tempFolder = TemporaryFolder() + @field:[Rule JvmField] val tempFolder = TemporaryFolder() @Before fun setUp() { diff --git a/scripts/src/javatests/org/oppia/android/scripts/testfile/TestFileCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/testfile/TestFileCheckTest.kt index 1b6ded9633a..3ed1817dc82 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/testfile/TestFileCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/testfile/TestFileCheckTest.kt @@ -21,9 +21,7 @@ class TestFileCheckTest { "Refer to https://github.com/oppia/oppia-android/wiki/Static-Analysis-Checks" + "#test-file-presence-check for more details on how to fix this." - @Rule - @JvmField - var tempFolder = TemporaryFolder() + @field:[Rule JvmField] val tempFolder = TemporaryFolder() @Before fun setUp() { diff --git a/scripts/src/javatests/org/oppia/android/scripts/testing/TestBazelWorkspaceTest.kt b/scripts/src/javatests/org/oppia/android/scripts/testing/TestBazelWorkspaceTest.kt index e20b633bbc9..cab5a24d7ef 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/testing/TestBazelWorkspaceTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/testing/TestBazelWorkspaceTest.kt @@ -99,7 +99,7 @@ class TestBazelWorkspaceTest { } @Test - fun testSetupWorkspaceForRulesJvmExternal_withOneDep_containsCorrectList() { + fun testSetUpWorkspaceForRulesJvmExternal_withOneDep_containsCorrectList() { val testBazelWorkspace = TestBazelWorkspace(tempFolder) testBazelWorkspace.setUpWorkspaceForRulesJvmExternal( @@ -112,7 +112,7 @@ class TestBazelWorkspaceTest { } @Test - fun testSetupWorkspaceForRulesJvmExternal_withOneDep_setsUpBazelVersion() { + fun testSetUpWorkspaceForRulesJvmExternal_withOneDep_setsUpBazelVersion() { val testBazelWorkspace = TestBazelWorkspace(tempFolder) testBazelWorkspace.setUpWorkspaceForRulesJvmExternal( @@ -124,7 +124,7 @@ class TestBazelWorkspaceTest { } @Test - fun testSetupWorkspaceForRulesJvmExternal_withOneDep_setsUpBazelRc() { + fun testSetUpWorkspaceForRulesJvmExternal_withOneDep_setsUpBazelRc() { val testBazelWorkspace = TestBazelWorkspace(tempFolder) testBazelWorkspace.setUpWorkspaceForRulesJvmExternal( @@ -136,7 +136,7 @@ class TestBazelWorkspaceTest { } @Test - fun testSetupWorkspaceForRulesJvmExternal_withTwoDeps_containsCorrectList() { + fun testSetUpWorkspaceForRulesJvmExternal_withTwoDeps_containsCorrectList() { val testBazelWorkspace = TestBazelWorkspace(tempFolder) testBazelWorkspace.setUpWorkspaceForRulesJvmExternal( @@ -154,7 +154,7 @@ class TestBazelWorkspaceTest { } @Test - fun testSetupWorkspaceForRulesJvmExternal_withMultipleDeps_containsCorrectList() { + fun testSetUpWorkspaceForRulesJvmExternal_withMultipleDeps_containsCorrectList() { val testBazelWorkspace = TestBazelWorkspace(tempFolder) testBazelWorkspace.setUpWorkspaceForRulesJvmExternal( @@ -176,7 +176,7 @@ class TestBazelWorkspaceTest { } @Test - fun testSetupWorkspaceForRulesJvmExternal_multipleCalls_containsOnlyFirstTimeContent() { + fun testSetUpWorkspaceForRulesJvmExternal_multipleCalls_containsOnlyFirstTimeContent() { val testBazelWorkspace = TestBazelWorkspace(tempFolder) testBazelWorkspace.setUpWorkspaceForRulesJvmExternal( @@ -195,7 +195,7 @@ class TestBazelWorkspaceTest { } @Test - fun testSetupWorkspaceForRulesJvmExternal_addsMavenInstall() { + fun testSetUpWorkspaceForRulesJvmExternal_addsMavenInstall() { val testBazelWorkspace = TestBazelWorkspace(tempFolder) testBazelWorkspace.setUpWorkspaceForRulesJvmExternal( diff --git a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoIssueCommentCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoIssueCommentCheckTest.kt index 2d33e0e2a60..b0e136d7d96 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoIssueCommentCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoIssueCommentCheckTest.kt @@ -18,9 +18,7 @@ class TodoIssueCommentCheckTest { private val dummySha1 = "51ab6a0341cfb86d95a387438fc993b5eb977b83" private val dummySha2 = "74cd6a0341cfb86d95a387438fc993b5eb977b83" - @Rule - @JvmField - var tempFolder = TemporaryFolder() + @field:[Rule JvmField] val tempFolder = TemporaryFolder() @Before fun setUp() { diff --git a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoIssueResolvedCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoIssueResolvedCheckTest.kt index 3665278c101..608294bb226 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoIssueResolvedCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoIssueResolvedCheckTest.kt @@ -23,9 +23,7 @@ class TodoIssueResolvedCheckTest { "Refer to https://github.com/oppia/oppia-android/wiki/Static-Analysis-Checks" + "#todo-issue-resolved-check for more details on how to fix this." - @Rule - @JvmField - var tempFolder = TemporaryFolder() + @field:[Rule JvmField] val tempFolder = TemporaryFolder() @Before fun setUp() { @@ -57,7 +55,7 @@ class TodoIssueResolvedCheckTest { tempFile1.writeText(testContent1) tempFile2.writeText(testContent2) - main(retrieveTestFilesDirectoryPath(), "1200", "abmzuyt") + runScript(1200, "abmzuyt") assertThat(outContent.toString().trim()).isEqualTo(CLOSED_ISSUE_CHECK_PASSED_OUTPUT_INDICATOR) } @@ -81,7 +79,7 @@ class TodoIssueResolvedCheckTest { tempFile1.writeText(testContent1) tempFile2.writeText(testContent2) - main(retrieveTestFilesDirectoryPath(), "1200", "abmzuyt") + runScript(1200, "abmzuyt") assertThat(outContent.toString().trim()).isEqualTo(CLOSED_ISSUE_CHECK_PASSED_OUTPUT_INDICATOR) } @@ -105,9 +103,7 @@ class TodoIssueResolvedCheckTest { tempFile1.writeText(testContent1) tempFile2.writeText(testContent2) - val exception = assertThrows() { - main(retrieveTestFilesDirectoryPath(), "169877", "abmzuyt") - } + val exception = assertThrows() { runScript(169877, "abmzuyt") } assertThat(exception).hasMessageThat().contains(CLOSED_ISSUE_CHECK_FAILED_OUTPUT_INDICATOR) val failureMessage = @@ -123,9 +119,10 @@ class TodoIssueResolvedCheckTest { @Test fun testClosedIssueCheck_todosCorrespondsToClosedIssue_logsShouldBeLexicographicallySorted() { + tempFolder.newFolder("testfiles/extra_dir") val tempFile3 = tempFolder.newFile("testfiles/TempFile3.xml") val tempFile2 = tempFolder.newFile("testfiles/TempFile2.bazel") - val tempFile1 = tempFolder.newFile("testfiles/TempFile1.kt") + val tempFile1 = tempFolder.newFile("testfiles/extra_dir/TempFile1.kt") val testContent1 = """ // TODO(#169877): test description 1 @@ -149,18 +146,16 @@ class TodoIssueResolvedCheckTest { tempFile2.writeText(testContent2) tempFile3.writeText(testContent3) - val exception = assertThrows() { - main(retrieveTestFilesDirectoryPath(), "169877", "abmzuyt") - } + val exception = assertThrows() { runScript(169877, "abmzuyt") } assertThat(exception).hasMessageThat().contains(CLOSED_ISSUE_CHECK_FAILED_OUTPUT_INDICATOR) val failureMessage = """ The following TODOs are unresolved for the closed issue: - - TempFile1.kt:1 - TempFile2.bazel:3 - TempFile3.xml:1 - TempFile3.xml:4 + - extra_dir/TempFile1.kt:1 $wikiReferenceNote """.trimIndent() @@ -195,11 +190,8 @@ class TodoIssueResolvedCheckTest { tempFile2.writeText(testContent2) tempFile3.writeText(testContent3) - val exception = assertThrows() { - main(retrieveTestFilesDirectoryPath(), "169877", "abmzuyt") - } - val fileContentList = - File("${retrieveTestFilesDirectoryPath()}/script_failures.txt").readLines() + assertThrows() { runScript(169877, "abmzuyt") } + val fileContentList = File("${tempFolder.root}/testfiles/script_failures.txt").readLines() assertThat(fileContentList).containsExactly( "The issue is reopened because of the following unresolved TODOs:", "https://github.com/oppia/oppia-android/blob/abmzuyt/TempFile1.kt#L1", @@ -209,8 +201,7 @@ class TodoIssueResolvedCheckTest { ).inOrder() } - /** Retrieves the absolute path of testfiles directory. */ - private fun retrieveTestFilesDirectoryPath(): String { - return "${tempFolder.root}/testfiles" + private fun runScript(closedIssueNumber: Int, latestCommitHash: String) { + main("${tempFolder.root}/testfiles", closedIssueNumber.toString(), latestCommitHash) } } diff --git a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt index f23c2a01bb4..b614bc361d7 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt @@ -329,7 +329,8 @@ class TodoOpenCheckTest { @Test fun testTodoCheck_combineMultipleFailures_checkShouldFailWithAllErrorsLogged() { setUpGitHubService(issueNumbers = listOf(1000000, 152440222, 152440223, 11001)) - val tempFile1 = tempFolder.newFile("testfiles/TempFile1.kt") + tempFolder.newFolder("testfiles/extra_dir") + val tempFile1 = tempFolder.newFile("testfiles/extra_dir/TempFile1.kt") val tempFile2 = tempFolder.newFile("testfiles/TempFile2.kt") val testContent1 = """ @@ -348,7 +349,7 @@ class TodoOpenCheckTest { this.addAllTodoOpenExemption( listOf( TodoOpenExemption.newBuilder().apply { - this.exemptedFilePath = "TempFile1.kt" + this.exemptedFilePath = "extra_dir/TempFile1.kt" this.addAllLineNumber(listOf(1, 2)).build() }.build() ) @@ -362,14 +363,14 @@ class TodoOpenCheckTest { val failureMessage = """ Redundant exemptions (there are no TODOs corresponding to these lines): - - TempFile1.kt:2 + - extra_dir/TempFile1.kt:2 Please remove them from scripts/assets/todo_exemptions.textproto TODOs not in correct format: - TempFile2.kt:1 TODOs not corresponding to open issues on GitHub: - - TempFile1.kt:3 + - extra_dir/TempFile1.kt:3 $wikiReferenceNote diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/StringLanguageTranslationCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/xml/StringLanguageTranslationCheckTest.kt index bbe2c2ca818..f0f0de57f0f 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/xml/StringLanguageTranslationCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/StringLanguageTranslationCheckTest.kt @@ -42,7 +42,7 @@ class StringLanguageTranslationCheckTest { mapOf("nigerian_pidgin_only_string" to "Abeg select all di correct choices.") } - @field:[Rule JvmField] var tempFolder = TemporaryFolder() + @field:[Rule JvmField] val tempFolder = TemporaryFolder() private val originalOut: PrintStream = System.out private val documentBuilderFactory by lazy { DocumentBuilderFactory.newInstance() } diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/StringResourceValidationCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/xml/StringResourceValidationCheckTest.kt index 424a0adb8c3..7896336d6bf 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/xml/StringResourceValidationCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/StringResourceValidationCheckTest.kt @@ -41,7 +41,7 @@ class StringResourceValidationCheckTest { private const val PCM_STRING_TWO_NEWLINES = "\\nPause di audio\\n" } - @field:[Rule JvmField] var tempFolder = TemporaryFolder() + @field:[Rule JvmField] val tempFolder = TemporaryFolder() private val originalOut: PrintStream = System.out private val documentBuilderFactory by lazy { DocumentBuilderFactory.newInstance() } diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/XmlSyntaxCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/xml/XmlSyntaxCheckTest.kt index bc78d44296e..434c9097eef 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/xml/XmlSyntaxCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/XmlSyntaxCheckTest.kt @@ -24,9 +24,7 @@ class XmlSyntaxCheckTest { "Refer to https://github.com/oppia/oppia-android/wiki/Static-Analysis-Checks" + "#xml-syntax-check for more details on how to fix this." - @Rule - @JvmField - var tempFolder = TemporaryFolder() + @field:[Rule JvmField] val tempFolder = TemporaryFolder() @Before fun setUp() { diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/XmlSyntaxErrorHandlerTest.kt b/scripts/src/javatests/org/oppia/android/scripts/xml/XmlSyntaxErrorHandlerTest.kt index ec3e896517d..708db0f4a74 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/xml/XmlSyntaxErrorHandlerTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/XmlSyntaxErrorHandlerTest.kt @@ -14,9 +14,7 @@ import javax.xml.parsers.DocumentBuilderFactory class XmlSyntaxErrorHandlerTest { private val builderFactory = DocumentBuilderFactory.newInstance() - @Rule - @JvmField - var tempFolder = TemporaryFolder() + @field:[Rule JvmField] val tempFolder = TemporaryFolder() @Before fun setUp() {