Skip to content

Commit

Permalink
Fix part of #5343: Code Coverage script edge cases (#5453)
Browse files Browse the repository at this point in the history
<!-- 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 

This is an extended part of Milestone 1 to address any edge cases.

### Project
[Project 4.1]

### Changes Made

- Added arg names to the run_coverage scripts to cover edge cases on
handling formats and reordering the arguments.

Updated script run command:
```
bazel run //scripts:run_coverage -- $(pwd) utility/src/main/java/org/oppia/android/util/parser/math/MathModel.kt format=HTML
```

---

## 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
  • Loading branch information
Rd4dev authored Jul 12, 2024
1 parent 1c4b17a commit b33ca1c
Show file tree
Hide file tree
Showing 7 changed files with 368 additions and 1,546 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,13 @@ class CoverageRunner(
val sfStartIdx = coverageData.indexOfFirst {
it.startsWith("SF:") && it.substringAfter("SF:").substringAfterLast("/") == extractedFileName
}
if (sfStartIdx == -1) throw IllegalArgumentException("File not found")
require(sfStartIdx != -1) {
"Coverage data not found for the file: $extractedFileName"
}
val eofIdx = coverageData.subList(sfStartIdx, coverageData.size).indexOfFirst {
it.startsWith("end_of_record")
}
if (eofIdx == -1) throw IllegalArgumentException("End of record not found")

require(eofIdx != -1) { "End of record not found" }
val fileSpecificCovDatLines = coverageData.subList(sfStartIdx, sfStartIdx + eofIdx + 1)

val coverageDataProps = fileSpecificCovDatLines.groupBy { line ->
Expand All @@ -77,7 +78,7 @@ class CoverageRunner(
}

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

val linesFound = coverageDataProps["LF"]?.singleOrNull()?.single()?.toInt() ?: 0
val linesHit = coverageDataProps["LH"]?.singleOrNull()?.single()?.toInt() ?: 0
Expand Down
21 changes: 12 additions & 9 deletions scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import java.util.concurrent.TimeUnit
*
* Example:
* bazel run //scripts:run_coverage -- $(pwd)
* utility/src/main/java/org/oppia/android/util/parser/math/MathModel.kt HTML
* utility/src/main/java/org/oppia/android/util/parser/math/MathModel.kt format=HTML
* Example with custom process timeout:
* bazel run //scripts:run_coverage -- $(pwd)
* utility/src/main/java/org/oppia/android/util/parser/math/MathModel.kt processTimeout=10
Expand All @@ -33,10 +33,14 @@ import java.util.concurrent.TimeUnit
fun main(vararg args: String) {
val repoRoot = args[0]
val filePath = args[1]
val format = args.getOrNull(2)
val reportFormat = when {
format.equals("HTML", ignoreCase = true) -> ReportFormat.HTML
format.equals("MARKDOWN", ignoreCase = true) || format == null -> ReportFormat.MARKDOWN

val format = args.find { it.startsWith("format=", ignoreCase = true) }
?.substringAfter("=")
?.uppercase() ?: "MARKDOWN"

val reportFormat = when (format) {
"HTML" -> ReportFormat.HTML
"MARKDOWN" -> ReportFormat.MARKDOWN
else -> throw IllegalArgumentException("Unsupported report format: $format")
}

Expand Down Expand Up @@ -98,14 +102,15 @@ 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(): String {
fun execute() {
val testFileExemptionList = loadTestFileExemptionsProto(testFileExemptionTextProto)
.testFileExemptionList
.filter { it.testFileNotRequired }
.map { it.exemptedFilePath }

if (filePath in testFileExemptionList) {
return "This file is exempted from having a test file; skipping coverage check."
println("This file is exempted from having a test file; skipping coverage check.")
return
}

val testFilePaths = findTestFile(repoRoot, filePath)
Expand Down Expand Up @@ -133,8 +138,6 @@ class RunCoverage(
println("\nGenerated report at: $reportOutputPath\n")
}
} ?: println("No coverage reports generated.")

return reportOutputPath
}

private fun runCoverageForTarget(testTarget: String): CoverageReport {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,14 @@ class TestBazelWorkspace(private val temporaryRootFolder: TemporaryFolder) {
temporaryRootFolder.newFile(".bazelversion").also { it.writeText(BAZEL_VERSION) }
}
private val bazelRcFile by lazy {
temporaryRootFolder.newFile(".bazelrc").also { it.writeText("--noenable_bzlmod") }
temporaryRootFolder.newFile(".bazelrc").also {
it.writeText(
"""
--noenable_bzlmod
build --java_runtime_version=remotejdk_11 --tool_java_runtime_version=remotejdk_11
""".trimIndent()
)
}
}

private val testFileMap = mutableMapOf<String, File>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ load("@io_bazel_rules_kotlin//kotlin:jvm.bzl", "kt_jvm_test")

kt_jvm_test(
name = "RunCoverageTest",
size = "large",
srcs = ["RunCoverageTest.kt"],
shard_count = 4,
deps = [
"//scripts:test_file_check_assets",
"//scripts/src/java/org/oppia/android/scripts/coverage:run_coverage_lib",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class CoverageReporterTest {
}

@Test
fun testCoverageReporter_generateHTMLReport_hasCorrectContentAndFormatting() {
fun testCoverageReporter_generateHtmlReport_hasCorrectContentAndFormatting() {
val sourceFile = tempFolder.newFile("SampleFile.kt")
sourceFile.writeText(
"""
Expand All @@ -95,7 +95,7 @@ class CoverageReporterTest {
)
val (_, reportText) = reporter.generateRichTextReport()

val expectedHTML =
val expectedHtml =
"""
<!DOCTYPE html>
<html lang="en">
Expand Down Expand Up @@ -261,6 +261,6 @@ class CoverageReporterTest {
</html>
""".trimIndent()

assertThat(reportText).isEqualTo(expectedHTML)
assertThat(reportText).isEqualTo(expectedHtml)
}
}
Loading

0 comments on commit b33ca1c

Please sign in to comment.