Skip to content

Commit

Permalink
Fix part of oppia#5343: Introduce Utilities to execute code coverage …
Browse files Browse the repository at this point in the history
…for a specific bazel test target (oppia#5423)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

Fixes part of oppia#5343 

### Project
[PR 1.1 of Project 4.1]

### This PR introduces 2 utilities:
1. **RunCoverageForTestTarget** - A new script for running code coverage
locally
2. **CoverageRunner** - A new scripting utility for running code
coverage for a selected target

### Source:
- The RunCoverageForTestTarget script takes in a test target as an
argument.
- The CoverageRunner executes the bazel coverage command asynchronously
utilizing bazelClient.
- Implementation is added to BazelClient to execute code coverage
commands.
- The getCoverage() from CoverageRunner returns a list of string
containing the output result.
- The result is parsed via parseCoverageDataFile() to extract the
coverage.dat file path.

The extracted coverage.dat file will then be used to acquire the actual
coverage data.

### Tests:
- Tests are introduced for RunCoverageForTestTarget and CoverageRunner
utilities.
- To test the execution, sample test files are generated and utilized
for testing.
- Sample test files include - 
  - [sourceFile.kt, BUILD.bazel], 
  - [testFile.kt,BUILD.bazel], 
  - WORKSPACE 
  in their appropriate subpackages
- To add functionality to introduce source and test files with content
TestBazelWorkspace utility is utilized.
- The addSourceAndTestFileWithContent() in TestBazelWorkspace utility
achieves this.
- Tests are added for
  - BazelClientTest
  - TestBazelWorkspaceTest

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

---------

Co-authored-by: Ben Henning <[email protected]>
  • Loading branch information
Rd4dev and BenHenning authored Jun 24, 2024
1 parent dd80399 commit 2059f9d
Show file tree
Hide file tree
Showing 8 changed files with 650 additions and 2 deletions.
34 changes: 34 additions & 0 deletions scripts/src/java/org/oppia/android/scripts/common/BazelClient.kt
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,40 @@ class BazelClient(private val rootDirectory: File, private val commandExecutor:
return correctedTargets
}

/**
* Runs code coverage for the specified Bazel test target.
*
* Null return typically occurs when the coverage command fails to generate the 'coverage.dat' file
* This can happen due to: Test failures or misconfigurations that prevent the coverage data
* from being generated properly.
*
* @param bazelTestTarget Bazel test target for which code coverage will be run
* @return the generated coverage data as a list of strings
* or null if the coverage data file could not be parsed
*/
fun runCoverageForTestTarget(bazelTestTarget: String): List<String>? {
val coverageCommandOutputLines = executeBazelCommand(
"coverage",
bazelTestTarget
)
return parseCoverageDataFilePath(coverageCommandOutputLines)?.let { path ->
File(path).readLines()
}
}

private fun parseCoverageDataFilePath(coverageCommandOutputLines: List<String>): String? {
val regex = ".*coverage\\.dat$".toRegex()
for (line in coverageCommandOutputLines) {
val match = regex.find(line)
val extractedPath = match?.value?.substringAfterLast(",")?.trim()
if (extractedPath != null) {
println("Parsed Coverage Data File: $extractedPath")
return extractedPath
}
}
return null
}

/**
* Returns the results of a query command with a potentially large list of [values] that will be
* split up into multiple commands to avoid overflow the system's maximum argument limit.
Expand Down
17 changes: 17 additions & 0 deletions scripts/src/java/org/oppia/android/scripts/coverage/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
"""
Libraries corresponding to developer scripts that obtain coverage data for test targets.
"""

load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_library")

kt_jvm_library(
name = "coverage_runner",
testonly = True,
srcs = [
"CoverageRunner.kt",
],
visibility = ["//scripts:oppia_script_binary_visibility"],
deps = [
"//scripts/src/java/org/oppia/android/scripts/common:bazel_client",
],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package org.oppia.android.scripts.coverage

import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Deferred
import kotlinx.coroutines.async
import org.oppia.android.scripts.common.BazelClient
import org.oppia.android.scripts.common.CommandExecutor
import org.oppia.android.scripts.common.ScriptBackgroundCoroutineDispatcher
import java.io.File

/**
* Class responsible for running coverage analysis asynchronously.
*
* @param repoRoot the root directory of the repository
* @param scriptBgDispatcher the [ScriptBackgroundCoroutineDispatcher] to be used for running the coverage command
* @param commandExecutor executes the specified command in the specified working directory
*/
class CoverageRunner(
private val repoRoot: File,
private val scriptBgDispatcher: ScriptBackgroundCoroutineDispatcher,
private val commandExecutor: CommandExecutor
) {
private val bazelClient by lazy { BazelClient(repoRoot, commandExecutor) }

/**
* Runs coverage analysis asynchronously for the Bazel test target.
*
* @param bazelTestTarget Bazel test target to analyze coverage
* @return a deferred value that contains the coverage data
*/
fun runWithCoverageAsync(
bazelTestTarget: String
): Deferred<List<String>?> {
return CoroutineScope(scriptBgDispatcher).async {
retrieveCoverageResult(bazelTestTarget)
}
}

private fun retrieveCoverageResult(
bazelTestTarget: String
): List<String>? {
return bazelClient.runCoverageForTestTarget(bazelTestTarget)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,141 @@ class TestBazelWorkspace(private val temporaryRootFolder: TemporaryFolder) {
assertThat(bazelRcFile.exists()).isTrue()
}

/**
* Adds a source file and test file with the specified name and content,
* and updates the corresponding build configuration.
*
* @param filename the name of the source file (without the .kt extension)
* @param sourceContent the content of the source file
* @param testContent the content of the test file
* @param subpackage the subpackage under which the source and test files should be added
*/
fun addSourceAndTestFileWithContent(
filename: String,
sourceContent: String,
testContent: String,
subpackage: String
) {
val sourceSubpackage = "$subpackage/main/java/com/example"
addSourceContentAndBuildFile(
filename,
sourceContent,
sourceSubpackage
)

val testSubpackage = "$subpackage/test/java/com/example"
val testFileName = "${filename}Test"
addTestContentAndBuildFile(
filename,
testFileName,
testContent,
sourceSubpackage,
testSubpackage
)
}

/**
* Adds a source file with the specified name and content to the specified subpackage,
* and updates the corresponding build configuration.
*
* @param filename the name of the source file (without the .kt extension)
* @param sourceContent the content of the source file
* @param sourceSubpackage the subpackage under which the source file should be added
* @return the target name of the added source file
*/
fun addSourceContentAndBuildFile(
filename: String,
sourceContent: String,
sourceSubpackage: String
) {
initEmptyWorkspace()
ensureWorkspaceIsConfiguredForKotlin()
setUpWorkspaceForRulesJvmExternal(
listOf("junit:junit:4.12")
)

// Create the source subpackage directory if it doesn't exist
if (!File(temporaryRootFolder.root, sourceSubpackage.replace(".", "/")).exists()) {
temporaryRootFolder.newFolder(*(sourceSubpackage.split(".")).toTypedArray())
}

// Create the source file
val sourceFile = temporaryRootFolder.newFile(
"${sourceSubpackage.replace(".", "/")}/$filename.kt"
)
sourceFile.writeText(sourceContent)

// Create or update the BUILD file for the source file
val buildFileRelativePath = "${sourceSubpackage.replace(".", "/")}/BUILD.bazel"
val buildFile = File(temporaryRootFolder.root, buildFileRelativePath)
if (!buildFile.exists()) {
temporaryRootFolder.newFile(buildFileRelativePath)
}
prepareBuildFileForLibraries(buildFile)

buildFile.appendText(
"""
kt_jvm_library(
name = "${filename.lowercase()}",
srcs = ["$filename.kt"],
visibility = ["//visibility:public"]
)
""".trimIndent() + "\n"
)
}

/**
* Adds a test file with the specified name and content to the specified subpackage,
* and updates the corresponding build configuration.
*
* @param filename the name of the source file (without the .kt extension)
* @param testName the name of the test file (without the .kt extension)
* @param testContent the content of the test file
* @param testSubpackage the subpackage for the test file
*/
fun addTestContentAndBuildFile(
filename: String,
testName: String,
testContent: String,
sourceSubpackage: String,
testSubpackage: String
) {
initEmptyWorkspace()

// Create the test subpackage directory for the test file if it doesn't exist
if (!File(temporaryRootFolder.root, testSubpackage.replace(".", "/")).exists()) {
temporaryRootFolder.newFolder(*(testSubpackage.split(".")).toTypedArray())
}

// Create the test file
val testFile = temporaryRootFolder.newFile("${testSubpackage.replace(".", "/")}/$testName.kt")
testFile.writeText(testContent)

// Create or update the BUILD file for the test file
val testBuildFileRelativePath = "${testSubpackage.replace(".", "/")}/BUILD.bazel"
val testBuildFile = File(temporaryRootFolder.root, testBuildFileRelativePath)
if (!testBuildFile.exists()) {
temporaryRootFolder.newFile(testBuildFileRelativePath)
}
prepareBuildFileForTests(testBuildFile)

// Add the test file to the BUILD file with appropriate dependencies
testBuildFile.appendText(
"""
kt_jvm_test(
name = "test",
srcs = ["$testName.kt"],
deps = [
"//$sourceSubpackage:${filename.lowercase()}",
"@maven//:junit_junit",
],
visibility = ["//visibility:public"],
test_class = "com.example.$testName",
)
""".trimIndent() + "\n"
)
}

/**
* Generates and adds a new kt_jvm_test target with the target name [testName] and test file
* [testFile]. This can be used to add multiple tests to the same build file, and will
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import java.util.concurrent.TimeUnit
* Note that this test executes real commands on the local filesystem & requires Bazel in the local
* environment.
*/
// Same parameter value: helpers reduce test context, even if they are used by 1 test.
// Function name: test names are conventionally named with underscores.
// Same parameter value: helpers reduce test context, even if they are used by 1 test
// Function name: test names are conventionally named with underscores
@Suppress("SameParameterValue", "FunctionName")
class BazelClientTest {
@field:[Rule JvmField] val tempFolder = TemporaryFolder()
Expand Down Expand Up @@ -379,6 +379,95 @@ class BazelClientTest {
assertThat(thirdPartyDependenciesList).doesNotContain("@maven//:androidx_annotation_annotation")
}

@Test
fun testRunCodeCoverage_forSampleTestTarget_returnsCoverageResult() {
val bazelClient = BazelClient(tempFolder.root, longCommandExecutor)
testBazelWorkspace.initEmptyWorkspace()

val sourceContent =
"""
package com.example
class TwoSum {
companion object {
fun sumNumbers(a: Int, b: Int): Any {
return if (a == 0 && b == 0) {
"Both numbers are zero"
} else {
a + b
}
}
}
}
""".trimIndent()

val testContent =
"""
package com.example
import org.junit.Assert.assertEquals
import org.junit.Test
class TwoSumTest {
@Test
fun testSumNumbers() {
assertEquals(TwoSum.sumNumbers(0, 1), 1)
assertEquals(TwoSum.sumNumbers(3, 4), 7)
assertEquals(TwoSum.sumNumbers(0, 0), "Both numbers are zero")
}
}
""".trimIndent()

testBazelWorkspace.addSourceAndTestFileWithContent(
filename = "TwoSum",
sourceContent = sourceContent,
testContent = testContent,
subpackage = "coverage"
)

val result = bazelClient.runCoverageForTestTarget("//coverage/test/java/com/example:test")
val expectedResult = listOf(
"SF:coverage/main/java/com/example/TwoSum.kt",
"FN:7,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;",
"FN:3,com/example/TwoSum::<init> ()V",
"FNDA:1,com/example/TwoSum${'$'}Companion::sumNumbers (II)Ljava/lang/Object;",
"FNDA:0,com/example/TwoSum::<init> ()V",
"FNF:2",
"FNH:1",
"BRDA:7,0,0,1",
"BRDA:7,0,1,1",
"BRDA:7,0,2,1",
"BRDA:7,0,3,1",
"BRF:4",
"BRH:4",
"DA:3,0",
"DA:7,1",
"DA:8,1",
"DA:10,1",
"LH:3",
"LF:4",
"end_of_record"
)

assertThat(result).isEqualTo(expectedResult)
}

@Test
fun testRunCodeCoverage_forNonTestTarget_fails() {
val bazelClient = BazelClient(tempFolder.root, longCommandExecutor)
testBazelWorkspace.initEmptyWorkspace()

val exception = assertThrows<IllegalStateException>() {
bazelClient.runCoverageForTestTarget("//coverage/test/java/com/example:test")
}

// Verify that the underlying Bazel command failed since the test target was not available.
assertThat(exception).hasMessageThat().contains("Expected non-zero exit code")
assertThat(exception).hasMessageThat().contains("no such package")
}

private fun fakeCommandExecutorWithResult(singleLine: String) {
// Fake a Bazel command's results to return jumbled results. This has been observed to happen
// sometimes in CI, but doesn't have a known cause. The utility is meant to de-jumble these in
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
"""
Tests corresponding to developer scripts that help with obtaining coverage data for test targets.
"""

load("@io_bazel_rules_kotlin//kotlin:jvm.bzl", "kt_jvm_test")

kt_jvm_test(
name = "CoverageRunnerTest",
srcs = ["CoverageRunnerTest.kt"],
deps = [
"//scripts/src/java/org/oppia/android/scripts/coverage:coverage_runner",
"//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",
],
)
Loading

0 comments on commit 2059f9d

Please sign in to comment.