From 6fed0c0fe0b697bd88fc6693d1e297aa8485fcd9 Mon Sep 17 00:00:00 2001 From: Rd Date: Mon, 15 Jul 2024 22:16:53 +0530 Subject: [PATCH] Refactored runCoverageForFile generation separate function and added test cases for sets of success, failure and anomaly cases --- .../android/scripts/coverage/RunCoverage.kt | 159 +++---- .../scripts/coverage/RunCoverageTest.kt | 389 +++++++++++++++--- 2 files changed, 419 insertions(+), 129 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt b/scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt index d74624632ca..fd855671de2 100644 --- a/scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt +++ b/scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt @@ -14,6 +14,8 @@ import org.oppia.android.scripts.proto.TestFileExemptions import java.io.File import java.util.concurrent.TimeUnit +private val MIN_THRESHOLD = 10 // yet to be decided on a value + /** * Entry point function for running coverage analysis for a source file. * @@ -103,6 +105,7 @@ class RunCoverage( private val scriptBgDispatcher: ScriptBackgroundCoroutineDispatcher ) { private val bazelClient by lazy { BazelClient(File(repoRoot), commandExecutor) } + private var coverageCheckState = CoverageCheck.PASS private val rootDirectory = File(repoRoot).absoluteFile private val testFileExemptionTextProto = "scripts/assets/test_file_exemptions" @@ -112,9 +115,6 @@ class RunCoverage( .associateBy { it.exemptedFilePath } } - private val MIN_THRESHOLD = 10 // yet to be decided on a value - private var coverageCheckState = CoverageCheck.PASS - /** * Executes coverage analysis for the specified file. * @@ -135,7 +135,7 @@ class RunCoverage( if (coverageCheckState == CoverageCheck.FAIL) { error( "\nCoverage Analysis Failed as minimum coverage threshold not met!" + - "\nMinimum Coverage Threshold = $MIN_THRESHOLD%" + "\nMinimum Coverage Threshold = $MIN_THRESHOLD%" ) } else { println("\nCoverage Analysis Completed Succesffully!") @@ -176,30 +176,7 @@ class RunCoverage( } val aggregatedCoverageReport = calculateAggregateCoverageReport(coverageReports) - val reporter = CoverageReporter(repoRoot, aggregatedCoverageReport, reportFormat) - var (computedCoverageRatio, reportText) = reporter.generateRichTextReport() - - val coverageCheckThreshold = exemption?.overrideMinCoveragePercentRequired - ?: MIN_THRESHOLD - - if (computedCoverageRatio * 100 < coverageCheckThreshold) { - coverageCheckState = CoverageCheck.FAIL - } - - reportText += if (reportFormat == ReportFormat.MARKDOWN) { - computedCoverageRatio.takeIf { it * 100 < coverageCheckThreshold } - ?.let { "|:x:|" } ?: "|:white_check_mark:|" - } else "" - - val reportOutputPath = getReportOutputPath(repoRoot, filePath, reportFormat) - File(reportOutputPath).apply { - parentFile?.mkdirs() - writeText(reportText) - } - - if (File(reportOutputPath).exists()) { - println("\nGenerated report at: $reportOutputPath\n") - } + val reportText = generateAggregatedCoverageReport(aggregatedCoverageReport) return reportText } @@ -232,25 +209,25 @@ class RunCoverage( val coverageFailuresRows = coverageFailures.joinToString(separator = "\n") val coverageSuccessesRows = coverageSuccesses.joinToString(separator = "\n") - val failureMarkdownTable = if (coverageFailuresRows.isNotEmpty()) { + val failureMarkdownTable = coverageFailuresRows.takeIf { it.isNotEmpty() }?.let { "### Failed Coverages\n" + - "Min Coverage Required: $MIN_THRESHOLD%\n\n" + - coverageTableHeader + - coverageFailuresRows - } else "" + "Min Coverage Required: $MIN_THRESHOLD%\n\n" + + coverageTableHeader + + it + } ?: "" - val successMarkdownTable = if (coverageSuccessesRows.isNotEmpty()) { + val successMarkdownTable = coverageSuccessesRows.takeIf { it.isNotEmpty() }?.let { "
\n" + - "Succeeded Coverages
\n\n" + - coverageTableHeader + - coverageSuccessesRows + - "\n
" - } else "" + "Succeeded Coverages
\n\n" + + coverageTableHeader + + it + + "\n" + } ?: "" val anomalyCasesList = anomalyCases.joinToString(separator = "\n") { "- $it" } - val anomalySection = if (anomalyCases.isNotEmpty()) { + val anomalySection = anomalyCases.takeIf { it.isNotEmpty() }?.let { "\n\n### Anomaly Cases\n$anomalyCasesList" - } else "" + } ?: "" val finalReportText = "## Coverage Report\n\n" + "- No of files assessed: ${coverageResults.size}\n" + @@ -266,48 +243,80 @@ class RunCoverage( } } - /** Corresponds to status of the coverage analysis. */ - private enum class CoverageCheck { - /** Indicates successful generation of coverage retrieval for a specified file. */ - PASS, - /** Indicates failure or anomaly during coverage retrieval for a specified file. */ - FAIL - } -} + private fun generateAggregatedCoverageReport(aggregatedCoverageReport: CoverageReport): String { + val reporter = CoverageReporter(repoRoot, aggregatedCoverageReport, reportFormat) + var (computedCoverageRatio, reportText) = reporter.generateRichTextReport() -private fun calculateAggregateCoverageReport( - coverageReports: List -): CoverageReport { - fun aggregateCoverage(coverages: List): Coverage { - return if (coverages.contains(Coverage.FULL)) Coverage.FULL - else Coverage.NONE - } + val coverageCheckThreshold = testFileExemptionList[aggregatedCoverageReport.filePath] + ?.overrideMinCoveragePercentRequired + ?: MIN_THRESHOLD - val allCoveredLines = coverageReports.flatMap { it.coveredLineList } + if (computedCoverageRatio * 100 < coverageCheckThreshold) { + coverageCheckState = CoverageCheck.FAIL + } - val groupedCoveredLines = allCoveredLines.groupBy { it.lineNumber } + reportText += if (reportFormat == ReportFormat.MARKDOWN) { + computedCoverageRatio.takeIf { it * 100 < coverageCheckThreshold } + ?.let { "|:x:|" } ?: "|:white_check_mark:|" + } else "" - val aggregatedCoveredLines = groupedCoveredLines.map { (lineNumber, coveredLines) -> - CoveredLine.newBuilder() - .setLineNumber(lineNumber) - .setCoverage(aggregateCoverage(coveredLines.map { it.coverage })) - .build() + val reportOutputPath = getReportOutputPath( + repoRoot, aggregatedCoverageReport.filePath, reportFormat + ) + File(reportOutputPath).apply { + parentFile?.mkdirs() + writeText(reportText) + } + + if (File(reportOutputPath).exists()) { + println("\nGenerated report at: $reportOutputPath\n") + } + + return reportText } - val totalLinesFound = aggregatedCoveredLines.size - val totalLinesHit = aggregatedCoveredLines.count { it.coverage == Coverage.FULL } + private fun calculateAggregateCoverageReport( + coverageReports: List + ): CoverageReport { + fun aggregateCoverage(coverages: List): Coverage { + return if (coverages.contains(Coverage.FULL)) Coverage.FULL + else Coverage.NONE + } + + val allCoveredLines = coverageReports.flatMap { it.coveredLineList } + + val groupedCoveredLines = allCoveredLines.groupBy { it.lineNumber } + + val aggregatedCoveredLines = groupedCoveredLines.map { (lineNumber, coveredLines) -> + CoveredLine.newBuilder() + .setLineNumber(lineNumber) + .setCoverage(aggregateCoverage(coveredLines.map { it.coverage })) + .build() + } + + val totalLinesFound = aggregatedCoveredLines.size + val totalLinesHit = aggregatedCoveredLines.count { it.coverage == Coverage.FULL } + + val aggregatedTargetList = coverageReports.joinToString(separator = ", ") { it.bazelTestTarget } - val aggregatedTargetList = coverageReports.joinToString(separator = ", ") { it.bazelTestTarget } + return CoverageReport.newBuilder() + .setBazelTestTarget(aggregatedTargetList) + .setFilePath(coverageReports.first().filePath) + .setFileSha1Hash(coverageReports.first().fileSha1Hash) + .addAllCoveredLine(aggregatedCoveredLines) + .setLinesFound(totalLinesFound) + .setLinesHit(totalLinesHit) + .setIsGenerated(true) + .build() + } - return CoverageReport.newBuilder() - .setBazelTestTarget(aggregatedTargetList) - .setFilePath(coverageReports.first().filePath) - .setFileSha1Hash(coverageReports.first().fileSha1Hash) - .addAllCoveredLine(aggregatedCoveredLines) - .setLinesFound(totalLinesFound) - .setLinesHit(totalLinesHit) - .setIsGenerated(true) - .build() + /** Corresponds to status of the coverage analysis. */ + private enum class CoverageCheck { + /** Indicates successful generation of coverage retrieval for a specified file. */ + PASS, + /** Indicates failure or anomaly during coverage retrieval for a specified file. */ + FAIL + } } private fun findTestFile(repoRoot: String, filePath: String): List { diff --git a/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt b/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt index 91a6d9b3af6..31ae60c68ff 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt @@ -32,8 +32,6 @@ class RunCoverageTest { private lateinit var addSourceContent: String private lateinit var addTestContent: String - private lateinit var subSourceContent: String - private lateinit var subTestContent: String @Before fun setUp() { @@ -75,40 +73,6 @@ class RunCoverageTest { } } """.trimIndent() - - subSourceContent = - """ - package com.example - - class SubNums { - companion object { - fun subNumbers(a: Int, b: Int): Any { - return if (a == 0 && b == 0) { - "Both numbers are zero" - } else { - a - b - } - } - } - } - """.trimIndent() - - subTestContent = - """ - package com.example - - import org.junit.Assert.assertEquals - import org.junit.Test - - class SubNumsTest { - @Test - fun testSubNumbers() { - assertEquals(SubNums.subNumbers(1, 0), 1) - assertEquals(SubNums.subNumbers(4, 3), 1) - assertEquals(SubNums.subNumbers(0, 0), "Both numbers are zero") - } - } - """.trimIndent() } @After @@ -320,6 +284,40 @@ class RunCoverageTest { "coverage/main/java/com/example/SubNums.kt" ) + val subSourceContent = + """ + package com.example + + class SubNums { + companion object { + fun subNumbers(a: Int, b: Int): Any { + return if (a == 0 && b == 0) { + "Both numbers are zero" + } else { + a - b + } + } + } + } + """.trimIndent() + + val subTestContent = + """ + package com.example + + import org.junit.Assert.assertEquals + import org.junit.Test + + class SubNumsTest { + @Test + fun testSubNumbers() { + assertEquals(SubNums.subNumbers(1, 0), 1) + assertEquals(SubNums.subNumbers(4, 3), 1) + assertEquals(SubNums.subNumbers(0, 0), "Both numbers are zero") + } + } + """.trimIndent() + testBazelWorkspace.initEmptyWorkspace() testBazelWorkspace.addSourceAndTestFileWithContent( @@ -366,6 +364,40 @@ class RunCoverageTest { "coverage/main/java/com/example/SubNums.kt" ) + val subSourceContent = + """ + package com.example + + class SubNums { + companion object { + fun subNumbers(a: Int, b: Int): Any { + return if (a == 0 && b == 0) { + "Both numbers are zero" + } else { + a - b + } + } + } + } + """.trimIndent() + + val subTestContent = + """ + package com.example + + import org.junit.Assert.assertEquals + import org.junit.Test + + class SubNumsTest { + @Test + fun testSubNumbers() { + assertEquals(SubNums.subNumbers(1, 0), 1) + assertEquals(SubNums.subNumbers(4, 3), 1) + assertEquals(SubNums.subNumbers(0, 0), "Both numbers are zero") + } + } + """.trimIndent() + testBazelWorkspace.initEmptyWorkspace() testBazelWorkspace.addSourceAndTestFileWithContent( @@ -406,7 +438,7 @@ class RunCoverageTest { } @Test - fun testRunCoverage_withCoverageStatusFailureFiles_throwsException() { + fun testRunCoverage_withCoverageStatusFail_throwsException() { val filePathList = listOf( "coverage/main/java/com/example/AddNums.kt", "coverage/main/java/com/example/LowTestNums.kt" @@ -478,6 +510,142 @@ class RunCoverageTest { ) } + @Test + fun testRunCoverage_withSuccessFiles_generatesFinalCoverageReport() { + val oppiaDevelopGitHubLink = "https://github.com/oppia/oppia-android/tree/develop" + val filePathList = listOf( + "coverage/main/java/com/example/AddNums.kt" + ) + + testBazelWorkspace.initEmptyWorkspace() + testBazelWorkspace.addSourceAndTestFileWithContent( + filename = "AddNums", + testFilename = "AddNumsTest", + sourceContent = addSourceContent, + testContent = addTestContent, + sourceSubpackage = "coverage/main/java/com/example", + testSubpackage = "coverage/test/java/com/example" + ) + + RunCoverage( + "${tempFolder.root}", + filePathList, + ReportFormat.MARKDOWN, + longCommandExecutor, + scriptBgDispatcher + ).execute() + + val outputReportText = File( + "${tempFolder.root}" + + "$coverageDir/CoverageReport.md" + ).readText() + + val expectedResult = + """ + ## Coverage Report + + - No of files assessed: 1 + - Coverage Status: **PASS** + + +
+ Succeeded Coverages
+ + | Covered File | Percentage | Line Coverage | Status | + |--------------|------------|---------------|--------| + |[AddNums.kt]($oppiaDevelopGitHubLink/${filePathList.get(0)})|75.00%|3 / 4|:white_check_mark:| +
+ """.trimIndent() + + assertThat(outputReportText).isEqualTo(expectedResult) + } + + @Test + fun testRunCoverage_withFailureFiles_generatesFinalCoverageReport() { + val oppiaDevelopGitHubLink = "https://github.com/oppia/oppia-android/tree/develop" + val filePathList = listOf( + "coverage/main/java/com/example/LowTestNums.kt" + ) + + val lowTestSourceContent = + """ + package com.example + + class LowTestNums { + companion object { + fun sumNumbers(a: Int, b: Int): Any { + return if (a == 0 && b == 0) { + "Both numbers are zero" + } else { + a + b + } + } + } + } + """.trimIndent() + + val lowTestTestContent = + """ + package com.example + + import org.junit.Assert.assertEquals + import org.junit.Test + + class LowTestNumsTest { + @Test + fun testSumNumbers() { + assertEquals(1, 1) + } + } + """.trimIndent() + + testBazelWorkspace.initEmptyWorkspace() + testBazelWorkspace.addSourceAndTestFileWithContent( + filename = "LowTestNums", + testFilename = "LowTestNumsTest", + sourceContent = lowTestSourceContent, + testContent = lowTestTestContent, + sourceSubpackage = "coverage/main/java/com/example", + testSubpackage = "coverage/test/java/com/example" + ) + + val exception = assertThrows() { + RunCoverage( + "${tempFolder.root}", + filePathList, + ReportFormat.MARKDOWN, + longCommandExecutor, + scriptBgDispatcher + ).execute() + } + + assertThat(exception).hasMessageThat().contains( + "Coverage Analysis Failed as minimum coverage threshold not met!" + ) + + val outputReportText = File( + "${tempFolder.root}" + + "$coverageDir/CoverageReport.md" + ).readText() + + val expectedResult = + """ + ## Coverage Report + + - No of files assessed: 1 + - Coverage Status: **FAIL** + ### Failed Coverages + Min Coverage Required: 10% + + | Covered File | Percentage | Line Coverage | Status | + |--------------|------------|---------------|--------| + |[LowTestNums.kt]($oppiaDevelopGitHubLink/${filePathList.get(0)})|0.00%|0 / 4|:x:| + + + """.trimIndent() + + assertThat(outputReportText).isEqualTo(expectedResult) + } @Test fun testRunCoverage_withSuccessAndFailureFiles_generatesFinalCoverageReport() { @@ -584,10 +752,65 @@ class RunCoverageTest { } @Test - fun testRunCoverage_withFailureFiles_generatesFinalCoverageReport() { + fun testRunCoverage_withSuccessAndAnomalyFiles_generatesFinalCoverageReport() { val oppiaDevelopGitHubLink = "https://github.com/oppia/oppia-android/tree/develop" val filePathList = listOf( - "coverage/main/java/com/example/LowTestNums.kt" + "coverage/main/java/com/example/AddNums.kt", + "app/src/main/java/org/oppia/android/app/activity/ActivityComponent.kt" + ) + + testBazelWorkspace.initEmptyWorkspace() + testBazelWorkspace.addSourceAndTestFileWithContent( + filename = "AddNums", + testFilename = "AddNumsTest", + sourceContent = addSourceContent, + testContent = addTestContent, + sourceSubpackage = "coverage/main/java/com/example", + testSubpackage = "coverage/test/java/com/example" + ) + + RunCoverage( + "${tempFolder.root}", + filePathList, + ReportFormat.MARKDOWN, + longCommandExecutor, + scriptBgDispatcher + ).execute() + + val outputReportText = File( + "${tempFolder.root}" + + "$coverageDir/CoverageReport.md" + ).readText() + + val expectedResult = + """ + ## Coverage Report + + - No of files assessed: 2 + - Coverage Status: **PASS** + + +
+ Succeeded Coverages
+ + | Covered File | Percentage | Line Coverage | Status | + |--------------|------------|---------------|--------| + |[AddNums.kt]($oppiaDevelopGitHubLink/${filePathList.get(0)})|75.00%|3 / 4|:white_check_mark:| +
+ + ### Anomaly Cases + - The file: [ActivityComponent.kt]($oppiaDevelopGitHubLink/${filePathList.get(1)}) is exempted from having a test file; skipping coverage check. + """.trimIndent() + + assertThat(outputReportText).isEqualTo(expectedResult) + } + + @Test + fun testRunCoverage_withFailureAndAnomalyFiles_generatesFinalCoverageReport() { + val oppiaDevelopGitHubLink = "https://github.com/oppia/oppia-android/tree/develop" + val filePathList = listOf( + "coverage/main/java/com/example/LowTestNums.kt", + "app/src/main/java/org/oppia/android/app/activity/ActivityComponent.kt" ) val lowTestSourceContent = @@ -623,6 +846,7 @@ class RunCoverageTest { """.trimIndent() testBazelWorkspace.initEmptyWorkspace() + testBazelWorkspace.addSourceAndTestFileWithContent( filename = "LowTestNums", testFilename = "LowTestNumsTest", @@ -655,30 +879,67 @@ class RunCoverageTest { """ ## Coverage Report - - No of files assessed: 1 + - No of files assessed: 2 - Coverage Status: **FAIL** ### Failed Coverages Min Coverage Required: 10% - + | Covered File | Percentage | Line Coverage | Status | |--------------|------------|---------------|--------| |[LowTestNums.kt]($oppiaDevelopGitHubLink/${filePathList.get(0)})|0.00%|0 / 4|:x:| - - + + + + ### Anomaly Cases + - The file: [ActivityComponent.kt]($oppiaDevelopGitHubLink/${filePathList.get(1)}) is exempted from having a test file; skipping coverage check. """.trimIndent() assertThat(outputReportText).isEqualTo(expectedResult) } @Test - fun testRunCoverage_withFailureAndAnomalyFiles_generatesFinalCoverageReport() { + fun testRunCoverage_withSuccessFailureAndAnomalyFiles_generatesFinalCoverageReport() { val oppiaDevelopGitHubLink = "https://github.com/oppia/oppia-android/tree/develop" val filePathList = listOf( "coverage/main/java/com/example/AddNums.kt", + "coverage/main/java/com/example/LowTestNums.kt", "app/src/main/java/org/oppia/android/app/activity/ActivityComponent.kt" ) + val lowTestSourceContent = + """ + package com.example + + class LowTestNums { + companion object { + fun sumNumbers(a: Int, b: Int): Any { + return if (a == 0 && b == 0) { + "Both numbers are zero" + } else { + a + b + } + } + } + } + """.trimIndent() + + val lowTestTestContent = + """ + package com.example + + import org.junit.Assert.assertEquals + import org.junit.Test + + class LowTestNumsTest { + @Test + fun testSumNumbers() { + assertEquals(1, 1) + } + } + """.trimIndent() + testBazelWorkspace.initEmptyWorkspace() + testBazelWorkspace.addSourceAndTestFileWithContent( filename = "AddNums", testFilename = "AddNumsTest", @@ -688,13 +949,28 @@ class RunCoverageTest { testSubpackage = "coverage/test/java/com/example" ) - RunCoverage( - "${tempFolder.root}", - filePathList, - ReportFormat.MARKDOWN, - longCommandExecutor, - scriptBgDispatcher - ).execute() + testBazelWorkspace.addSourceAndTestFileWithContent( + filename = "LowTestNums", + testFilename = "LowTestNumsTest", + sourceContent = lowTestSourceContent, + testContent = lowTestTestContent, + sourceSubpackage = "coverage/main/java/com/example", + testSubpackage = "coverage/test/java/com/example" + ) + + val exception = assertThrows() { + RunCoverage( + "${tempFolder.root}", + filePathList, + ReportFormat.MARKDOWN, + longCommandExecutor, + scriptBgDispatcher + ).execute() + } + + assertThat(exception).hasMessageThat().contains( + "Coverage Analysis Failed as minimum coverage threshold not met!" + ) val outputReportText = File( "${tempFolder.root}" + @@ -705,9 +981,14 @@ class RunCoverageTest { """ ## Coverage Report - - No of files assessed: 2 - - Coverage Status: **PASS** + - No of files assessed: 3 + - Coverage Status: **FAIL** + ### Failed Coverages + Min Coverage Required: 10% + | Covered File | Percentage | Line Coverage | Status | + |--------------|------------|---------------|--------| + |[LowTestNums.kt]($oppiaDevelopGitHubLink/${filePathList.get(1)})|0.00%|0 / 4|:x:|
Succeeded Coverages
@@ -718,7 +999,7 @@ class RunCoverageTest {
### Anomaly Cases - - The file: [ActivityComponent.kt]($oppiaDevelopGitHubLink/${filePathList.get(1)}) is exempted from having a test file; skipping coverage check. + - The file: [ActivityComponent.kt]($oppiaDevelopGitHubLink/${filePathList.get(2)}) is exempted from having a test file; skipping coverage check. """.trimIndent() assertThat(outputReportText).isEqualTo(expectedResult)