From b33ca1c36cafeca1581b835a978e18bf1e60759d Mon Sep 17 00:00:00 2001 From: RD Rama Devi <122200035+Rd4dev@users.noreply.github.com> Date: Sat, 13 Jul 2024 03:39:48 +0530 Subject: [PATCH 1/3] Fix part of #5343: Code Coverage script edge cases (#5453) ## Explanation 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 - [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 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 --- .../scripts/coverage/CoverageRunner.kt | 9 +- .../android/scripts/coverage/RunCoverage.kt | 21 +- .../scripts/testing/TestBazelWorkspace.kt | 9 +- .../android/scripts/coverage/BUILD.bazel | 2 + .../scripts/coverage/CoverageReporterTest.kt | 6 +- .../scripts/coverage/RunCoverageTest.kt | 1832 +++-------------- .../scripts/testing/TestBazelWorkspaceTest.kt | 35 +- 7 files changed, 368 insertions(+), 1546 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/coverage/CoverageRunner.kt b/scripts/src/java/org/oppia/android/scripts/coverage/CoverageRunner.kt index e1ff9cdb221..b27cb822dbb 100644 --- a/scripts/src/java/org/oppia/android/scripts/coverage/CoverageRunner.kt +++ b/scripts/src/java/org/oppia/android/scripts/coverage/CoverageRunner.kt @@ -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 -> @@ -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 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 bf65548aecc..db5d381a711 100644 --- a/scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt +++ b/scripts/src/java/org/oppia/android/scripts/coverage/RunCoverage.kt @@ -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 @@ -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") } @@ -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) @@ -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 { diff --git a/scripts/src/java/org/oppia/android/scripts/testing/TestBazelWorkspace.kt b/scripts/src/java/org/oppia/android/scripts/testing/TestBazelWorkspace.kt index d9ba7d2fd70..482425d64c7 100644 --- a/scripts/src/java/org/oppia/android/scripts/testing/TestBazelWorkspace.kt +++ b/scripts/src/java/org/oppia/android/scripts/testing/TestBazelWorkspace.kt @@ -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() diff --git a/scripts/src/javatests/org/oppia/android/scripts/coverage/BUILD.bazel b/scripts/src/javatests/org/oppia/android/scripts/coverage/BUILD.bazel index d67bf159ebe..b58902a767a 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/coverage/BUILD.bazel +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/BUILD.bazel @@ -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", diff --git a/scripts/src/javatests/org/oppia/android/scripts/coverage/CoverageReporterTest.kt b/scripts/src/javatests/org/oppia/android/scripts/coverage/CoverageReporterTest.kt index 67db3ebb1a4..155885cce3d 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/coverage/CoverageReporterTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/CoverageReporterTest.kt @@ -71,7 +71,7 @@ class CoverageReporterTest { } @Test - fun testCoverageReporter_generateHTMLReport_hasCorrectContentAndFormatting() { + fun testCoverageReporter_generateHtmlReport_hasCorrectContentAndFormatting() { val sourceFile = tempFolder.newFile("SampleFile.kt") sourceFile.writeText( """ @@ -95,7 +95,7 @@ class CoverageReporterTest { ) val (_, reportText) = reporter.generateRichTextReport() - val expectedHTML = + val expectedHtml = """ @@ -261,6 +261,6 @@ class CoverageReporterTest { """.trimIndent() - assertThat(reportText).isEqualTo(expectedHTML) + assertThat(reportText).isEqualTo(expectedHtml) } } 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 b00415fcc31..a61a580373b 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt @@ -23,21 +23,56 @@ class RunCoverageTest { private val originalOut: PrintStream = System.out private val scriptBgDispatcher by lazy { ScriptBackgroundCoroutineDispatcher() } - private val commandExecutor by lazy { CommandExecutorImpl(scriptBgDispatcher) } private val longCommandExecutor by lazy { initializeCommandExecutorWithLongProcessWaitTime() } private lateinit var testBazelWorkspace: TestBazelWorkspace - private lateinit var sampleFilePath: String - private lateinit var sampleMDOutputPath: String - private lateinit var sampleHTMLOutputPath: String + private lateinit var coverageDir: String + private lateinit var markdownOutputPath: String + private lateinit var htmlOutputPath: String + + private lateinit var sourceContent: String + private lateinit var testContent: String @Before fun setUp() { - sampleFilePath = "/path/to/Sample.kt" - sampleMDOutputPath = "${tempFolder.root}/coverage_reports/report.md" - sampleHTMLOutputPath = "${tempFolder.root}/coverage_reports/report.html" + coverageDir = "/coverage_reports" + markdownOutputPath = "${tempFolder.root}/coverage_reports/report.md" + htmlOutputPath = "${tempFolder.root}/coverage_reports/report.html" testBazelWorkspace = TestBazelWorkspace(tempFolder) - System.setOut(PrintStream(outContent)) + + 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() + + 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() } @After @@ -68,64 +103,121 @@ class RunCoverageTest { assertThat(exception).hasMessageThat().contains("No appropriate test file found") } + @Test + fun testRunCoverage_invalidFormat_throwsException() { + testBazelWorkspace.initEmptyWorkspace() + val exception = assertThrows() { + main(tempFolder.root.absolutePath, "file.kt", "format=PDF") + } + + assertThat(exception).hasMessageThat().contains("Unsupported report format") + } + + @Test + fun testRunCoverage_ignoreCaseMarkdownArgument_returnsCoverageData() { + val filePath = "coverage/main/java/com/example/TwoSum.kt" + + testBazelWorkspace.initEmptyWorkspace() + testBazelWorkspace.addSourceAndTestFileWithContent( + filename = "TwoSum", + testFilename = "TwoSumTest", + sourceContent = sourceContent, + testContent = testContent, + sourceSubpackage = "coverage/main/java/com/example", + testSubpackage = "coverage/test/java/com/example" + ) + + main( + "${tempFolder.root}", + filePath, + "format=Markdown", + "processTimeout=10" + ) + + val outputFilePath = "${tempFolder.root}" + + "$coverageDir/${filePath.removeSuffix(".kt")}/coverage.md" + + assertThat(File(outputFilePath).exists()).isTrue() + } + + @Test + fun testRunCoverage_ignoreCaseHtmlArgument_returnsCoverageData() { + val filePath = "coverage/main/java/com/example/TwoSum.kt" + + testBazelWorkspace.initEmptyWorkspace() + testBazelWorkspace.addSourceAndTestFileWithContent( + filename = "TwoSum", + testFilename = "TwoSumTest", + sourceContent = sourceContent, + testContent = testContent, + sourceSubpackage = "coverage/main/java/com/example", + testSubpackage = "coverage/test/java/com/example" + ) + + main( + "${tempFolder.root}", + filePath, + "format=Html", + "processTimeout=10" + ) + + val outputFilePath = "${tempFolder.root}" + + "$coverageDir/${filePath.removeSuffix(".kt")}/coverage.html" + + assertThat(File(outputFilePath).exists()).isTrue() + } + + @Test + fun testRunCoverage_reorderedArguments_returnsCoverageData() { + val filePath = "coverage/main/java/com/example/TwoSum.kt" + + testBazelWorkspace.initEmptyWorkspace() + testBazelWorkspace.addSourceAndTestFileWithContent( + filename = "TwoSum", + testFilename = "TwoSumTest", + sourceContent = sourceContent, + testContent = testContent, + sourceSubpackage = "coverage/main/java/com/example", + testSubpackage = "coverage/test/java/com/example" + ) + + main( + "${tempFolder.root}", + filePath, + "processTimeout=10", + "format=MARKDOWN" + ) + + val outputFilePath = "${tempFolder.root}" + + "$coverageDir/${filePath.removeSuffix(".kt")}/coverage.md" + + assertThat(File(outputFilePath).exists()).isTrue() + } + @Test fun testRunCoverage_testFileExempted_noCoverage() { + System.setOut(PrintStream(outContent)) val exemptedFilePath = "app/src/main/java/org/oppia/android/app/activity/ActivityComponent.kt" - val result = RunCoverage( + RunCoverage( "${tempFolder.root}", exemptedFilePath, ReportFormat.MARKDOWN, - sampleMDOutputPath, - commandExecutor, + markdownOutputPath, + longCommandExecutor, scriptBgDispatcher ).execute() - assertThat(result).isEqualTo( + assertThat(outContent.toString().trim()).isEqualTo( "This file is exempted from having a test file; skipping coverage check." ) } @Test fun testRunCoverage_sampleTestsDefaultFormat_returnsCoverageData() { - 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() + val filePath = "coverage/main/java/com/example/TwoSum.kt" + testBazelWorkspace.initEmptyWorkspace() testBazelWorkspace.addSourceAndTestFileWithContent( filename = "TwoSum", testFilename = "TwoSumTest", @@ -137,65 +229,24 @@ class RunCoverageTest { main( "${tempFolder.root}", - "coverage/main/java/com/example/TwoSum.kt", + filePath, ) val outputReportText = File( - "${tempFolder.root}/coverage_reports/coverage/main/java/com/example/TwoSum/coverage.md" + "${tempFolder.root}" + + "$coverageDir/${filePath.removeSuffix(".kt")}/coverage.md" ).readText() - val expectedResult = - """ - ## Coverage Report - - - **Covered File:** coverage/main/java/com/example/TwoSum.kt - - **Coverage percentage:** 75.00% covered - - **Line coverage:** 3 / 4 lines covered - """.trimIndent() + val expectedResult = getExpectedMarkdownText(filePath) assertThat(outputReportText).isEqualTo(expectedResult) } @Test fun testRunCoverage_sampleTestsMarkdownFormat_returnsCoverageData() { - 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() + val filePath = "coverage/main/java/com/example/TwoSum.kt" + testBazelWorkspace.initEmptyWorkspace() testBazelWorkspace.addSourceAndTestFileWithContent( filename = "TwoSum", testFilename = "TwoSumTest", @@ -207,67 +258,24 @@ class RunCoverageTest { RunCoverage( "${tempFolder.root}", - "coverage/main/java/com/example/TwoSum.kt", + filePath, ReportFormat.MARKDOWN, - sampleMDOutputPath, + markdownOutputPath, longCommandExecutor, scriptBgDispatcher ).execute() - val outputReportText = File(sampleMDOutputPath).readText() - - val expectedResult = - """ - ## Coverage Report - - - **Covered File:** coverage/main/java/com/example/TwoSum.kt - - **Coverage percentage:** 75.00% covered - - **Line coverage:** 3 / 4 lines covered - """.trimIndent() + val outputReportText = File(markdownOutputPath).readText() + val expectedResult = getExpectedMarkdownText(filePath) assertThat(outputReportText).isEqualTo(expectedResult) } @Test fun testRunCoverage_scriptTestsMarkdownFormat_returnsCoverageData() { - 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() + val filePath = "scripts/java/com/example/TwoSum.kt" + testBazelWorkspace.initEmptyWorkspace() testBazelWorkspace.addSourceAndTestFileWithContent( filename = "TwoSum", testFilename = "TwoSumTest", @@ -279,67 +287,24 @@ class RunCoverageTest { RunCoverage( "${tempFolder.root}", - "scripts/java/com/example/TwoSum.kt", + filePath, ReportFormat.MARKDOWN, - sampleMDOutputPath, + markdownOutputPath, longCommandExecutor, scriptBgDispatcher ).execute() - val outputReportText = File(sampleMDOutputPath).readText() - - val expectedResult = - """ - ## Coverage Report - - - **Covered File:** scripts/java/com/example/TwoSum.kt - - **Coverage percentage:** 75.00% covered - - **Line coverage:** 3 / 4 lines covered - """.trimIndent() + val outputReportText = File(markdownOutputPath).readText() + val expectedResult = getExpectedMarkdownText(filePath) assertThat(outputReportText).isEqualTo(expectedResult) } @Test fun testRunCoverage_appTestsMarkdownFormat_returnsCoverageData() { - 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() + val filePath = "app/main/java/com/example/TwoSum.kt" + testBazelWorkspace.initEmptyWorkspace() testBazelWorkspace.addSourceAndTestFileWithContent( filename = "TwoSum", testFilename = "TwoSumTest", @@ -351,50 +316,25 @@ class RunCoverageTest { RunCoverage( "${tempFolder.root}", - "app/main/java/com/example/TwoSum.kt", + filePath, ReportFormat.MARKDOWN, - sampleMDOutputPath, + markdownOutputPath, longCommandExecutor, scriptBgDispatcher ).execute() - val outputReportText = File(sampleMDOutputPath).readText() - - val expectedResult = - """ - ## Coverage Report - - - **Covered File:** app/main/java/com/example/TwoSum.kt - - **Coverage percentage:** 75.00% covered - - **Line coverage:** 3 / 4 lines covered - """.trimIndent() + val outputReportText = File(markdownOutputPath).readText() + val expectedResult = getExpectedMarkdownText(filePath) assertThat(outputReportText).isEqualTo(expectedResult) } @Test fun testRunCoverage_localTestsMarkdownFormat_returnsCoverageData() { - 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 filePath = "app/main/java/com/example/TwoSum.kt" - val testContent = + testBazelWorkspace.initEmptyWorkspace() + val testContentLocal = """ package com.example @@ -416,74 +356,31 @@ class RunCoverageTest { filename = "TwoSum", testFilename = "TwoSumLocalTest", sourceContent = sourceContent, - testContent = testContent, + testContent = testContentLocal, sourceSubpackage = "app/main/java/com/example", testSubpackage = "app/test/java/com/example" ) RunCoverage( "${tempFolder.root}", - "app/main/java/com/example/TwoSum.kt", + filePath, ReportFormat.MARKDOWN, - sampleMDOutputPath, + markdownOutputPath, longCommandExecutor, scriptBgDispatcher ).execute() - val outputReportText = File(sampleMDOutputPath).readText() - - val expectedResult = - """ - ## Coverage Report - - - **Covered File:** app/main/java/com/example/TwoSum.kt - - **Coverage percentage:** 75.00% covered - - **Line coverage:** 3 / 4 lines covered - """.trimIndent() + val outputReportText = File(markdownOutputPath).readText() + val expectedResult = getExpectedMarkdownText(filePath) assertThat(outputReportText).isEqualTo(expectedResult) } @Test fun testRunCoverage_sharedTestsMarkdownFormat_returnsCoverageData() { - 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() + val filePath = "app/main/java/com/example/TwoSum.kt" + testBazelWorkspace.initEmptyWorkspace() testBazelWorkspace.addSourceAndTestFileWithContent( filename = "TwoSum", testFilename = "TwoSumTest", @@ -495,67 +392,24 @@ class RunCoverageTest { RunCoverage( "${tempFolder.root}", - "app/main/java/com/example/TwoSum.kt", + filePath, ReportFormat.MARKDOWN, - sampleMDOutputPath, + markdownOutputPath, longCommandExecutor, scriptBgDispatcher ).execute() - val outputReportText = File(sampleMDOutputPath).readText() - - val expectedResult = - """ - ## Coverage Report - - - **Covered File:** app/main/java/com/example/TwoSum.kt - - **Coverage percentage:** 75.00% covered - - **Line coverage:** 3 / 4 lines covered - """.trimIndent() + val outputReportText = File(markdownOutputPath).readText() + val expectedResult = getExpectedMarkdownText(filePath) assertThat(outputReportText).isEqualTo(expectedResult) } @Test fun testRunCoverage_sharedAndLocalTestsMarkdownFormat_returnsCoverageData() { - 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 testContentShared = - """ - 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() + val filePath = "app/main/java/com/example/TwoSum.kt" + testBazelWorkspace.initEmptyWorkspace() val testContentLocal = """ package com.example @@ -577,1028 +431,126 @@ class RunCoverageTest { testBazelWorkspace.addMultiLevelSourceAndTestFileWithContent( filename = "TwoSum", sourceContent = sourceContent, - testContentShared = testContentShared, + testContentShared = testContent, testContentLocal = testContentLocal, subpackage = "app" ) RunCoverage( "${tempFolder.root}", - "app/main/java/com/example/TwoSum.kt", + filePath, ReportFormat.MARKDOWN, - sampleMDOutputPath, + markdownOutputPath, longCommandExecutor, scriptBgDispatcher ).execute() - val outputReportText = File(sampleMDOutputPath).readText() - - val expectedResult = - """ - ## Coverage Report - - - **Covered File:** app/main/java/com/example/TwoSum.kt - - **Coverage percentage:** 75.00% covered - - **Line coverage:** 3 / 4 lines covered - """.trimIndent() + val outputReportText = File(markdownOutputPath).readText() + val expectedResult = getExpectedMarkdownText(filePath) assertThat(outputReportText).isEqualTo(expectedResult) } @Test - fun testRunCoverage_sampleTestsHTMLFormat_returnsCoverageData() { + fun testRunCoverage_sampleTestsHtmlFormat_returnsCoverageData() { + val filePath = "coverage/main/java/com/example/TwoSum.kt" + testBazelWorkspace.initEmptyWorkspace() + testBazelWorkspace.addSourceAndTestFileWithContent( + filename = "TwoSum", + testFilename = "TwoSumTest", + sourceContent = sourceContent, + testContent = testContent, + sourceSubpackage = "coverage/main/java/com/example", + testSubpackage = "coverage/test/java/com/example" + ) - 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() + RunCoverage( + "${tempFolder.root}", + filePath, + ReportFormat.HTML, + htmlOutputPath, + longCommandExecutor, + scriptBgDispatcher + ).execute() - 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() + val outputReportText = File(htmlOutputPath).readText() + val expectedResult = getExpectedHtmlText(filePath) + + assertThat(outputReportText).isEqualTo(expectedResult) + } + @Test + fun testRunCoverage_scriptTestsHtmlFormat_returnsCoverageData() { + val filePath = "scripts/java/com/example/TwoSum.kt" + + testBazelWorkspace.initEmptyWorkspace() testBazelWorkspace.addSourceAndTestFileWithContent( filename = "TwoSum", testFilename = "TwoSumTest", sourceContent = sourceContent, testContent = testContent, - sourceSubpackage = "coverage/main/java/com/example", - testSubpackage = "coverage/test/java/com/example" + sourceSubpackage = "scripts/java/com/example", + testSubpackage = "scripts/javatests/com/example" ) RunCoverage( "${tempFolder.root}", - "coverage/main/java/com/example/TwoSum.kt", + filePath, ReportFormat.HTML, - sampleHTMLOutputPath, + htmlOutputPath, longCommandExecutor, scriptBgDispatcher ).execute() - val outputReportText = File(sampleHTMLOutputPath).readText() - - val expectedResult = - """ - - - - - - Coverage Report - - - -

Coverage Report

-
-
- Covered File: coverage/main/java/com/example/TwoSum.kt
-
-
- Covered -
- Uncovered -
-
-
-
Coverage percentage: 75.00%
-
Line coverage: 3 / 4 covered
-
-
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Line NoSource Code
1package com.example
2
3class TwoSum {
4
5 companion object {
6 fun sumNumbers(a: Int, b: Int): Any {
7 return if (a == 0 && b == 0) {
8 "Both numbers are zero"
9 } else {
10 a + b
11 }
12 }
13 }
14}
- - - """.trimIndent() - - assertThat(outputReportText).isEqualTo(expectedResult) - } - - @Test - fun testRunCoverage_scriptTestsHTMLFormat_returnsCoverageData() { - 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", - testFilename = "TwoSumTest", - sourceContent = sourceContent, - testContent = testContent, - sourceSubpackage = "scripts/java/com/example", - testSubpackage = "scripts/javatests/com/example" - ) - - RunCoverage( - "${tempFolder.root}", - "scripts/java/com/example/TwoSum.kt", - ReportFormat.HTML, - sampleHTMLOutputPath, - longCommandExecutor, - scriptBgDispatcher - ).execute() - - val outputReportText = File(sampleHTMLOutputPath).readText() - - val expectedResult = - """ - - - - - - Coverage Report - - - -

Coverage Report

-
-
- Covered File: scripts/java/com/example/TwoSum.kt
-
-
- Covered -
- Uncovered -
-
-
-
Coverage percentage: 75.00%
-
Line coverage: 3 / 4 covered
-
-
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Line NoSource Code
1package com.example
2
3class TwoSum {
4
5 companion object {
6 fun sumNumbers(a: Int, b: Int): Any {
7 return if (a == 0 && b == 0) {
8 "Both numbers are zero"
9 } else {
10 a + b
11 }
12 }
13 }
14}
- - - """.trimIndent() - - assertThat(outputReportText).isEqualTo(expectedResult) - } - - @Test - fun testRunCoverage_appTestsHTMLFormat_returnsCoverageData() { - 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", - testFilename = "TwoSumTest", - sourceContent = sourceContent, - testContent = testContent, - sourceSubpackage = "app/main/java/com/example", - testSubpackage = "app/test/java/com/example" - ) - - RunCoverage( - "${tempFolder.root}", - "app/main/java/com/example/TwoSum.kt", - ReportFormat.HTML, - sampleHTMLOutputPath, - longCommandExecutor, - scriptBgDispatcher - ).execute() - - val outputReportText = File(sampleHTMLOutputPath).readText() - - val expectedResult = - """ - - - - - - Coverage Report - - - -

Coverage Report

-
-
- Covered File: app/main/java/com/example/TwoSum.kt
-
-
- Covered -
- Uncovered -
-
-
-
Coverage percentage: 75.00%
-
Line coverage: 3 / 4 covered
-
-
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Line NoSource Code
1package com.example
2
3class TwoSum {
4
5 companion object {
6 fun sumNumbers(a: Int, b: Int): Any {
7 return if (a == 0 && b == 0) {
8 "Both numbers are zero"
9 } else {
10 a + b
11 }
12 }
13 }
14}
- - - """.trimIndent() + val outputReportText = File(htmlOutputPath).readText() + val expectedResult = getExpectedHtmlText(filePath) assertThat(outputReportText).isEqualTo(expectedResult) } @Test - fun testRunCoverage_localTestsHTMLFormat_returnsCoverageData() { - 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 TwoSumLocalTest { - - @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() + fun testRunCoverage_appTestsHtmlFormat_returnsCoverageData() { + val filePath = "app/main/java/com/example/TwoSum.kt" + testBazelWorkspace.initEmptyWorkspace() testBazelWorkspace.addSourceAndTestFileWithContent( filename = "TwoSum", - testFilename = "TwoSumLocalTest", + testFilename = "TwoSumTest", sourceContent = sourceContent, testContent = testContent, sourceSubpackage = "app/main/java/com/example", testSubpackage = "app/test/java/com/example" ) - RunCoverage( - "${tempFolder.root}", - "app/main/java/com/example/TwoSum.kt", - ReportFormat.HTML, - sampleHTMLOutputPath, - longCommandExecutor, - scriptBgDispatcher - ).execute() - - val outputReportText = File(sampleHTMLOutputPath).readText() - - val expectedResult = - """ - - - - - - Coverage Report - - - -

Coverage Report

-
-
- Covered File: app/main/java/com/example/TwoSum.kt
-
-
- Covered -
- Uncovered -
-
-
-
Coverage percentage: 75.00%
-
Line coverage: 3 / 4 covered
-
-
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Line NoSource Code
1package com.example
2
3class TwoSum {
4
5 companion object {
6 fun sumNumbers(a: Int, b: Int): Any {
7 return if (a == 0 && b == 0) {
8 "Both numbers are zero"
9 } else {
10 a + b
11 }
12 }
13 }
14}
- - - """.trimIndent() + RunCoverage( + "${tempFolder.root}", + filePath, + ReportFormat.HTML, + htmlOutputPath, + longCommandExecutor, + scriptBgDispatcher + ).execute() + + val outputReportText = File(htmlOutputPath).readText() + val expectedResult = getExpectedHtmlText(filePath) assertThat(outputReportText).isEqualTo(expectedResult) } @Test - fun testRunCoverage_sharedTestsHTMLFormat_returnsCoverageData() { - 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() + fun testRunCoverage_localTestsHtmlFormat_returnsCoverageData() { + val filePath = "app/main/java/com/example/TwoSum.kt" - val testContent = + testBazelWorkspace.initEmptyWorkspace() + val testContentLocal = """ package com.example import org.junit.Assert.assertEquals import org.junit.Test - class TwoSumTest { + class TwoSumLocalTest { @Test fun testSumNumbers() { @@ -1611,245 +563,62 @@ class RunCoverageTest { testBazelWorkspace.addSourceAndTestFileWithContent( filename = "TwoSum", - testFilename = "TwoSumTest", + testFilename = "TwoSumLocalTest", sourceContent = sourceContent, - testContent = testContent, + testContent = testContentLocal, sourceSubpackage = "app/main/java/com/example", - testSubpackage = "app/sharedTest/java/com/example" + testSubpackage = "app/test/java/com/example" ) RunCoverage( "${tempFolder.root}", - "app/main/java/com/example/TwoSum.kt", + filePath, ReportFormat.HTML, - sampleHTMLOutputPath, + htmlOutputPath, longCommandExecutor, scriptBgDispatcher ).execute() - val outputReportText = File(sampleHTMLOutputPath).readText() - - val expectedResult = - """ - - - - - - Coverage Report - - - -

Coverage Report

-
-
- Covered File: app/main/java/com/example/TwoSum.kt
-
-
- Covered -
- Uncovered -
-
-
-
Coverage percentage: 75.00%
-
Line coverage: 3 / 4 covered
-
-
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Line NoSource Code
1package com.example
2
3class TwoSum {
4
5 companion object {
6 fun sumNumbers(a: Int, b: Int): Any {
7 return if (a == 0 && b == 0) {
8 "Both numbers are zero"
9 } else {
10 a + b
11 }
12 }
13 }
14}
- - - """.trimIndent() + val outputReportText = File(htmlOutputPath).readText() + val expectedResult = getExpectedHtmlText(filePath) assertThat(outputReportText).isEqualTo(expectedResult) } @Test - fun testRunCoverage_sharedAndLocalTestsHTMLFormat_returnsCoverageData() { + fun testRunCoverage_sharedTestsHtmlFormat_returnsCoverageData() { + val filePath = "app/main/java/com/example/TwoSum.kt" + testBazelWorkspace.initEmptyWorkspace() + testBazelWorkspace.addSourceAndTestFileWithContent( + filename = "TwoSum", + testFilename = "TwoSumTest", + sourceContent = sourceContent, + testContent = testContent, + sourceSubpackage = "app/main/java/com/example", + testSubpackage = "app/sharedTest/java/com/example" + ) - 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() + RunCoverage( + "${tempFolder.root}", + filePath, + ReportFormat.HTML, + htmlOutputPath, + longCommandExecutor, + scriptBgDispatcher + ).execute() - val testContentShared = - """ - 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() + val outputReportText = File(htmlOutputPath).readText() + val expectedResult = getExpectedHtmlText(filePath) + + assertThat(outputReportText).isEqualTo(expectedResult) + } + @Test + fun testRunCoverage_sharedAndLocalTestsHtmlFormat_returnsCoverageData() { + val filePath = "app/main/java/com/example/TwoSum.kt" + + testBazelWorkspace.initEmptyWorkspace() val testContentLocal = """ package com.example @@ -1871,7 +640,7 @@ class RunCoverageTest { testBazelWorkspace.addMultiLevelSourceAndTestFileWithContent( filename = "TwoSum", sourceContent = sourceContent, - testContentShared = testContentShared, + testContentShared = testContent, testContentLocal = testContentLocal, subpackage = "app" ) @@ -1880,14 +649,32 @@ class RunCoverageTest { "${tempFolder.root}", "app/main/java/com/example/TwoSum.kt", ReportFormat.HTML, - sampleHTMLOutputPath, + htmlOutputPath, longCommandExecutor, scriptBgDispatcher ).execute() - val outputReportText = File(sampleHTMLOutputPath).readText() + val outputReportText = File(htmlOutputPath).readText() + val expectedResult = getExpectedHtmlText(filePath) + + assertThat(outputReportText).isEqualTo(expectedResult) + } + + private fun getExpectedMarkdownText(filePath: String): String { + val markdownText = + """ + ## Coverage Report + + - **Covered File:** $filePath + - **Coverage percentage:** 75.00% covered + - **Line coverage:** 3 / 4 lines covered + """.trimIndent() + + return markdownText + } - val expectedResult = + private fun getExpectedHtmlText(filePath: String): String { + val htmlText = """ @@ -1997,7 +784,7 @@ class RunCoverageTest {

Coverage Report

- Covered File: app/main/java/com/example/TwoSum.kt
+ Covered File: $filePath
Covered @@ -2028,36 +815,33 @@ class RunCoverageTest { class TwoSum { 4 - + companion object { 5 - companion object { + fun sumNumbers(a: Int, b: Int): Any { 6 - fun sumNumbers(a: Int, b: Int): Any { + return if (a == 0 && b == 0) { 7 - return if (a == 0 && b == 0) { + "Both numbers are zero" 8 - "Both numbers are zero" + } else { 9 - } else { + a + b 10 - a + b + } 11 - } + } 12 - } + } 13 - } - - 14 } @@ -2065,7 +849,7 @@ class RunCoverageTest { """.trimIndent() - assertThat(outputReportText).isEqualTo(expectedResult) + return htmlText } private fun initializeCommandExecutorWithLongProcessWaitTime(): CommandExecutorImpl { diff --git a/scripts/src/javatests/org/oppia/android/scripts/testing/TestBazelWorkspaceTest.kt b/scripts/src/javatests/org/oppia/android/scripts/testing/TestBazelWorkspaceTest.kt index aeab27fe8d3..00d7c167a71 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/testing/TestBazelWorkspaceTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/testing/TestBazelWorkspaceTest.kt @@ -67,7 +67,12 @@ class TestBazelWorkspaceTest { // A .bazelversion file should now exist with the correct flags. val bazelRcFile = File(tempFolder.root, ".bazelrc") assertThat(bazelRcFile.exists()).isTrue() - assertThat(bazelRcFile.readText().trim()).isEqualTo("--noenable_bzlmod") + assertThat(bazelRcFile.readText().trim()).isEqualTo( + """ + --noenable_bzlmod + build --java_runtime_version=remotejdk_11 --tool_java_runtime_version=remotejdk_11 + """.trimIndent() + ) } @Test @@ -132,7 +137,12 @@ class TestBazelWorkspaceTest { ) val bazelRcContent = tempFolder.getBazelRcFile().readText().trim() - assertThat(bazelRcContent).isEqualTo("--noenable_bzlmod") + assertThat(bazelRcContent).isEqualTo( + """ + --noenable_bzlmod + build --java_runtime_version=remotejdk_11 --tool_java_runtime_version=remotejdk_11 + """.trimIndent() + ) } @Test @@ -636,7 +646,12 @@ class TestBazelWorkspaceTest { ) val bazelRcContent = tempFolder.getBazelRcFile().readText().trim() - assertThat(bazelRcContent).isEqualTo("--noenable_bzlmod") + assertThat(bazelRcContent).isEqualTo( + """ + --noenable_bzlmod + build --java_runtime_version=remotejdk_11 --tool_java_runtime_version=remotejdk_11 + """.trimIndent() + ) } @Test @@ -911,7 +926,12 @@ class TestBazelWorkspaceTest { testBazelWorkspace.createTest(testName = "FirstTest") val bazelRcContent = tempFolder.getBazelRcFile().readText().trim() - assertThat(bazelRcContent).isEqualTo("--noenable_bzlmod") + assertThat(bazelRcContent).isEqualTo( + """ + --noenable_bzlmod + build --java_runtime_version=remotejdk_11 --tool_java_runtime_version=remotejdk_11 + """.trimIndent() + ) } @Test @@ -1131,7 +1151,12 @@ class TestBazelWorkspaceTest { testBazelWorkspace.createLibrary(dependencyName = "ExampleDep") val bazelRcContent = tempFolder.getBazelRcFile().readText().trim() - assertThat(bazelRcContent).isEqualTo("--noenable_bzlmod") + assertThat(bazelRcContent).isEqualTo( + """ + --noenable_bzlmod + build --java_runtime_version=remotejdk_11 --tool_java_runtime_version=remotejdk_11 + """.trimIndent() + ) } @Test From 6e12374c00d5acfcd7ee90d750be9407b733174f Mon Sep 17 00:00:00 2001 From: "Mr. 17" Date: Wed, 17 Jul 2024 04:00:13 +0530 Subject: [PATCH 2/3] Fix part of #5344: Implement event logs for multiple classrooms (#5456) ## Explanation Fixes part of #5344 - Ensures `open_home` & `complete_app_onboarding` event logs are captured in the new screen. - Adds `classroomId` field to the `ExplorationContext` object and updates related tests. - Implements logging of `start_exploration` event and adds tests. - Updates `FeatureFlagLogger` with the `ENABLE_MULTIPLE_CLASSROOMS` feature flag. ## Screen Recording ### Feature Flag On https://github.com/user-attachments/assets/10c695fa-abfd-4873-826c-c322524c9453 ### Feature Flag Off https://github.com/user-attachments/assets/a6581895-2c10-4561-9912-5a065c8c937b ## Essential Checklist - [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 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 --- .../app/activity/ActivityIntentFactories.kt | 20 +- .../app/classroom/ClassroomListActivity.kt | 19 +- .../ClassroomListFragmentPresenter.kt | 52 ++ .../CompletedStoryItemViewModel.kt | 15 +- .../oppia/android/app/home/HomeActivity.kt | 14 +- .../android/app/home/HomeFragmentPresenter.kt | 1 + .../app/home/RouteToExplorationListener.kt | 1 + .../android/app/home/RouteToTopicListener.kt | 2 +- .../app/home/RouteToTopicPlayStoryListener.kt | 7 +- .../promotedlist/PromotedStoryViewModel.kt | 1 + .../recentlyplayed/RecentlyPlayedActivity.kt | 4 + .../RecentlyPlayedFragmentPresenter.kt | 9 +- .../OngoingTopicItemViewModel.kt | 5 +- .../player/exploration/ExplorationActivity.kt | 3 + .../ExplorationActivityPresenter.kt | 11 +- .../player/exploration/ExplorationFragment.kt | 2 + .../ExplorationFragmentPresenter.kt | 13 +- .../testing/StateFragmentTestActivity.kt | 2 + .../StateFragmentTestActivityPresenter.kt | 19 +- .../RecentlyPlayedStorySummaryViewModel.kt | 12 +- .../app/resumelesson/ResumeLessonActivity.kt | 5 + .../ResumeLessonActivityPresenter.kt | 2 + .../app/resumelesson/ResumeLessonFragment.kt | 3 + .../ResumeLessonFragmentPresenter.kt | 11 +- .../android/app/shim/IntentFactoryShim.kt | 2 + .../android/app/shim/IntentFactoryShimImpl.kt | 7 +- .../app/story/ExplorationSelectionListener.kt | 1 + .../oppia/android/app/story/StoryActivity.kt | 12 +- .../app/story/StoryActivityPresenter.kt | 9 +- .../oppia/android/app/story/StoryFragment.kt | 21 +- .../app/story/StoryFragmentPresenter.kt | 9 + .../oppia/android/app/story/StoryViewModel.kt | 6 + .../StoryChapterSummaryViewModel.kt | 4 + .../app/testing/ExplorationTestActivity.kt | 2 + .../ExplorationTestActivityPresenter.kt | 4 + .../app/testing/HomeFragmentTestActivity.kt | 9 +- .../testing/NavigationDrawerTestActivity.kt | 14 +- .../android/app/testing/TopicTestActivity.kt | 12 +- .../app/testing/TopicTestActivityForStory.kt | 14 +- .../app/topic/RouteToResumeLessonListener.kt | 1 + .../android/app/topic/RouteToStoryListener.kt | 2 +- .../oppia/android/app/topic/TopicActivity.kt | 50 +- .../app/topic/TopicActivityPresenter.kt | 10 +- .../oppia/android/app/topic/TopicFragment.kt | 3 + .../app/topic/TopicFragmentPresenter.kt | 19 +- .../android/app/topic/ViewPagerAdapter.kt | 3 +- .../app/topic/lessons/TopicLessonsFragment.kt | 6 + .../lessons/TopicLessonsFragmentPresenter.kt | 17 +- .../classroom/ClassroomListFragmentTest.kt | 4 + .../CompletedStoryListActivityTest.kt | 3 + .../android/app/home/HomeActivityTest.kt | 5 + .../app/home/RecentlyPlayedFragmentTest.kt | 5 + .../OngoingTopicListActivityTest.kt | 11 +- .../exploration/ExplorationActivityTest.kt | 153 +++- .../app/player/state/StateFragmentTest.kt | 2 + .../ProfileProgressFragmentTest.kt | 2 + .../resumelesson/ResumeLessonActivityTest.kt | 2 + .../resumelesson/ResumeLessonFragmentTest.kt | 3 + .../android/app/story/StoryActivityTest.kt | 7 + .../android/app/story/StoryFragmentTest.kt | 4 + .../android/app/topic/TopicActivityTest.kt | 14 +- .../android/app/topic/TopicFragmentTest.kt | 103 ++- .../app/topic/info/TopicInfoFragmentTest.kt | 18 + .../topic/lessons/TopicLessonsFragmentTest.kt | 257 +++++- .../practice/TopicPracticeFragmentTest.kt | 23 +- .../revision/TopicRevisionFragmentTest.kt | 21 +- .../activity/ActivityIntentFactoriesTest.kt | 9 +- .../ExplorationActivityLocalTest.kt | 16 + .../player/state/StateFragmentLocalTest.kt | 2 + .../app/story/StoryActivityLocalTest.kt | 5 +- .../state/StateFragmentAccessibilityTest.kt | 2 + .../topic/info/TopicInfoFragmentLocalTest.kt | 5 +- .../lessons/TopicLessonsFragmentLocalTest.kt | 7 +- .../domain/classroom/ClassroomController.kt | 89 ++ .../exploration/ExplorationDataController.kt | 10 + .../domain/exploration/ExplorationProgress.kt | 1 + .../ExplorationProgressController.kt | 11 + .../android/domain/oppialogger/OppiaLogger.kt | 2 + .../analytics/FeatureFlagsLogger.kt | 9 +- .../analytics/LearnerAnalyticsLogger.kt | 14 + .../android/domain/topic/TopicController.kt | 7 +- .../domain/audio/AudioPlayerControllerTest.kt | 72 +- .../classroom/ClassroomControllerTest.kt | 42 + .../ExplorationActiveTimeControllerTest.kt | 35 +- .../ExplorationDataControllerTest.kt | 125 ++- .../ExplorationProgressControllerTest.kt | 833 ++++++++++++++---- .../ExplorationCheckpointControllerTest.kt | 1 + .../domain/oppialogger/OppiaLoggerTest.kt | 2 + .../analytics/AnalyticsControllerTest.kt | 3 + .../analytics/FeatureFlagsLoggerTest.kt | 4 +- .../analytics/LearnerAnalyticsLoggerTest.kt | 114 ++- model/src/main/proto/arguments.proto | 30 + model/src/main/proto/oppia_logger.proto | 6 + model/src/main/proto/topic.proto | 6 + .../testing/logging/EventLogSubject.kt | 35 + .../util/logging/EventBundleCreator.kt | 3 + ...entTypeToHumanReadableNameConverterImpl.kt | 1 + ...entTypeToHumanReadableNameConverterImpl.kt | 1 + .../util/logging/EventBundleCreatorTest.kt | 97 +- .../KenyaAlphaEventBundleCreatorTest.kt | 93 +- 100 files changed, 2326 insertions(+), 473 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/activity/ActivityIntentFactories.kt b/app/src/main/java/org/oppia/android/app/activity/ActivityIntentFactories.kt index ab6ab90bf77..078032aabcf 100644 --- a/app/src/main/java/org/oppia/android/app/activity/ActivityIntentFactories.kt +++ b/app/src/main/java/org/oppia/android/app/activity/ActivityIntentFactories.kt @@ -16,15 +16,23 @@ interface ActivityIntentFactories { * This must be injected within an activity context. */ interface TopicActivityIntentFactory { - /** Returns a new [Intent] to start the topic activity for the specified profile and topic. */ - fun createIntent(profileId: ProfileId, topicId: String): Intent + /** + * Returns a new [Intent] to start the topic activity for the specified profile, classroom + * and topic. + */ + fun createIntent(profileId: ProfileId, classroomId: String, topicId: String): Intent /** - * Returns a new [Intent] to start the topic activity for the specified profile, topic, and - * story (where the activity will automatically navigate to & expand the specified story in the - * topic). + * Returns a new [Intent] to start the topic activity for the specified profile, classroom, + * topic, and story (where the activity will automatically navigate to & expand the specified + * story in the topic). */ - fun createIntent(profileId: ProfileId, topicId: String, storyId: String): Intent + fun createIntent( + profileId: ProfileId, + classroomId: String, + topicId: String, + storyId: String + ): Intent } /** diff --git a/app/src/main/java/org/oppia/android/app/classroom/ClassroomListActivity.kt b/app/src/main/java/org/oppia/android/app/classroom/ClassroomListActivity.kt index 4ba06da7d58..c8f075f10bf 100644 --- a/app/src/main/java/org/oppia/android/app/classroom/ClassroomListActivity.kt +++ b/app/src/main/java/org/oppia/android/app/classroom/ClassroomListActivity.kt @@ -19,7 +19,8 @@ import org.oppia.android.app.model.ProfileId import org.oppia.android.app.model.RecentlyPlayedActivityParams import org.oppia.android.app.model.RecentlyPlayedActivityTitle import org.oppia.android.app.model.ScreenName.CLASSROOM_LIST_ACTIVITY -import org.oppia.android.app.topic.TopicActivity +import org.oppia.android.app.topic.TopicActivity.Companion.createTopicActivityIntent +import org.oppia.android.app.topic.TopicActivity.Companion.createTopicPlayStoryActivityIntent import org.oppia.android.app.translation.AppLanguageResourceHandler import org.oppia.android.util.logging.CurrentAppScreenNameIntentDecorator.decorateWithScreenName import org.oppia.android.util.profile.CurrentUserProfileIdIntentDecorator.decorateWithUserProfileId @@ -98,15 +99,23 @@ class ClassroomListActivity : ) } - override fun routeToTopic(internalProfileId: Int, topicId: String) { - startActivity(TopicActivity.createTopicActivityIntent(this, internalProfileId, topicId)) + override fun routeToTopic(internalProfileId: Int, classroomId: String, topicId: String) { + startActivity( + createTopicActivityIntent(this, internalProfileId, classroomId, topicId) + ) } - override fun routeToTopicPlayStory(internalProfileId: Int, topicId: String, storyId: String) { + override fun routeToTopicPlayStory( + internalProfileId: Int, + classroomId: String, + topicId: String, + storyId: String + ) { startActivity( - TopicActivity.createTopicPlayStoryActivityIntent( + createTopicPlayStoryActivityIntent( this, internalProfileId, + classroomId, topicId, storyId ) diff --git a/app/src/main/java/org/oppia/android/app/classroom/ClassroomListFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/classroom/ClassroomListFragmentPresenter.kt index 7bc81fd5bdf..ba75fda470a 100644 --- a/app/src/main/java/org/oppia/android/app/classroom/ClassroomListFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/classroom/ClassroomListFragmentPresenter.kt @@ -27,6 +27,7 @@ import androidx.compose.ui.res.integerResource import androidx.compose.ui.unit.dp import androidx.databinding.ObservableList import androidx.fragment.app.Fragment +import androidx.lifecycle.Observer import org.oppia.android.R import org.oppia.android.app.classroom.classroomlist.ClassroomList import org.oppia.android.app.classroom.promotedlist.PromotedStoryList @@ -40,6 +41,7 @@ import org.oppia.android.app.home.classroomlist.ClassroomSummaryViewModel import org.oppia.android.app.home.promotedlist.PromotedStoryListViewModel import org.oppia.android.app.home.topiclist.AllTopicsViewModel import org.oppia.android.app.home.topiclist.TopicSummaryViewModel +import org.oppia.android.app.model.AppStartupState import org.oppia.android.app.model.ClassroomSummary import org.oppia.android.app.model.LessonThumbnail import org.oppia.android.app.model.LessonThumbnailGraphic @@ -48,10 +50,14 @@ import org.oppia.android.app.translation.AppLanguageResourceHandler import org.oppia.android.app.utility.datetime.DateTimeUtil import org.oppia.android.databinding.ClassroomListFragmentBinding import org.oppia.android.domain.classroom.ClassroomController +import org.oppia.android.domain.onboarding.AppStartupStateController import org.oppia.android.domain.oppialogger.OppiaLogger +import org.oppia.android.domain.oppialogger.analytics.AnalyticsController import org.oppia.android.domain.profile.ProfileManagementController import org.oppia.android.domain.topic.TopicListController import org.oppia.android.domain.translation.TranslationController +import org.oppia.android.util.data.AsyncResult +import org.oppia.android.util.data.DataProviders.Companion.toLiveData import org.oppia.android.util.locale.OppiaLocale import org.oppia.android.util.parser.html.StoryHtmlParserEntityType import org.oppia.android.util.parser.html.TopicHtmlParserEntityType @@ -75,6 +81,8 @@ class ClassroomListFragmentPresenter @Inject constructor( private val dateTimeUtil: DateTimeUtil, private val translationController: TranslationController, private val machineLocale: OppiaLocale.MachineLocale, + private val appStartupStateController: AppStartupStateController, + private val analyticsController: AnalyticsController, ) { private val routeToTopicPlayStoryListener = activity as RouteToTopicPlayStoryListener private lateinit var binding: ClassroomListFragmentBinding @@ -92,6 +100,8 @@ class ClassroomListFragmentPresenter @Inject constructor( internalProfileId = profileId.internalId + logHomeActivityEvent() + classroomListViewModel = ClassroomListViewModel( activity, fragment, @@ -144,6 +154,8 @@ class ClassroomListFragmentPresenter @Inject constructor( } ) + logAppOnboardedEvent() + return binding.root } @@ -151,6 +163,7 @@ class ClassroomListFragmentPresenter @Inject constructor( fun onTopicSummaryClicked(topicSummary: TopicSummary) { routeToTopicPlayStoryListener.routeToTopicPlayStory( internalProfileId, + topicSummary.classroomId, topicSummary.topicId, topicSummary.firstStoryId ) @@ -225,6 +238,45 @@ class ClassroomListFragmentPresenter @Inject constructor( } } } + + private fun logAppOnboardedEvent() { + val startupStateProvider = appStartupStateController.getAppStartupState() + val liveData = startupStateProvider.toLiveData() + liveData.observe( + activity, + object : Observer> { + override fun onChanged(startUpStateResult: AsyncResult?) { + when (startUpStateResult) { + null, is AsyncResult.Pending -> { + // Do nothing. + } + is AsyncResult.Success -> { + liveData.removeObserver(this) + + if (startUpStateResult.value.startupMode == + AppStartupState.StartupMode.USER_NOT_YET_ONBOARDED + ) { + analyticsController.logAppOnboardedEvent(profileId) + } + } + is AsyncResult.Failure -> { + oppiaLogger.e( + "ClassroomListFragment", + "Failed to retrieve app startup state" + ) + } + } + } + } + ) + } + + private fun logHomeActivityEvent() { + analyticsController.logImportantEvent( + oppiaLogger.createOpenHomeContext(), + profileId + ) + } } /** Adds a grid of items to a LazyListScope with specified arrangement and item content. */ diff --git a/app/src/main/java/org/oppia/android/app/completedstorylist/CompletedStoryItemViewModel.kt b/app/src/main/java/org/oppia/android/app/completedstorylist/CompletedStoryItemViewModel.kt index 355389d59b5..6e8507554c0 100644 --- a/app/src/main/java/org/oppia/android/app/completedstorylist/CompletedStoryItemViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/completedstorylist/CompletedStoryItemViewModel.kt @@ -31,13 +31,24 @@ class CompletedStoryItemViewModel( /** Called when user clicks on CompletedStoryItem. */ fun onCompletedStoryItemClicked() { - routeToTopicPlayStory(internalProfileId, completedStory.topicId, completedStory.storyId) + routeToTopicPlayStory( + internalProfileId, + completedStory.classroomId, + completedStory.topicId, + completedStory.storyId + ) } - override fun routeToTopicPlayStory(internalProfileId: Int, topicId: String, storyId: String) { + override fun routeToTopicPlayStory( + internalProfileId: Int, + classroomId: String, + topicId: String, + storyId: String + ) { val intent = intentFactoryShim.createTopicPlayStoryActivityIntent( activity.applicationContext, internalProfileId, + classroomId, topicId, storyId ) diff --git a/app/src/main/java/org/oppia/android/app/home/HomeActivity.kt b/app/src/main/java/org/oppia/android/app/home/HomeActivity.kt index 8d6f828abd0..34885717a33 100644 --- a/app/src/main/java/org/oppia/android/app/home/HomeActivity.kt +++ b/app/src/main/java/org/oppia/android/app/home/HomeActivity.kt @@ -67,8 +67,10 @@ class HomeActivity : homeActivityPresenter.handleOnRestart() } - override fun routeToTopic(internalProfileId: Int, topicId: String) { - startActivity(TopicActivity.createTopicActivityIntent(this, internalProfileId, topicId)) + override fun routeToTopic(internalProfileId: Int, classroomId: String, topicId: String) { + startActivity( + TopicActivity.createTopicActivityIntent(this, internalProfileId, classroomId, topicId) + ) } override fun onBackPressed() { @@ -87,11 +89,17 @@ class HomeActivity : dialogFragment.showNow(supportFragmentManager, TAG_SWITCH_PROFILE_DIALOG) } - override fun routeToTopicPlayStory(internalProfileId: Int, topicId: String, storyId: String) { + override fun routeToTopicPlayStory( + internalProfileId: Int, + classroomId: String, + topicId: String, + storyId: String + ) { startActivity( TopicActivity.createTopicPlayStoryActivityIntent( this, internalProfileId, + classroomId, topicId, storyId ) diff --git a/app/src/main/java/org/oppia/android/app/home/HomeFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/home/HomeFragmentPresenter.kt index 64d53c68009..b3ef5d04e3f 100644 --- a/app/src/main/java/org/oppia/android/app/home/HomeFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/home/HomeFragmentPresenter.kt @@ -197,6 +197,7 @@ class HomeFragmentPresenter @Inject constructor( fun onTopicSummaryClicked(topicSummary: TopicSummary) { routeToTopicPlayStoryListener.routeToTopicPlayStory( internalProfileId, + topicSummary.classroomId, topicSummary.topicId, topicSummary.firstStoryId ) diff --git a/app/src/main/java/org/oppia/android/app/home/RouteToExplorationListener.kt b/app/src/main/java/org/oppia/android/app/home/RouteToExplorationListener.kt index 3c8081a3152..9523938373d 100755 --- a/app/src/main/java/org/oppia/android/app/home/RouteToExplorationListener.kt +++ b/app/src/main/java/org/oppia/android/app/home/RouteToExplorationListener.kt @@ -7,6 +7,7 @@ import org.oppia.android.app.model.ProfileId interface RouteToExplorationListener { fun routeToExploration( profileId: ProfileId, + classroomId: String, topicId: String, storyId: String, explorationId: String, diff --git a/app/src/main/java/org/oppia/android/app/home/RouteToTopicListener.kt b/app/src/main/java/org/oppia/android/app/home/RouteToTopicListener.kt index 36c7a1a1d94..c00a7dc8389 100755 --- a/app/src/main/java/org/oppia/android/app/home/RouteToTopicListener.kt +++ b/app/src/main/java/org/oppia/android/app/home/RouteToTopicListener.kt @@ -2,5 +2,5 @@ package org.oppia.android.app.home /** Listener for when an activity should route to a topic. */ interface RouteToTopicListener { - fun routeToTopic(internalProfileId: Int, topicId: String) + fun routeToTopic(internalProfileId: Int, classroomId: String, topicId: String) } diff --git a/app/src/main/java/org/oppia/android/app/home/RouteToTopicPlayStoryListener.kt b/app/src/main/java/org/oppia/android/app/home/RouteToTopicPlayStoryListener.kt index aa7121f1c07..4d45619ede8 100755 --- a/app/src/main/java/org/oppia/android/app/home/RouteToTopicPlayStoryListener.kt +++ b/app/src/main/java/org/oppia/android/app/home/RouteToTopicPlayStoryListener.kt @@ -2,5 +2,10 @@ package org.oppia.android.app.home /** Listener for when an activity should route to a story-item in TopicPlay tab. */ interface RouteToTopicPlayStoryListener { - fun routeToTopicPlayStory(internalProfileId: Int, topicId: String, storyId: String) + fun routeToTopicPlayStory( + internalProfileId: Int, + classroomId: String, + topicId: String, + storyId: String + ) } diff --git a/app/src/main/java/org/oppia/android/app/home/promotedlist/PromotedStoryViewModel.kt b/app/src/main/java/org/oppia/android/app/home/promotedlist/PromotedStoryViewModel.kt index 6b300f6355d..dae19f0866e 100755 --- a/app/src/main/java/org/oppia/android/app/home/promotedlist/PromotedStoryViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/home/promotedlist/PromotedStoryViewModel.kt @@ -63,6 +63,7 @@ class PromotedStoryViewModel( fun clickOnStoryTile() { routeToTopicPlayStoryListener.routeToTopicPlayStory( internalProfileId, + promotedStory.classroomId, promotedStory.topicId, promotedStory.storyId ) diff --git a/app/src/main/java/org/oppia/android/app/home/recentlyplayed/RecentlyPlayedActivity.kt b/app/src/main/java/org/oppia/android/app/home/recentlyplayed/RecentlyPlayedActivity.kt index bf852642259..3450d510fbe 100644 --- a/app/src/main/java/org/oppia/android/app/home/recentlyplayed/RecentlyPlayedActivity.kt +++ b/app/src/main/java/org/oppia/android/app/home/recentlyplayed/RecentlyPlayedActivity.kt @@ -62,6 +62,7 @@ class RecentlyPlayedActivity : override fun routeToExploration( profileId: ProfileId, + classroomId: String, topicId: String, storyId: String, explorationId: String, @@ -72,6 +73,7 @@ class RecentlyPlayedActivity : ExplorationActivity.createExplorationActivityIntent( this, profileId, + classroomId, topicId, storyId, explorationId, @@ -83,6 +85,7 @@ class RecentlyPlayedActivity : override fun routeToResumeLesson( profileId: ProfileId, + classroomId: String, topicId: String, storyId: String, explorationId: String, @@ -93,6 +96,7 @@ class RecentlyPlayedActivity : ResumeLessonActivity.createResumeLessonActivityIntent( this, profileId, + classroomId, topicId, storyId, explorationId, diff --git a/app/src/main/java/org/oppia/android/app/home/recentlyplayed/RecentlyPlayedFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/home/recentlyplayed/RecentlyPlayedFragmentPresenter.kt index fcd8a8b3c7c..98b7617e0eb 100755 --- a/app/src/main/java/org/oppia/android/app/home/recentlyplayed/RecentlyPlayedFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/home/recentlyplayed/RecentlyPlayedFragmentPresenter.kt @@ -108,6 +108,7 @@ class RecentlyPlayedFragmentPresenter @Inject constructor( explorationCheckpointLiveData.removeObserver(this) routeToResumeLessonListener.routeToResumeLesson( profileId, + promotedStory.classroomId, promotedStory.topicId, promotedStory.storyId, promotedStory.explorationId, @@ -117,6 +118,7 @@ class RecentlyPlayedFragmentPresenter @Inject constructor( } else if (it is AsyncResult.Failure) { explorationCheckpointLiveData.removeObserver(this) playExploration( + promotedStory.classroomId, promotedStory.topicId, promotedStory.storyId, promotedStory.explorationId, @@ -128,6 +130,7 @@ class RecentlyPlayedFragmentPresenter @Inject constructor( ) } else { playExploration( + promotedStory.classroomId, promotedStory.topicId, promotedStory.storyId, promotedStory.explorationId, @@ -162,6 +165,7 @@ class RecentlyPlayedFragmentPresenter @Inject constructor( } private fun playExploration( + classroomId: String, topicId: String, storyId: String, explorationId: String, @@ -174,13 +178,13 @@ class RecentlyPlayedFragmentPresenter @Inject constructor( // cases, lessons played from this fragment are known to be in progress, and that progress // can't be resumed here (hence the restart). explorationDataController.restartExploration( - profileId.internalId, topicId, storyId, explorationId + profileId.internalId, classroomId, topicId, storyId, explorationId ) } else { // The only lessons that can't have their progress saved are those that were already // completed. explorationDataController.replayExploration( - profileId.internalId, topicId, storyId, explorationId + profileId.internalId, classroomId, topicId, storyId, explorationId ) } startPlayingProvider.toLiveData().observe(fragment) { result -> @@ -192,6 +196,7 @@ class RecentlyPlayedFragmentPresenter @Inject constructor( oppiaLogger.d("RecentlyPlayedFragment", "Successfully loaded exploration") routeToExplorationListener.routeToExploration( profileId, + classroomId, topicId, storyId, explorationId, diff --git a/app/src/main/java/org/oppia/android/app/ongoingtopiclist/OngoingTopicItemViewModel.kt b/app/src/main/java/org/oppia/android/app/ongoingtopiclist/OngoingTopicItemViewModel.kt index 2f8e6f9e84d..1b074ed6f1b 100644 --- a/app/src/main/java/org/oppia/android/app/ongoingtopiclist/OngoingTopicItemViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/ongoingtopiclist/OngoingTopicItemViewModel.kt @@ -27,7 +27,7 @@ class OngoingTopicItemViewModel( } fun onTopicItemClicked() { - routeToTopic(internalProfileId, topic.topicId) + routeToTopic(internalProfileId, topic.classroomId, topic.topicId) } fun computeStoryCountText(): String { @@ -36,10 +36,11 @@ class OngoingTopicItemViewModel( ) } - override fun routeToTopic(internalProfileId: Int, topicId: String) { + override fun routeToTopic(internalProfileId: Int, classroomId: String, topicId: String) { val intent = intentFactoryShim.createTopicActivityIntent( activity.applicationContext, internalProfileId, + classroomId, topicId ) activity.startActivity(intent) diff --git a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivity.kt b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivity.kt index 4e73ac992e3..b95785bf2d0 100755 --- a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivity.kt +++ b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivity.kt @@ -60,6 +60,7 @@ class ExplorationActivity : explorationActivityPresenter.handleOnCreate( this, params.profileId, + params.classroomId, params.topicId, params.storyId, params.explorationId, @@ -79,6 +80,7 @@ class ExplorationActivity : fun createExplorationActivityIntent( context: Context, profileId: ProfileId, + classroomId: String, topicId: String, storyId: String, explorationId: String, @@ -87,6 +89,7 @@ class ExplorationActivity : ): Intent { val params = ExplorationActivityParams.newBuilder().apply { this.profileId = profileId + this.classroomId = classroomId this.topicId = topicId this.storyId = storyId this.explorationId = explorationId diff --git a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivityPresenter.kt b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivityPresenter.kt index 76812ff7522..9d1c50ec2ea 100644 --- a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivityPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivityPresenter.kt @@ -70,6 +70,7 @@ class ExplorationActivityPresenter @Inject constructor( private lateinit var explorationToolbar: Toolbar private lateinit var explorationToolbarTitle: TextView private lateinit var profileId: ProfileId + private lateinit var classroomId: String private lateinit var topicId: String private lateinit var storyId: String private lateinit var explorationId: String @@ -85,6 +86,7 @@ class ExplorationActivityPresenter @Inject constructor( fun handleOnCreate( context: Context, profileId: ProfileId, + classroomId: String, topicId: String, storyId: String, explorationId: String, @@ -125,6 +127,7 @@ class ExplorationActivityPresenter @Inject constructor( } this.profileId = profileId + this.classroomId = classroomId this.topicId = topicId this.storyId = storyId this.explorationId = explorationId @@ -187,6 +190,7 @@ class ExplorationActivityPresenter @Inject constructor( R.id.exploration_fragment_placeholder, ExplorationFragment.newInstance( profileId, + classroomId, topicId, storyId, explorationId, @@ -386,7 +390,12 @@ class ExplorationActivityPresenter @Inject constructor( ExplorationActivityParams.ParentScreen.UNRECOGNIZED -> { // Default to the topic activity. activity.startActivity( - TopicActivity.createTopicActivityIntent(context, profileId.internalId, topicId) + TopicActivity.createTopicActivityIntent( + context, + profileId.internalId, + classroomId, + topicId + ) ) activity.finish() } diff --git a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragment.kt b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragment.kt index 8411ab49c46..fdffb73b32d 100755 --- a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragment.kt +++ b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragment.kt @@ -22,6 +22,7 @@ class ExplorationFragment : InjectableFragment() { /** Returns a new [ExplorationFragment] with the corresponding fragment parameters. */ fun newInstance( profileId: ProfileId, + classroomId: String, topicId: String, storyId: String, explorationId: String, @@ -29,6 +30,7 @@ class ExplorationFragment : InjectableFragment() { ): ExplorationFragment { val args = ExplorationFragmentArguments.newBuilder().apply { this.profileId = profileId + this.classroomId = classroomId this.topicId = topicId this.storyId = storyId this.explorationId = explorationId diff --git a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragmentPresenter.kt index a64fa466a65..151f2456f53 100755 --- a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragmentPresenter.kt @@ -58,7 +58,7 @@ class ExplorationFragmentPresenter @Inject constructor( StateFragment.newInstance( args.profileId.internalId, args.topicId, args.storyId, args.explorationId ) - logPracticeFragmentEvent(args.topicId, args.storyId, args.explorationId) + logPracticeFragmentEvent(args.classroomId, args.topicId, args.storyId, args.explorationId) if (getStateFragment() == null) { fragment.childFragmentManager.beginTransaction().add( R.id.state_fragment_placeholder, @@ -153,9 +153,16 @@ class ExplorationFragmentPresenter @Inject constructor( ) as StateFragment? } - private fun logPracticeFragmentEvent(topicId: String, storyId: String, explorationId: String) { + private fun logPracticeFragmentEvent( + classroomId: String, + topicId: String, + storyId: String, + explorationId: String + ) { analyticsController.logImportantEvent( - oppiaLogger.createOpenExplorationActivityContext(topicId, storyId, explorationId), + oppiaLogger.createOpenExplorationActivityContext( + classroomId, topicId, storyId, explorationId + ), ProfileId.newBuilder().apply { internalId = internalProfileId }.build() ) } diff --git a/app/src/main/java/org/oppia/android/app/player/state/testing/StateFragmentTestActivity.kt b/app/src/main/java/org/oppia/android/app/player/state/testing/StateFragmentTestActivity.kt index e8117516284..2fc44ad11f5 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/testing/StateFragmentTestActivity.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/testing/StateFragmentTestActivity.kt @@ -71,6 +71,7 @@ class StateFragmentTestActivity : fun createTestActivityIntent( context: Context, profileId: Int, + classroomId: String, topicId: String, storyId: String, explorationId: String, @@ -78,6 +79,7 @@ class StateFragmentTestActivity : ): Intent { val args = StateFragmentTestActivityParams.newBuilder().apply { this.internalProfileId = profileId + this.classroomId = classroomId this.topicId = topicId this.storyId = storyId this.explorationId = explorationId diff --git a/app/src/main/java/org/oppia/android/app/player/state/testing/StateFragmentTestActivityPresenter.kt b/app/src/main/java/org/oppia/android/app/player/state/testing/StateFragmentTestActivityPresenter.kt index 3c52bb6e777..6beed9aaf21 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/testing/StateFragmentTestActivityPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/testing/StateFragmentTestActivityPresenter.kt @@ -12,6 +12,7 @@ import org.oppia.android.app.player.exploration.TAG_HINTS_AND_SOLUTION_EXPLORATI import org.oppia.android.app.player.state.StateFragment import org.oppia.android.app.player.state.testing.StateFragmentTestActivity.Companion.STATE_FRAGMENT_TEST_ACTIVITY_PARAMS_KEY import org.oppia.android.databinding.StateFragmentTestActivityBinding +import org.oppia.android.domain.classroom.TEST_CLASSROOM_ID_0 import org.oppia.android.domain.exploration.ExplorationDataController import org.oppia.android.domain.oppialogger.OppiaLogger import org.oppia.android.domain.topic.TEST_EXPLORATION_ID_2 @@ -34,6 +35,7 @@ class StateFragmentTestActivityPresenter @Inject constructor( ) { private var profileId: Int = 1 + private lateinit var classroomId: String private lateinit var topicId: String private lateinit var storyId: String private lateinit var explorationId: String @@ -54,6 +56,7 @@ class StateFragmentTestActivityPresenter @Inject constructor( StateFragmentTestActivityParams.getDefaultInstance() ) profileId = args?.internalProfileId ?: 1 + classroomId = args?.classroomId ?: TEST_CLASSROOM_ID_0 topicId = args?.topicId ?: TEST_TOPIC_ID_0 storyId = @@ -63,7 +66,14 @@ class StateFragmentTestActivityPresenter @Inject constructor( ?: TEST_EXPLORATION_ID_2 shouldSavePartialProgress = args?.shouldSavePartialProgress ?: false activity.findViewById