Skip to content

Commit

Permalink
Some follow-up post-merge fixes.
Browse files Browse the repository at this point in the history 
This doesn't address all timeouts being observed in scripts. More
analysis may be needed.
  • Loading branch information
@BenHenning
BenHenning committed Jan 17, 2024
1 parent d9b38ea commit 9f1a2ec
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 25 deletions.
19 changes: 14 additions & 5 deletions scripts/src/java/org/oppia/android/scripts/testing/TestBazelWorkspace.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ import com.google.common.truth.Truth.assertThat
import org.junit.rules.TemporaryFolder
import java.io.File

/** The version of Bazel to use in tests that set up Bazel workspaces. */
const val BAZEL_VERSION = "4.0.0"

/**
* Test utility for generating various test & library targets in the specified [TemporaryFolder].
* This is meant to be used to arrange the local test filesystem for use with a real Bazel
Expand All @@ -23,6 +26,10 @@ class TestBazelWorkspace(private val temporaryRootFolder: TemporaryFolder) {
*/
val rootBuildFile: File by lazy { temporaryRootFolder.newFile("BUILD.bazel") }

private val bazelVersionFile by lazy {
temporaryRootFolder.newFile(".bazelversion").also { it.writeText(BAZEL_VERSION) }
}

private val testFileMap = mutableMapOf<String, File>()
private val libraryFileMap = mutableMapOf<String, File>()
private val testDependencyNameMap = mutableMapOf<String, String>()
Expand All @@ -33,9 +40,10 @@ class TestBazelWorkspace(private val temporaryRootFolder: TemporaryFolder) {

/** Initializes the local Bazel workspace by introducing a new, empty WORKSPACE file. */
fun initEmptyWorkspace() {
// Sanity check, but in reality this is just initializing workspaceFile to ensure that it
// Sanity checks, but in reality this is just initializing workspaceFile to ensure that it
// exists.
assertThat(workspaceFile.exists()).isTrue()
assertThat(bazelVersionFile.exists()).isTrue()
}

/**
Expand Down Expand Up @@ -187,6 +195,7 @@ class TestBazelWorkspace(private val temporaryRootFolder: TemporaryFolder) {
/** Appends rules_jvm_external configuration to the WORKSPACE file if not done already. */
fun setUpWorkspaceForRulesJvmExternal(depsList: List<String>) {
if (!isConfiguredForRulesJvmExternal) {
initEmptyWorkspace()
workspaceFile.appendText("artifactsList = [")
for (dep in depsList) {
workspaceFile.appendText("\"$dep\",\n")
Expand All @@ -198,23 +207,23 @@ class TestBazelWorkspace(private val temporaryRootFolder: TemporaryFolder) {
RULES_JVM_EXTERNAL_TAG = "4.0"
RULES_JVM_EXTERNAL_SHA = "31701ad93dbfe544d597dbe62c9a1fdd76d81d8a9150c2bf1ecf928ecdf97169"
http_archive(
name = "rules_jvm_external",
strip_prefix = "rules_jvm_external-%s" % RULES_JVM_EXTERNAL_TAG,
sha256 = RULES_JVM_EXTERNAL_SHA,
url = "https://github.com/bazelbuild/rules_jvm_external/archive/%s.zip" % RULES_JVM_EXTERNAL_TAG,
)
load("@rules_jvm_external//:defs.bzl", "maven_install")
maven_install(
artifacts = artifactsList,
repositories = [
"https://maven.google.com",
"https://repo1.maven.org/maven2",
],
)
)
""".trimIndent() + "\n"
)

Expand Down
2 changes: 1 addition & 1 deletion scripts/src/javatests/org/oppia/android/scripts/common/BazelClientTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ class BazelClientTest {
}

@Test
fun testRetrieveMavenDepsList_binaryDependsOnArtifactNotViaThirdParty_doesNotreturnArtifact() {
fun testRetrieveMavenDepsList_binaryDependsOnArtifactNotViaThirdParty_doesNotReturnArtifact() {
testBazelWorkspace.initEmptyWorkspace()
testBazelWorkspace.setUpWorkspaceForRulesJvmExternal(
listOf("com.android.support:support-annotations:28.0.0")
Expand Down
2 changes: 1 addition & 1 deletion ...src/javatests/org/oppia/android/scripts/common/ScriptBackgroundCoroutineDispatcherTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class ScriptBackgroundCoroutineDispatcherTest {
dispatcher.close()

// The task should fail to schedule since the dispatcher has been closed.
assertThrows(CancellationException::class) {
assertThrows<CancellationException>() {
runBlocking { withContext(dispatcher) { mockRunnable.run() } }
}
}
Expand Down
28 changes: 14 additions & 14 deletions scripts/src/javatests/org/oppia/android/scripts/license/MavenDependenciesListCheckTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,14 +138,14 @@ class MavenDependenciesListCheckTest {
license_name: "The Apache License, Version 2.0"
original_link: "https://www.apache.org/licenses/LICENSE-2.0.txt"
}
artifact_name: "com.google.firebase:firebase-analytics:17.5.0"
artifact_version: "17.5.0"
license {
license_name: "Android Software Development Kit License"
original_link: "https://developer.android.com/studio/terms.html"
}
Refer to https://github.com/oppia/oppia-android/wiki/Updating-Maven-Dependencies for more details.
""".trimIndent() + "\n"
)
Expand Down Expand Up @@ -215,7 +215,7 @@ class MavenDependenciesListCheckTest {
license_name: "Android Software Development Kit License"
original_link: "https://developer.android.com/studio/terms.html"
}
Refer to https://github.com/oppia/oppia-android/wiki/Updating-Maven-Dependencies for more details.
""".trimIndent() + "\n"
)
Expand Down Expand Up @@ -280,14 +280,14 @@ class MavenDependenciesListCheckTest {
license_name: "The Apache License, Version 2.0"
original_link: "https://www.apache.org/licenses/LICENSE-2.0.txt"
}
artifact_name: "$FIREBASE_ANALYTICS_DEP"
artifact_version: "$FIREBASE_ANALYTICS_VERSION"
license {
license_name: "Android Software Development Kit License"
original_link: "https://developer.android.com/studio/terms.html"
}
Refer to https://github.com/oppia/oppia-android/wiki/Updating-Maven-Dependencies for more details.
""".trimIndent() + "\n"
)
Expand Down Expand Up @@ -437,7 +437,7 @@ class MavenDependenciesListCheckTest {
license_name: "The Apache License, Version 2.0"
original_link: "https://www.apache.org/licenses/LICENSE-2.0.txt"
}
artifact_name: "$FIREBASE_ANALYTICS_DEP"
artifact_version: "$FIREBASE_ANALYTICS_VERSION"
license {
Expand Down Expand Up @@ -516,14 +516,14 @@ class MavenDependenciesListCheckTest {
}
Missing dependencies that need to be added:
artifact_name: "$FIREBASE_ANALYTICS_DEP"
artifact_version: "$FIREBASE_ANALYTICS_VERSION"
license {
license_name: "Android Software Development Kit License"
original_link: "https://developer.android.com/studio/terms.html"
}
Refer to https://github.com/oppia/oppia-android/wiki/Updating-Maven-Dependencies for more details.
""".trimIndent() + "\n"
)
Expand Down Expand Up @@ -595,14 +595,14 @@ class MavenDependenciesListCheckTest {
}
Missing dependencies that need to be added:
artifact_name: "$FIREBASE_ANALYTICS_UPGRADED_DEP"
artifact_version: "$FIREBASE_ANALYTICS_UPGRADED_VERSION"
license {
license_name: "Android Software Development Kit License"
original_link: "https://developer.android.com/studio/terms.html"
}
Refer to https://github.com/oppia/oppia-android/wiki/Updating-Maven-Dependencies for more details.
""".trimIndent() + "\n"
)
Expand Down Expand Up @@ -661,7 +661,7 @@ class MavenDependenciesListCheckTest {
}
assertThat(exception).hasMessageThat().contains(MISSING_AND_REDUNDANT_DEPENDENCIES_FAILURE)
assertThat(outContent.toString()).isEqualTo(
"""
"""
Errors were encountered. Please run script GenerateMavenDependenciesList.kt to fix.
Redundant dependencies that need to be removed:
Expand All @@ -674,14 +674,14 @@ class MavenDependenciesListCheckTest {
}
Missing dependencies that need to be added:
artifact_name: "$FIREBASE_ANALYTICS_DEP"
artifact_version: "$FIREBASE_ANALYTICS_VERSION"
license {
license_name: "Android Software Development Kit License"
original_link: "https://developer.android.com/studio/terms.html"
}
Refer to https://github.com/oppia/oppia-android/wiki/Updating-Maven-Dependencies for more details.
""".trimIndent() + "\n"
)
Expand Down Expand Up @@ -1032,7 +1032,7 @@ class MavenDependenciesListCheckTest {
}
]
}
}
}
""".trimIndent()
)
}
Expand Down
2 changes: 1 addition & 1 deletion scripts/src/javatests/org/oppia/android/scripts/license/MavenDependenciesRetrieverTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1221,7 +1221,7 @@ class MavenDependenciesRetrieverTest {
}
]
}
}
}
""".trimIndent()
)
}
Expand Down
9 changes: 9 additions & 0 deletions scripts/src/javatests/org/oppia/android/scripts/testing/TestBazelWorkspaceTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,15 @@ class TestBazelWorkspaceTest {
assertThat(workspaceFile.readLines()).isEmpty()
}

@Test
fun testInitEmptyWorkspace_createsBazelVersionFileWithCorrectVersion() {
val testBazelWorkspace = TestBazelWorkspace(tempFolder)

testBazelWorkspace.initEmptyWorkspace()

assertThat(File(tempFolder.root, ".bazelversion").readText().trim()).isEqualTo("4.0.0")
}

@Test
fun testSetupWorkspaceForRulesJvmExternal_withOneDep_containsCorrectList() {
val testBazelWorkspace = TestBazelWorkspace(tempFolder)
Expand Down
4 changes: 1 addition & 3 deletions scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,9 +449,7 @@ class TodoOpenCheckTest {
}.build()
exemptions.writeTo(exemptionFile.outputStream())

val exception = assertThrows(Exception::class) {
runScript(regenerateFile = true)
}
val exception = assertThrows<Exception>() { runScript(regenerateFile = true) }

assertThat(exception).hasMessageThat().contains(TODO_SYNTAX_CHECK_FAILED_OUTPUT_INDICATOR)
val failureMessage =
Expand Down

0 comments on commit 9f1a2ec

Please sign in to comment.