Skip to content

Commit

Permalink
Fix part of #5343: Building proto with coverage results for data proc…
Browse files Browse the repository at this point in the history
…essing (#5439)

<!-- 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 #5343 

### Project
[PR 1.4 of Project 4.1]

### Changes Made
- Introduced `coverage.proto` to store the coverage data generated
during coverage analysis.
- Parsed coverage data according to the LCOV (geninfo) file format
specifications: [LCOV geninfo
specifications](https://manpages.debian.org/stretch/lcov/geninfo.1.en.html#FILES)
- This maps the Bazel test target to the corresponding SF: source file
path and filters the data to generate the Coverage Report proto that
will only the coverage of the tests related to the single source file.
- With the previous implementation in PR 1.3 to get the related
LocalTest and Test file mappings, we utilize that to run coverage for
both the test and return them as a list of Coverage Report protos.

**To Note:**
- Only the app module has the multi test file case.
- But as of date, the many app module coverage executions fail.
- So to test acquiring multi test coverage execution as list of Coverage
Report Proto, I hardcoded and tested the implementation [[hardcoded
comment](https://github.com/oppia/oppia-android/pull/5439/files#diff-62c81482e7c4b720da2d4efc73a48d10369eee097e0bf3439def6648ff150c23R89-R93)]
(just for testing the actual code wouldn't have that)

**Result:**
A representation of the list of Coverage Report proto is presented
below:

This is the output on running 2 hardcoded test cases:
`math:MathModelTest` and `math:FloatExtensionsTest`
<details>
<summary>List of Coverage Report</summary>
<br>

<pre>
Parsed Coverage Data File:
/home/rddev/.../bazel-out/.../util/parser/math/MathModelTest/coverage.dat
Parsed Coverage Data File:
/home/rddev/.../bazel-out/.../util/math/FloatExtensionsTest/coverage.dat
Coverage Data List: [bazel_test_target:
"//utility/src/test/java/org/oppia/android/util/parser/math:MathModelTest"
covered_file {
file_path:
"utility/src/main/java/org/oppia/android/util/parser/math/MathModel.kt"
  file_sha1_hash: "37eb0c6a3a16c300da100e245e7d320c61cc008f"
  covered_line {
    line_number: 14
    coverage: FULL
  }
  covered_line {
    line_number: 15
    coverage: FULL
  }
  covered_line {
    line_number: 16
    coverage: FULL
  }
  covered_line {
    line_number: 17
    coverage: FULL
  }
  covered_line {
    line_number: 21
    coverage: FULL
  }
  covered_line {
    line_number: 33
    coverage: FULL
  }
  covered_line {
    line_number: 34
    coverage: FULL
  }
  covered_line {
    line_number: 35
    coverage: FULL
  }
  covered_line {
    line_number: 36
    coverage: FULL
  }
  covered_line {
    line_number: 41
    coverage: FULL
  }
  covered_line {
    line_number: 42
    coverage: FULL
  }
  covered_line {
    line_number: 43
    coverage: FULL
  }
  covered_line {
    line_number: 44
    coverage: FULL
  }
  covered_line {
    line_number: 45
    coverage: FULL
  }
  covered_line {
    line_number: 46
    coverage: FULL
  }
  covered_line {
    line_number: 47
    coverage: FULL
  }
  covered_line {
    line_number: 49
    coverage: FULL
  }
  covered_line {
    line_number: 58
    coverage: FULL
  }
  covered_line {
    line_number: 59
    coverage: FULL
  }
  lines_found: 19
  lines_hit: 19
  function_coverage {
    line_number: 58
function_name:
"org/oppia/android/util/parser/math/MathModel$MathModelSignature$Companion::createSignature$utility_src_main_java_org_oppia_android_util_parser_math_math_latex_model_kt
(Ljava/lang/String;FZ)Lorg/oppia/android/util/parser/math/MathModel$MathModelSignature;"
    execution_count: 1
    coverage: FULL
  }
  function_coverage {
    line_number: 33
function_name:
"org/oppia/android/util/parser/math/MathModel$MathModelSignature::<init>
(Ljava/lang/String;IZ)V"
    execution_count: 1
    coverage: FULL
  }
  function_coverage {
    line_number: 35
function_name:
"org/oppia/android/util/parser/math/MathModel$MathModelSignature::getLineHeightHundredX
()I"
    execution_count: 1
    coverage: FULL
  }
  function_coverage {
    line_number: 34
function_name:
"org/oppia/android/util/parser/math/MathModel$MathModelSignature::getRawLatex
()Ljava/lang/String;"
    execution_count: 0
    coverage: NONE
  }
  function_coverage {
    line_number: 36
function_name:
"org/oppia/android/util/parser/math/MathModel$MathModelSignature::getUseInlineRendering
()Z"
    execution_count: 1
    coverage: FULL
  }
  function_coverage {
    line_number: 41
function_name:
"org/oppia/android/util/parser/math/MathModel$MathModelSignature::updateDiskCacheKey
(Ljava/security/MessageDigest;)V"
    execution_count: 1
    coverage: FULL
  }
  function_coverage {
    line_number: 14
function_name: "org/oppia/android/util/parser/math/MathModel::<init>
(Ljava/lang/String;FZ)V"
    execution_count: 1
    coverage: FULL
  }
  function_coverage {
    line_number: 16
function_name:
"org/oppia/android/util/parser/math/MathModel::getLineHeight ()F"
    execution_count: 0
    coverage: NONE
  }
  function_coverage {
    line_number: 15
function_name:
"org/oppia/android/util/parser/math/MathModel::getRawLatex
()Ljava/lang/String;"
    execution_count: 0
    coverage: NONE
  }
  function_coverage {
    line_number: 17
function_name:
"org/oppia/android/util/parser/math/MathModel::getUseInlineRendering
()Z"
    execution_count: 0
    coverage: NONE
  }
  function_coverage {
    line_number: 21
function_name:
"org/oppia/android/util/parser/math/MathModel::toKeySignature
()Lorg/oppia/android/util/parser/math/MathModel$MathModelSignature;"
    execution_count: 1
    coverage: FULL
  }
  functions_found: 11
  functions_hit: 7
  branch_coverage {
    line_number: 46
    block_number: 0
    branch_number: 0
    hit_count: 1
    coverage: FULL
  }
  branch_coverage {
    line_number: 46
    block_number: 0
    branch_number: 1
    hit_count: 1
    coverage: FULL
  }
  branches_found: 2
  branches_hit: 2
}
, bazel_test_target:
"//utility/src/test/java/org/oppia/android/util/math:FloatExtensionsTest"
covered_file {
file_path:
"utility/src/main/java/org/oppia/android/util/math/FloatExtensions.kt"
  file_sha1_hash: "178a2d84e9182a11b41adf34abc0a92a7d365f50"
  covered_line {
    line_number: 28
    coverage: FULL
  }
  covered_line {
    line_number: 36
    coverage: FULL
  }
  covered_line {
    line_number: 43
    coverage: FULL
  }
  lines_found: 3
  lines_hit: 3
  function_coverage {
    line_number: 36
function_name:
"org/oppia/android/util/math/FloatExtensionsKt::isApproximatelyEqualTo
(DD)Z"
    execution_count: 1
    coverage: FULL
  }
  function_coverage {
    line_number: 28
function_name:
"org/oppia/android/util/math/FloatExtensionsKt::isApproximatelyEqualTo
(FF)Z"
    execution_count: 1
    coverage: FULL
  }
  function_coverage {
    line_number: 43
function_name:
"org/oppia/android/util/math/FloatExtensionsKt::toPlainString
(D)Ljava/lang/String;"
    execution_count: 1
    coverage: FULL
  }
  functions_found: 3
  functions_hit: 3
  branch_coverage {
    line_number: 28
    block_number: 0
    branch_number: 0
    hit_count: 1
    coverage: FULL
  }
  branch_coverage {
    line_number: 28
    block_number: 0
    branch_number: 1
    hit_count: 1
    coverage: FULL
  }
  branch_coverage {
    line_number: 36
    block_number: 0
    branch_number: 0
    hit_count: 1
    coverage: FULL
  }
  branch_coverage {
    line_number: 36
    block_number: 0
    branch_number: 1
    hit_count: 1
    coverage: FULL
  }
  branches_found: 4
  branches_hit: 4
}
]
</pre>
</details>

**Representation in Protobuf**
- Function Coverage: Interpreted execution counts to determine coverage
status. Any execution count greater than 0 signifies full coverage,
otherwise considered null.

- Branch Coverage: Determined coverage based on the recorded hit count
for each branch. A hit count greater than 0 denotes full coverage,
otherwise marked as null.

**Usage:**
The stored proto will be used to generate coverage reports in **md** and
**HTML** formats [to be handled in PR 1.5]

## 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".
- [ ] 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 <ben@oppia.org>
Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com>
  • Loading branch information
3 people authored Jun 27, 2024
1 parent 3d044ed commit c5de68b
Show file tree
Hide file tree
Showing 10 changed files with 527 additions and 152 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,6 @@ kt_jvm_library(
visibility = ["//scripts:oppia_script_binary_visibility"],
deps = [
"//scripts/src/java/org/oppia/android/scripts/common:bazel_client",
"//scripts/src/java/org/oppia/android/scripts/proto:coverage_java_proto",
],
)
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@ 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 org.oppia.android.scripts.proto.Coverage
import org.oppia.android.scripts.proto.CoverageReport
import org.oppia.android.scripts.proto.CoveredLine
import java.io.File
import java.nio.file.Files
import java.nio.file.Paths
import java.security.MessageDigest

/**
* Class responsible for running coverage analysis asynchronously.
Expand All @@ -30,9 +36,12 @@ class CoverageRunner(
*/
fun runWithCoverageAsync(
bazelTestTarget: String
): Deferred<List<String>?> {
): Deferred<CoverageReport> {
return CoroutineScope(scriptBgDispatcher).async {
retrieveCoverageResult(bazelTestTarget)
val coverageResult = retrieveCoverageResult(bazelTestTarget)
?: error("Failed to retrieve coverage result for $bazelTestTarget")

coverageDataFileLines(coverageResult, bazelTestTarget)
}
}

Expand All @@ -41,4 +50,67 @@ class CoverageRunner(
): List<String>? {
return bazelClient.runCoverageForTestTarget(bazelTestTarget)
}

private fun coverageDataFileLines(
coverageData: List<String>,
bazelTestTarget: String
): CoverageReport {
val extractedFileName = "${extractTargetName(bazelTestTarget)}.kt"

val sfStartIdx = coverageData.indexOfFirst {
it.startsWith("SF:") && it.substringAfter("SF:").substringAfterLast("/") == extractedFileName
}
if (sfStartIdx == -1) throw IllegalArgumentException("File not found")
val eofIdx = coverageData.subList(sfStartIdx, coverageData.size).indexOfFirst {
it.startsWith("end_of_record")
}
if (eofIdx == -1) throw IllegalArgumentException("End of record not found")

val fileSpecificCovDatLines = coverageData.subList(sfStartIdx, sfStartIdx + eofIdx + 1)

val coverageDataProps = fileSpecificCovDatLines.groupBy { line ->
line.substringBefore(":")
}.mapValues { (_, lines) ->
lines.map { line ->
line.substringAfter(":").split(",")
}
}

val filePath = coverageDataProps["SF"]?.firstOrNull()?.get(0)
?: throw IllegalArgumentException("File path not found")

val linesFound = coverageDataProps["LF"]?.singleOrNull()?.single()?.toInt() ?: 0
val linesHit = coverageDataProps["LH"]?.singleOrNull()?.single()?.toInt() ?: 0

val coveredLines = coverageDataProps["DA"]?.map { (lineNumStr, hitCountStr) ->
CoveredLine.newBuilder().apply {
this.lineNumber = lineNumStr.toInt()
this.coverage = if (hitCountStr.toInt() > 0) Coverage.FULL else Coverage.NONE
}.build()
}.orEmpty()

val file = File(repoRoot, filePath)
val fileSha1Hash = calculateSha1(file.absolutePath)

return CoverageReport.newBuilder()
.setBazelTestTarget(bazelTestTarget)
.setFilePath(filePath)
.setFileSha1Hash(fileSha1Hash)
.addAllCoveredLine(coveredLines)
.setLinesFound(linesFound)
.setLinesHit(linesHit)
.build()
}
}

private fun extractTargetName(bazelTestTarget: String): String {
val targetName = bazelTestTarget.substringAfterLast(":").trim()
return targetName.removeSuffix("LocalTest").removeSuffix("Test")
}

private fun calculateSha1(filePath: String): String {
val fileBytes = Files.readAllBytes(Paths.get(filePath))
val digest = MessageDigest.getInstance("SHA-1")
val hashBytes = digest.digest(fileBytes)
return hashBytes.joinToString("") { "%02x".format(it) }
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ 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.ScriptBackgroundCoroutineDispatcher
import org.oppia.android.scripts.proto.CoverageReport
import org.oppia.android.scripts.proto.TestFileExemptions
import java.io.File
import java.util.concurrent.TimeUnit
Expand Down Expand Up @@ -78,7 +79,7 @@ class RunCoverage(
* @return a list of lists containing coverage data for each requested test target, if
* the file is exempted from having a test file, an empty list is returned
*/
fun execute(): List<List<String>> {
fun execute(): List<CoverageReport> {
val testFileExemptionList = loadTestFileExemptionsProto(testFileExemptionTextProto)
.testFileExemptionList
.filter { it.testFileNotRequired }
Expand All @@ -97,15 +98,11 @@ class RunCoverage(
val testTargets = bazelClient.retrieveBazelTargets(testFilePaths)

return testTargets.mapNotNull { testTarget ->
val coverageData = runCoverageForTarget(testTarget)
if (coverageData == null) {
println("Coverage data for $testTarget is null")
}
coverageData
runCoverageForTarget(testTarget)
}
}

private fun runCoverageForTarget(testTarget: String): List<String>? {
private fun runCoverageForTarget(testTarget: String): CoverageReport {
return runBlocking {
CoverageRunner(rootDirectory, scriptBgDispatcher, commandExecutor)
.runWithCoverageAsync(testTarget.removeSuffix(".kt"))
Expand Down
11 changes: 11 additions & 0 deletions scripts/src/java/org/oppia/android/scripts/proto/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,17 @@ java_proto_library(
deps = [":affected_tests_proto"],
)

oppia_proto_library(
name = "coverage_proto",
srcs = ["coverage.proto"],
)

java_proto_library(
name = "coverage_java_proto",
visibility = ["//scripts:oppia_script_library_visibility"],
deps = [":coverage_proto"],
)

oppia_proto_library(
name = "filename_pattern_validation_checks_proto",
srcs = ["filename_pattern_validation_checks.proto"],
Expand Down
40 changes: 40 additions & 0 deletions scripts/src/java/org/oppia/android/scripts/proto/coverage.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
syntax = "proto3";

package proto;

option java_package = "org.oppia.android.scripts.proto";
option java_multiple_files = true;

// Coverage Report that contains the bazel coverage data retrieved from the
// Bazel coverage execution.
message CoverageReport {
// The test target for which the coverage report is generated.
string bazel_test_target = 1;
// The relative path of the covered file.
string file_path = 2;
// SHA-1 hash of the file content at the time of report (to guard against changes).
string file_sha1_hash = 3;
// The lines of code covered in the report.
repeated CoveredLine covered_line = 4;
// The total number of lines found in the covered file.
int32 lines_found = 5;
// The total number of lines hit in the covered file.
int32 lines_hit = 6;
}

// Information about a single line that was covered during the tests.
message CoveredLine {
// The line number of the covered line.
int32 line_number = 1;
// The coverage status of the covered line.
Coverage coverage = 2;
}

enum Coverage {
// Coverage status is unspecified.
UNSPECIFIED = 0;
// The line, branch, or function is fully covered, ie. executed atleast once.
FULL = 1;
// The line, branch, or function is not covered at all.
NONE = 2;
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ class TestBazelWorkspace(private val temporaryRootFolder: TemporaryFolder) {
*/
fun addSourceAndTestFileWithContent(
filename: String,
testFilename: String,
sourceContent: String,
testContent: String,
sourceSubpackage: String,
Expand All @@ -73,10 +74,9 @@ class TestBazelWorkspace(private val temporaryRootFolder: TemporaryFolder) {
sourceSubpackage
)

val testFileName = "${filename}Test"
addTestContentAndBuildFile(
filename,
testFileName,
testFilename,
testContent,
sourceSubpackage,
testSubpackage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,7 @@ class BazelClientTest {

testBazelWorkspace.addSourceAndTestFileWithContent(
filename = "TwoSum",
testFilename = "TwoSumTest",
sourceContent = sourceContent,
testContent = testContent,
sourceSubpackage = "coverage/main/java/com/example",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import org.junit.Test
import org.junit.rules.TemporaryFolder
import org.oppia.android.scripts.common.CommandExecutorImpl
import org.oppia.android.scripts.common.ScriptBackgroundCoroutineDispatcher
import org.oppia.android.scripts.proto.Coverage
import org.oppia.android.scripts.proto.CoverageReport
import org.oppia.android.scripts.proto.CoveredLine
import org.oppia.android.scripts.testing.TestBazelWorkspace
import org.oppia.android.testing.assertThrows
import java.util.concurrent.TimeUnit
Expand Down Expand Up @@ -103,6 +106,7 @@ class CoverageRunnerTest {

testBazelWorkspace.addSourceAndTestFileWithContent(
filename = "TwoSum",
testFilename = "TwoSumTest",
sourceContent = sourceContent,
testContent = testContent,
sourceSubpackage = "coverage/main/java/com/example",
Expand All @@ -114,28 +118,38 @@ class CoverageRunnerTest {
"//coverage/test/java/com/example:TwoSumTest"
).await()
}
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"
)

val expectedResult = CoverageReport.newBuilder()
.setBazelTestTarget("//coverage/test/java/com/example:TwoSumTest")
.setFilePath("coverage/main/java/com/example/TwoSum.kt")
.setFileSha1Hash("f6fb075e115775f6729615a79f0e7e34fe9735b5")
.addCoveredLine(
CoveredLine.newBuilder()
.setLineNumber(3)
.setCoverage(Coverage.NONE)
.build()
)
.addCoveredLine(
CoveredLine.newBuilder()
.setLineNumber(7)
.setCoverage(Coverage.FULL)
.build()
)
.addCoveredLine(
CoveredLine.newBuilder()
.setLineNumber(8)
.setCoverage(Coverage.FULL)
.build()
)
.addCoveredLine(
CoveredLine.newBuilder()
.setLineNumber(10)
.setCoverage(Coverage.FULL)
.build()
)
.setLinesFound(4)
.setLinesHit(3)
.build()

assertThat(result).isEqualTo(expectedResult)
}
Expand Down
Loading

0 comments on commit c5de68b

Please sign in to comment.