From 0ef7a460ce1e41546f2ba1510ff12bbf88ad8177 Mon Sep 17 00:00:00 2001 From: RD Rama Devi <122200035+Rd4dev@users.noreply.github.com> Date: Wed, 28 Aug 2024 11:09:13 +0530 Subject: [PATCH 1/9] Fix part of #5343: Update Incorrect Link for the Oppia Android Code Coverage Wiki Page (#5511) ## Explanation Fixes Part of #5343 ### This PR includes - Updated the incorrect link reference in the #5483 for the Oppia Android Code Coverage Page - Current Broken link: https://github.com/oppia/oppia-android-workflow/wiki/Oppia-Android-Code-Coverage - Correct link to wiki: https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage - Fixed an incorrect SKIP status that triggered even after 'Unit Tests - Bazel' failure causing it to miscalculate the pb file list as zero thereby posting a skip comment. - Solved by adding an additional condition for the evaluation job. ``` if: ${{ !cancelled() && needs.check_unit_tests_completed.result == 'success'}} ``` - The code coverage comment got triggered even after assignment changes due to the presence of 'assigned' in the on `pull_request_target` triggered, fixed it by removing the 'assigned' trigger. - Utilized 'HtmlEscapers' to escape HTML characters from source code lines in CoverageReporter.kt as strings those already included html tags overlapped / conflicted with the report html templates. - Added limitations section to the 'Oppia-Android-Code-Coverage' wiki page - Filed all possible coverage gaps that exist right now based on the available pass cases. ## 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 --- .github/workflows/code_coverage.yml | 10 ++- .github/workflows/comment_coverage_report.yml | 4 +- .../scripts/coverage/reporter/BUILD.bazel | 1 + .../coverage/reporter/CoverageReporter.kt | 3 +- .../scripts/coverage/RunCoverageTest.kt | 86 ++++++++++++++++--- .../scripts/coverage/reporter/BUILD.bazel | 1 + .../coverage/reporter/CoverageReporterTest.kt | 8 +- wiki/Oppia-Android-Code-Coverage.md | 28 +++++- ...ing-tests-with-good-behavioral-coverage.md | 2 +- wiki/_Sidebar.md | 2 +- 10 files changed, 121 insertions(+), 24 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 992a7e86cb1..276f2841312 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -258,10 +258,10 @@ jobs: evaluate_code_coverage_reports: name: Evaluate Code Coverage Reports runs-on: ubuntu-20.04 - needs: code_coverage_run + needs: [ check_unit_tests_completed, code_coverage_run ] # The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations, # serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows. - if: ${{ !cancelled() }} + if: ${{ !cancelled() && needs.check_unit_tests_completed.result == 'success'}} env: CACHE_DIRECTORY: ~/.bazel_cache steps: @@ -311,12 +311,16 @@ jobs: # Reference: https://github.community/t/127354/7. check_coverage_results: name: Check Code Coverage Results - needs: [ compute_changed_files, code_coverage_run, evaluate_code_coverage_reports ] + needs: [ check_unit_tests_completed, compute_changed_files, code_coverage_run, evaluate_code_coverage_reports ] # The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations, # serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows. if: ${{ !cancelled() }} runs-on: ubuntu-20.04 steps: + - name: Check unit tests passed + if: ${{ needs.check_unit_tests_completed.result != 'success' }} + run: exit 1 + - name: Check coverages passed if: ${{ needs.compute_changed_files.outputs.can_skip_files != 'true' && needs.code_coverage_run.result != 'success' }} run: exit 1 diff --git a/.github/workflows/comment_coverage_report.yml b/.github/workflows/comment_coverage_report.yml index 9ecb4e2e95c..16c1ae0da63 100644 --- a/.github/workflows/comment_coverage_report.yml +++ b/.github/workflows/comment_coverage_report.yml @@ -3,11 +3,11 @@ name: Comment Coverage Report # Controls when the action will run. Triggers the workflow on pull request events -# (assigned, opened, synchronize, reopened) +# (opened, synchronize, reopened) on: pull_request_target: - types: [assigned, opened, synchronize, reopened] + types: [opened, synchronize, reopened] concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }} diff --git a/scripts/src/java/org/oppia/android/scripts/coverage/reporter/BUILD.bazel b/scripts/src/java/org/oppia/android/scripts/coverage/reporter/BUILD.bazel index 85b59087cc2..217bc961aa1 100644 --- a/scripts/src/java/org/oppia/android/scripts/coverage/reporter/BUILD.bazel +++ b/scripts/src/java/org/oppia/android/scripts/coverage/reporter/BUILD.bazel @@ -15,5 +15,6 @@ kt_jvm_library( "//scripts/src/java/org/oppia/android/scripts/common:bazel_client", "//scripts/src/java/org/oppia/android/scripts/proto:coverage_java_proto", "//scripts/src/java/org/oppia/android/scripts/proto:script_exemptions_java_proto", + "//third_party:com_google_guava_guava", ], ) diff --git a/scripts/src/java/org/oppia/android/scripts/coverage/reporter/CoverageReporter.kt b/scripts/src/java/org/oppia/android/scripts/coverage/reporter/CoverageReporter.kt index 31763392945..f447a58c256 100644 --- a/scripts/src/java/org/oppia/android/scripts/coverage/reporter/CoverageReporter.kt +++ b/scripts/src/java/org/oppia/android/scripts/coverage/reporter/CoverageReporter.kt @@ -1,5 +1,6 @@ package org.oppia.android.scripts.coverage.reporter +import com.google.common.html.HtmlEscapers import org.oppia.android.scripts.proto.Coverage import org.oppia.android.scripts.proto.CoverageReport import org.oppia.android.scripts.proto.CoverageReportContainer @@ -277,7 +278,7 @@ class CoverageReporter( """ ${lineNumber.toString().padStart(4, ' ')} - $line + ${HtmlEscapers.htmlEscaper().escape(line)} """.trimIndent() ) 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 7ef0183f00d..3d28a816c7d 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt @@ -317,7 +317,13 @@ class RunCoverageTest { val expectedResult = getExpectedHtmlText(kotlinFilePath) - assertThat(readHtmlReport(kotlinFilePath)).isEqualTo(expectedResult) + val unescapedHtmlReport = readHtmlReport(kotlinFilePath) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedHtmlReport).isEqualTo(expectedResult) } @Test @@ -342,7 +348,13 @@ class RunCoverageTest { val expectedResult = getExpectedHtmlText(sourceFilePath) - assertThat(readHtmlReport(sourceFilePath)).isEqualTo(expectedResult) + val unescapedHtmlReport = readHtmlReport(sourceFilePath) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedHtmlReport).isEqualTo(expectedResult) } @Test @@ -541,7 +553,13 @@ class RunCoverageTest { val expectedResult = getExpectedHtmlText(filePath) - assertThat(readHtmlReport(filePath)).isEqualTo(expectedResult) + val unescapedHtmlReport = readHtmlReport(filePath) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedHtmlReport).isEqualTo(expectedResult) } @Test @@ -1880,10 +1898,20 @@ class RunCoverageTest { ).execute() val expectedResult1 = getExpectedHtmlText(filePathList.get(0)) - assertThat(readHtmlReport(filePathList.get(0))).isEqualTo(expectedResult1) + val unescapedHtmlReport1 = readHtmlReport(filePathList.get(0)) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + assertThat(unescapedHtmlReport1).isEqualTo(expectedResult1) val expectedResult2 = getExpectedHtmlText(filePathList.get(1)) - assertThat(readHtmlReport(filePathList.get(1))).isEqualTo(expectedResult2) + val unescapedHtmlReport2 = readHtmlReport(filePathList.get(1)) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + assertThat(unescapedHtmlReport2).isEqualTo(expectedResult2) } @Test @@ -1910,7 +1938,13 @@ class RunCoverageTest { val expectedResult = getExpectedHtmlText(filePathList.get(0)) - assertThat(readHtmlReport(filePathList.get(0))).isEqualTo(expectedResult) + val unescapedHtmlReport = readHtmlReport(filePathList.get(0)) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedHtmlReport).isEqualTo(expectedResult) } @Test @@ -1937,7 +1971,13 @@ class RunCoverageTest { val expectedResult = getExpectedHtmlText(filePathList.get(0)) - assertThat(readHtmlReport(filePathList.get(0))).isEqualTo(expectedResult) + val unescapedHtmlReport = readHtmlReport(filePathList.get(0)) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedHtmlReport).isEqualTo(expectedResult) } @Test @@ -1964,7 +2004,13 @@ class RunCoverageTest { val expectedResult = getExpectedHtmlText(filePathList.get(0)) - assertThat(readHtmlReport(filePathList.get(0))).isEqualTo(expectedResult) + val unescapedHtmlReport = readHtmlReport(filePathList.get(0)) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedHtmlReport).isEqualTo(expectedResult) } @Test @@ -2009,7 +2055,13 @@ class RunCoverageTest { val expectedResult = getExpectedHtmlText(filePathList.get(0)) - assertThat(readHtmlReport(filePathList.get(0))).isEqualTo(expectedResult) + val unescapedHtmlReport = readHtmlReport(filePathList.get(0)) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedHtmlReport).isEqualTo(expectedResult) } @Test @@ -2036,7 +2088,13 @@ class RunCoverageTest { val expectedResult = getExpectedHtmlText(filePathList.get(0)) - assertThat(readHtmlReport(filePathList.get(0))).isEqualTo(expectedResult) + val unescapedHtmlReport = readHtmlReport(filePathList.get(0)) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedHtmlReport).isEqualTo(expectedResult) } @Test @@ -2262,7 +2320,13 @@ class RunCoverageTest { """.trimIndent() - assertThat(readHtmlReport(filePathList.get(0))).isEqualTo(expectedResult) + val unescapedHtmlReport = readHtmlReport(filePathList.get(0)) + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedHtmlReport).isEqualTo(expectedResult) } @Test diff --git a/scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/BUILD.bazel b/scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/BUILD.bazel index 6571e76119e..0747162663f 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/BUILD.bazel +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/BUILD.bazel @@ -13,6 +13,7 @@ kt_jvm_test( "//scripts/src/java/org/oppia/android/scripts/coverage/reporter:coverage_reporter_lib", "//scripts/src/java/org/oppia/android/scripts/proto:script_exemptions_java_proto", "//testing:assertion_helpers", + "//third_party:com_google_guava_guava", "//third_party:com_google_truth_truth", "//third_party:org_jetbrains_kotlin_kotlin-test-junit", ], diff --git a/scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/CoverageReporterTest.kt b/scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/CoverageReporterTest.kt index b0af56a1115..ae06e6a693e 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/CoverageReporterTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/reporter/CoverageReporterTest.kt @@ -697,7 +697,13 @@ class CoverageReporterTest { """.trimIndent() - assertThat(outputReportText).isEqualTo(expectedHtml) + val unescapedOutputReportText = outputReportText + .replace(""", "\"") + .replace("&", "&") + .replace("<", "<") + .replace(">", ">") + + assertThat(unescapedOutputReportText).isEqualTo(expectedHtml) } @Test diff --git a/wiki/Oppia-Android-Code-Coverage.md b/wiki/Oppia-Android-Code-Coverage.md index e5ed0479619..499b3591642 100644 --- a/wiki/Oppia-Android-Code-Coverage.md +++ b/wiki/Oppia-Android-Code-Coverage.md @@ -139,7 +139,7 @@ Coverage Analysis: **FAIL** :x:
| File | Coverage | Lines Hit | Status | Min Required | |------|:--------:|----------:|:------:|:------------:| -|
MathTokenizer.ktutility/src/main/java/org/oppia/android/util/math/MathTokenizer.kt
| 94.26% | 197 / 209 | :white_check_mark: | 70% | +|
Pass.ktutility/src/main/java/org/oppia/android/util/math/Pass.kt
| 94.26% | 197 / 209 | :white_check_mark: | 70% | ### Exempted coverage @@ -282,7 +282,7 @@ Certain files are exempt from coverage checks. These exemptions include: 1. **Test File Exemptions:** Files that are exempted from having corresponding test files are also exempted from coverage checks. Since no test files are available for these sources, coverage analysis cannot be performed, and these files are therefore skipped. -2. **Source File Incompatibility Exemptions:** Some files are currently incompatible with Bazel coverage execution ([see tracking issue #5481](https://github.com/oppia/oppia-android/issues/5481)) and are temporarily excluded from coverage checks. +2. **Source File Incompatibility Exemptions:** Some files are currently incompatible with Bazel coverage execution (see tracking issue [#5481](https://github.com/oppia/oppia-android/issues/5481)) and are temporarily excluded from coverage checks. You can find the complete list of exemptions in this file: [test_file_exemptions.textproto](https://github.com/oppia/oppia-android/blob/develop/scripts/assets/test_file_exemptions.textproto) @@ -311,7 +311,19 @@ bazel run //scripts:run_coverage -- : Your root directory. - : Files you want to generate coverage reports for. -For example, to analyze coverage for the file MathTokenizer.kt, use the relative path: +To get the relative path of a file: + +1. Navigate to the Project view on the left-hand side in Android Studio. +2. Locate the file to analyze Code Coverage for. +3. Right click the file and select Copy Path. To get the path relative to the root. + +Alternatively, the coverage report itself provides the relative paths. You can reveal this information by clicking on the drop-down that precedes the file name in the report. + +| File | Coverage | Lines Hit | Status | Min Required | +|------|:--------:|----------:|:------:|:------------:| +|
MathTokenizer.ktutility/src/main/java/org/oppia/android/util/math/MathTokenizer.kt
| 94.26% | 197 / 209 | :white_check_mark: | 70% | + +To analyze coverage for the file MathTokenizer.kt, use the relative path: ```sh bazel run //scripts:run_coverage -- $(pwd) utility/src/main/java/org/oppia/android/util/math/MathTokenizer.kt @@ -456,6 +468,14 @@ It’s essential to ensure that each source file is directly tested by its own c ## Limitations of the code coverage tool -1. **Incompatibility with Code Coverage Analysis:** Certain test targets in the Oppia-Android codebase fail to execute and collect coverage using the Bazel coverage command. The underlying issues are still being investigated ([see tracking issue #5481](https://github.com/oppia/oppia-android/issues/5481)), and these files are currently exempt from coverage checks. However, it's expected that all new test files should work without needing this exemption. +1. **Incompatibility with Code Coverage Analysis:** Certain test targets in the Oppia-Android codebase fail to execute and collect coverage using the Bazel coverage command. The underlying issues are still being investigated (see tracking issue [#5481](https://github.com/oppia/oppia-android/issues/5481)), and these files are currently exempt from coverage checks. However, it's expected that all new test files should work without needing this exemption. 2. **Function and Branch Coverage:** The Oppia-Android code coverage tool currently provides only line coverage data. It does not include information on function or branch coverage. + +3. **Kotlin inline functions:** With JaCoCo coverage gaps, Kotlin inline functions may be inaccurately reported as uncovered in coverage reports. (See tracking issue [#5501](https://github.com/oppia/oppia-android/issues/5501)) + +4. **Line and Partial Coverages:** The current line coverage analysis in Oppia Android is limited and may not accurately reflect the execution of complex or multi-branch code within a single line, reporting lines as fully covered even if only part of the logic within those lines is executed, leading to potentially misleading coverage data. (See tracking issue [#5503](https://github.com/oppia/oppia-android/issues/5503)) + +5. **Flow Interrupting Statements:** The coverage reports may inaccurately reflect the coverage of flow-interrupting statements (e.g., exitProcess(1), assertion failures, break). These lines may be marked as uncovered even when executed, due to JaCoCo's limitations in tracking code execution after abrupt control flow interruptions. (See tracking issue [#5506](https://github.com/oppia/oppia-android/issues/5506)) + +6. **Uncovered Last Curly Brace in Kotlin:** The last curly brace of some Kotlin functions may be reported as uncovered, even when the function is fully executed during tests. This issue requires further investigation to determine if it's due to incomplete test execution or dead code generated by the Kotlin compiler. (See tracking issue [#5523](https://github.com/oppia/oppia-android/issues/5523)) \ No newline at end of file diff --git a/wiki/Writing-tests-with-good-behavioral-coverage.md b/wiki/Writing-tests-with-good-behavioral-coverage.md index 7a0aa5caebc..db50cfa83a6 100644 --- a/wiki/Writing-tests-with-good-behavioral-coverage.md +++ b/wiki/Writing-tests-with-good-behavioral-coverage.md @@ -1017,7 +1017,7 @@ Note: For more information on how to utilize the code coverage analysis tool, pl ## Testing a Single Outcome in Multiple Ways -When testing a single outcome like a successful withdrawal, you can use multiple approaches to verify the if the balance is updated correctly. Here are different ways to ensure the single outcome of withdrawal was processed correctly, each following a distinct approach. +When testing a single outcome, such as a successful withdrawal, you can use multiple approaches to verify if the balance is updated correctly. Here are different ways to ensure the single outcome of withdrawal was processed correctly, each following a distinct approach. **a. To verify correctness of output:** diff --git a/wiki/_Sidebar.md b/wiki/_Sidebar.md index ea5a254f2c6..c1f9834bc49 100644 --- a/wiki/_Sidebar.md +++ b/wiki/_Sidebar.md @@ -25,7 +25,7 @@ * Testing * [Oppia Android Testing](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Testing) * [End to End Testing Guide](https://github.com/oppia/oppia-android/wiki/End-to-End-Testing-Guide) - * [Oppia Android Code Coverage](https://github.com/oppia/oppia-android-workflow/wiki/Oppia-Android-Code-Coverage) + * [Oppia Android Code Coverage](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage) * [Writing Tests with Good Behavioral Coverage](https://github.com/oppia/oppia-android/wiki/Writing-Tests-With-Good-Behavioral-Coverage) * [Developing Skills](https://github.com/oppia/oppia-android/wiki/Developing-skills) * [Frequent Errors and Solutions](https://github.com/oppia/oppia-android/wiki/Frequent-Errors-and-Solutions) From a1fbe0c98b512eec45bb94804b50095606a5e64b Mon Sep 17 00:00:00 2001 From: Vishwajith Shettigar <76042077+Vishwajith-Shettigar@users.noreply.github.com> Date: Fri, 30 Aug 2024 18:16:33 +0530 Subject: [PATCH 2/9] Fix #5395: Fixed concept card not closing when opened from hint (#5509) ## Explanation Fixes #5395. `(fragment.requireActivity() as? ConceptCardListener)?.dismissConceptCard()` in `ConceptCardFragmentPresenter` calls the `dismissConceptCard()` method in` ExplorationActivity` since `ExplorationActivity `implements `ConceptCardListener` and overrides the `dismissConceptCard()` method. This makes call `dismissAll()` in `ConceptCardFragment` through `StateFragment` there it passes its `childFragmentManager`. Even though it managed to call the `dismissAll()` method in `ConceptCardFragment`, but the wrong fragment manager was passed. We passed `StateFragment's childFragmentManager`, but `ConceptCardFragment `is nested within `HintAndSolutionFragment`, so it should be passed `HintAndSolutionFragment's childFragmentManager`. [concept_card.webm](https://github.com/user-attachments/assets/d1959296-79bf-4e4f-b2d9-04acff089c37) ## 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 --- .../HintsAndSolutionDialogFragment.kt | 9 ++ ...HintsAndSolutionDialogFragmentPresenter.kt | 5 ++ .../player/exploration/ExplorationActivity.kt | 4 +- .../ExplorationActivityPresenter.kt | 4 - .../player/exploration/ExplorationFragment.kt | 2 - .../ExplorationFragmentPresenter.kt | 2 - .../android/app/player/state/StateFragment.kt | 2 - .../player/state/StateFragmentPresenter.kt | 5 -- .../ConceptCardFragmentTestActivity.kt | 2 +- .../QuestionPlayerActivityPresenter.kt | 4 +- .../questionplayer/QuestionPlayerFragment.kt | 2 - .../QuestionPlayerFragmentPresenter.kt | 5 -- .../exploration/ExplorationActivityTest.kt | 89 +++++++++++++++++++ 13 files changed, 110 insertions(+), 25 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragment.kt b/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragment.kt index c36cafe9ecc..e81aca55b3f 100644 --- a/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragment.kt +++ b/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragment.kt @@ -14,6 +14,7 @@ import org.oppia.android.app.model.HintsAndSolutionDialogFragmentStateBundle import org.oppia.android.app.model.ProfileId import org.oppia.android.app.model.State import org.oppia.android.app.model.WrittenTranslationContext +import org.oppia.android.app.topic.conceptcard.ConceptCardFragment import org.oppia.android.util.extensions.getProto import org.oppia.android.util.extensions.putProto import javax.inject.Inject @@ -192,4 +193,12 @@ class HintsAndSolutionDialogFragment : isSolutionRevealed ) } + + /** + * Delegates the removal of all [ConceptCardFragment] instances + * to the [hintsAndSolutionDialogFragmentPresenter]. + */ + fun dismissConceptCard() { + hintsAndSolutionDialogFragmentPresenter.dismissConceptCard() + } } diff --git a/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragmentPresenter.kt index 16f9dec7b19..824c777ca78 100644 --- a/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragmentPresenter.kt @@ -331,4 +331,9 @@ class HintsAndSolutionDialogFragmentPresenter @Inject constructor( override fun onConceptCardLinkClicked(view: View, skillId: String) { ConceptCardFragment.bringToFrontOrCreateIfNew(skillId, profileId, fragment.childFragmentManager) } + + /** Removes all [ConceptCardFragment] in the given FragmentManager. */ + fun dismissConceptCard() { + ConceptCardFragment.dismissAll(fragment.childFragmentManager) + } } 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 b95785bf2d0..6912d639ed9 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 @@ -190,7 +190,9 @@ class ExplorationActivity : this.writtenTranslationContext = writtenTranslationContext } - override fun dismissConceptCard() = explorationActivityPresenter.dismissConceptCard() + override fun dismissConceptCard() { + getHintsAndSolution()?.dismissConceptCard() + } override fun requestVoiceOverIconSpotlight(numberOfLogins: Int) { explorationActivityPresenter.requestVoiceOverIconSpotlight(numberOfLogins) 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 9d1c50ec2ea..1c493c19bfa 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 @@ -334,10 +334,6 @@ class ExplorationActivityPresenter @Inject constructor( showDialogFragmentBasedOnCurrentCheckpointState() } - fun dismissConceptCard() { - getExplorationFragment()?.dismissConceptCard() - } - private fun updateToolbarTitle(explorationId: String) { subscribeToExploration( explorationDataController.getExplorationById(profileId, explorationId).toLiveData() 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 fdffb73b32d..6d4cb2a6330 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 @@ -84,7 +84,5 @@ class ExplorationFragment : InjectableFragment() { explorationFragmentPresenter.viewSolution() } - fun dismissConceptCard() = explorationFragmentPresenter.dismissConceptCard() - fun getExplorationCheckpointState() = explorationFragmentPresenter.getExplorationCheckpointState() } 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 151f2456f53..207f5bf9e7f 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 @@ -141,8 +141,6 @@ class ExplorationFragmentPresenter @Inject constructor( getStateFragment()?.viewSolution() } - fun dismissConceptCard() = getStateFragment()?.dismissConceptCard() - fun getExplorationCheckpointState() = getStateFragment()?.getExplorationCheckpointState() private fun getStateFragment(): StateFragment? { diff --git a/app/src/main/java/org/oppia/android/app/player/state/StateFragment.kt b/app/src/main/java/org/oppia/android/app/player/state/StateFragment.kt index 50f05d60082..df5e8a07dde 100755 --- a/app/src/main/java/org/oppia/android/app/player/state/StateFragment.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StateFragment.kt @@ -162,8 +162,6 @@ class StateFragment : stateFragmentPresenter.viewSolution() } - fun dismissConceptCard() = stateFragmentPresenter.dismissConceptCard() - fun getExplorationCheckpointState() = stateFragmentPresenter.getExplorationCheckpointState() override fun onSaveInstanceState(outState: Bundle) { diff --git a/app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt index 672595d81ef..0147fb7e82c 100755 --- a/app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt @@ -38,7 +38,6 @@ import org.oppia.android.app.player.state.listener.RouteToHintsAndSolutionListen import org.oppia.android.app.player.stopplaying.StopStatePlayingSessionWithSavedProgressListener import org.oppia.android.app.survey.SurveyWelcomeDialogFragment import org.oppia.android.app.survey.TAG_SURVEY_WELCOME_DIALOG -import org.oppia.android.app.topic.conceptcard.ConceptCardFragment import org.oppia.android.app.translation.AppLanguageResourceHandler import org.oppia.android.app.utility.SplitScreenManager import org.oppia.android.app.utility.lifecycle.LifecycleSafeTimerFactory @@ -428,10 +427,6 @@ class StateFragmentPresenter @Inject constructor( subscribeToAnswerOutcome(explorationProgressController.submitAnswer(answer).toLiveData()) } - fun dismissConceptCard() { - ConceptCardFragment.dismissAll(fragment.childFragmentManager) - } - private fun moveToNextState() { stateViewModel.setCanSubmitAnswer(canSubmitAnswer = false) explorationProgressController.moveToNextState().toLiveData().observe( diff --git a/app/src/main/java/org/oppia/android/app/testing/ConceptCardFragmentTestActivity.kt b/app/src/main/java/org/oppia/android/app/testing/ConceptCardFragmentTestActivity.kt index b87c9c8a431..c428093696f 100644 --- a/app/src/main/java/org/oppia/android/app/testing/ConceptCardFragmentTestActivity.kt +++ b/app/src/main/java/org/oppia/android/app/testing/ConceptCardFragmentTestActivity.kt @@ -29,7 +29,7 @@ class ConceptCardFragmentTestActivity : } override fun dismissConceptCard() { - getConceptCardFragment()?.dismiss() + ConceptCardFragment.dismissAll(supportFragmentManager) } private fun getConceptCardFragment(): ConceptCardFragment? { diff --git a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityPresenter.kt b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityPresenter.kt index 5c1f7484766..523ebf2cc4d 100644 --- a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityPresenter.kt @@ -274,7 +274,9 @@ class QuestionPlayerActivityPresenter @Inject constructor( getHintsAndSolutionDialogFragment()?.dismiss() } - fun dismissConceptCard() = getQuestionPlayerFragment()?.dismissConceptCard() + fun dismissConceptCard() { + getHintsAndSolutionDialogFragment()?.dismissConceptCard() + } private fun getHintsAndSolutionDialogFragment(): HintsAndSolutionDialogFragment? { return activity.supportFragmentManager.findFragmentByTag( diff --git a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragment.kt b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragment.kt index b5e67ec1318..c825f53e5cf 100644 --- a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragment.kt +++ b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragment.kt @@ -106,8 +106,6 @@ class QuestionPlayerFragment : questionPlayerFragmentPresenter.revealSolution() } - fun dismissConceptCard() = questionPlayerFragmentPresenter.dismissConceptCard() - companion object { /** Arguments key for [QuestionPlayerFragment]. */ diff --git a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragmentPresenter.kt index 7b4861580ab..6319e930125 100644 --- a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragmentPresenter.kt @@ -27,7 +27,6 @@ import org.oppia.android.app.player.state.StatePlayerRecyclerViewAssembler import org.oppia.android.app.player.state.listener.RouteToHintsAndSolutionListener import org.oppia.android.app.player.stopplaying.RestartPlayingSessionListener import org.oppia.android.app.player.stopplaying.StopStatePlayingSessionListener -import org.oppia.android.app.topic.conceptcard.ConceptCardFragment import org.oppia.android.app.utility.FontScaleConfigurationUtil import org.oppia.android.app.utility.SplitScreenManager import org.oppia.android.databinding.QuestionPlayerFragmentBinding @@ -124,10 +123,6 @@ class QuestionPlayerFragmentPresenter @Inject constructor( subscribeToHintSolution(questionAssessmentProgressController.submitSolutionIsRevealed()) } - fun dismissConceptCard() { - ConceptCardFragment.dismissAll(fragment.childFragmentManager) - } - private fun retrieveArguments(): QuestionPlayerFragmentArguments { return fragment.requireArguments().getProto( QuestionPlayerFragment.ARGUMENTS_KEY, QuestionPlayerFragmentArguments.getDefaultInstance() diff --git a/app/src/sharedTest/java/org/oppia/android/app/player/exploration/ExplorationActivityTest.kt b/app/src/sharedTest/java/org/oppia/android/app/player/exploration/ExplorationActivityTest.kt index ec53953d954..32d29315737 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/player/exploration/ExplorationActivityTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/player/exploration/ExplorationActivityTest.kt @@ -3,7 +3,9 @@ package org.oppia.android.app.player.exploration import android.app.Application import android.content.Context import android.content.Intent +import android.text.Spannable import android.text.TextUtils +import android.text.style.ClickableSpan import android.view.View import android.widget.TextView import androidx.appcompat.app.AppCompatActivity @@ -48,6 +50,7 @@ import org.hamcrest.CoreMatchers.containsString import org.hamcrest.Description import org.hamcrest.Matcher import org.hamcrest.Matchers.not +import org.hamcrest.TypeSafeMatcher import org.junit.After import org.junit.Before import org.junit.Ignore @@ -2559,6 +2562,92 @@ class ExplorationActivityTest { } } + @Test + @RunOn(TestPlatform.ROBOLECTRIC) // TODO(#3858): Enable for Espresso. + fun testExpActivity_openConceptCard_selectNavigationUp_conceptCardCloses() { + markAllSpotlightsSeen() + launch( + createExplorationActivityIntent( + internalProfileId, + TEST_CLASSROOM_ID_0, + TEST_TOPIC_ID_0, + TEST_STORY_ID_0, + TEST_EXPLORATION_ID_2, + shouldSavePartialProgress = false + ) + ).use { + explorationDataController.startPlayingNewExploration( + internalProfileId, + TEST_CLASSROOM_ID_0, + TEST_TOPIC_ID_0, + TEST_STORY_ID_0, + TEST_EXPLORATION_ID_2 + ) + testCoroutineDispatchers.runCurrent() + clickContinueButton() + // Submit two incorrect answers. + submitFractionAnswer(answerText = "1/3") + submitFractionAnswer(answerText = "1/4") + + // Reveal the hint. + openHintsAndSolutionsDialog() + pressRevealHintButton(hintPosition = 0) + + onView(withId(R.id.hints_and_solution_summary)) + .inRoot(isDialog()) + .perform(openClickableSpan("test_skill_id_1 concept card")) + + testCoroutineDispatchers.runCurrent() + + onView(withText("Concept Card")).inRoot(isDialog()).check(matches(isDisplayed())) + onView(withText("Another important skill")).inRoot(isDialog()).check(matches(isDisplayed())) + onView(withId(R.id.concept_card_toolbar)).check(matches(isDisplayed())) + + onView(withContentDescription(R.string.navigate_up)).perform(click()) + + testCoroutineDispatchers.runCurrent() + onView(withId(R.id.concept_card_toolbar)).check(doesNotExist()) + } + explorationDataController.stopPlayingExploration(isCompletion = false) + } + + private fun openClickableSpan(text: String): ViewAction { + return object : ViewAction { + override fun getDescription(): String = "openClickableSpan" + + override fun getConstraints(): Matcher = hasClickableSpanWithText(text) + + override fun perform(uiController: UiController?, view: View?) { + // The view shouldn't be null if the constraints are being met. + (view as? TextView)?.getClickableSpans()?.findMatchingTextOrNull(text)?.onClick(view) + } + } + } + + private fun List>.findMatchingTextOrNull(text: String) = + find { text in it.first }?.second + + private fun TextView.getClickableSpans(): List> { + val viewText = text + return (viewText as Spannable).getSpans( + /* start= */ 0, /* end= */ text.length, ClickableSpan::class.java + ).map { + viewText.subSequence(viewText.getSpanStart(it), viewText.getSpanEnd(it)).toString() to it + } + } + + private fun hasClickableSpanWithText(text: String): Matcher { + return object : TypeSafeMatcher(TextView::class.java) { + override fun describeTo(description: Description?) { + description?.appendText("has ClickableSpan with text")?.appendValue(text) + } + + override fun matchesSafely(item: View?): Boolean { + return (item as? TextView)?.getClickableSpans()?.findMatchingTextOrNull(text) != null + } + } + } + private fun markSpotlightSeen(feature: Spotlight.FeatureCase) { val profileId = ProfileId.newBuilder().setInternalId(internalProfileId).build() spotlightStateController.markSpotlightViewed(profileId, feature) From 16082aac839fc3e0a4c245bbc16e4b5f1a1dfe57 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Thu, 5 Sep 2024 13:02:41 +0530 Subject: [PATCH 3/9] Fix #5485 Create means for verifying Fragment Arguments (#5522) ## Explanation Feature Request part1 #5485 Added test for 10 fragments arguments and saveInstanceState. ## 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/options/AppLanguageFragment.kt | 3 +- .../app/options/AudioLanguageFragment.kt | 3 +- .../MarkChaptersCompletedFragmentTest.kt | 67 +++++++++++++++++++ .../MarkStoriesCompletedFragmentTest.kt | 54 +++++++++++++++ .../MarkTopicsCompletedFragmentTest.kt | 52 ++++++++++++++ .../android/app/help/HelpFragmentTest.kt | 32 +++++++++ .../app/options/AppLanguageFragmentTest.kt | 46 +++++++++++++ .../app/options/AudioLanguageFragmentTest.kt | 62 +++++++++++++++-- .../app/player/audio/AudioFragmentTest.kt | 20 ++++++ .../profile/ProfileEditFragmentTest.kt | 34 ++++++++++ .../app/thirdparty/LicenseListFragmentTest.kt | 30 +++++++++ .../conceptcard/ConceptCardFragmentTest.kt | 35 ++++++++++ 12 files changed, 431 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/options/AppLanguageFragment.kt b/app/src/main/java/org/oppia/android/app/options/AppLanguageFragment.kt index b946c93555b..477b78c49fd 100644 --- a/app/src/main/java/org/oppia/android/app/options/AppLanguageFragment.kt +++ b/app/src/main/java/org/oppia/android/app/options/AppLanguageFragment.kt @@ -42,7 +42,8 @@ class AppLanguageFragment : InjectableFragment(), AppLanguageRadioButtonListener } } - private fun Bundle.retrieveLanguageFromArguments(): OppiaLanguage { + /** Returns the [OppiaLanguage] stored in the fragment's arguments. */ + fun Bundle.retrieveLanguageFromArguments(): OppiaLanguage { return getProto( FRAGMENT_ARGUMENTS_KEY, AppLanguageFragmentArguments.getDefaultInstance() ).oppiaLanguage diff --git a/app/src/main/java/org/oppia/android/app/options/AudioLanguageFragment.kt b/app/src/main/java/org/oppia/android/app/options/AudioLanguageFragment.kt index 71ea48ca09e..4cb067f8cc7 100644 --- a/app/src/main/java/org/oppia/android/app/options/AudioLanguageFragment.kt +++ b/app/src/main/java/org/oppia/android/app/options/AudioLanguageFragment.kt @@ -84,7 +84,8 @@ class AudioLanguageFragment : InjectableFragment(), AudioLanguageRadioButtonList } } - private fun Bundle.retrieveLanguageFromArguments(): AudioLanguage { + /** Returns the [AudioLanguage] stored in the fragment's arguments. */ + fun Bundle.retrieveLanguageFromArguments(): AudioLanguage { return getProto( FRAGMENT_ARGUMENTS_KEY, AudioLanguageFragmentArguments.getDefaultInstance() ).audioLanguage diff --git a/app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkChaptersCompletedFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkChaptersCompletedFragmentTest.kt index 8c79da699c3..2ce4a9323ab 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkChaptersCompletedFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkChaptersCompletedFragmentTest.kt @@ -40,8 +40,10 @@ import org.oppia.android.app.application.ApplicationInjectorProvider import org.oppia.android.app.application.ApplicationModule import org.oppia.android.app.application.ApplicationStartupListenerModule import org.oppia.android.app.application.testing.TestingBuildFlavorModule +import org.oppia.android.app.devoptions.markchapterscompleted.MarkChaptersCompletedFragment import org.oppia.android.app.devoptions.markchapterscompleted.testing.MarkChaptersCompletedTestActivity import org.oppia.android.app.model.ChapterPlayState +import org.oppia.android.app.model.MarkChaptersCompletedFragmentArguments import org.oppia.android.app.model.ProfileId import org.oppia.android.app.player.state.itemviewmodel.SplitScreenInteractionModule import org.oppia.android.app.recyclerview.RecyclerViewMatcher.Companion.atPositionOnView @@ -94,6 +96,7 @@ import org.oppia.android.testing.time.FakeOppiaClockModule import org.oppia.android.util.accessibility.AccessibilityTestModule import org.oppia.android.util.caching.AssetModule import org.oppia.android.util.caching.testing.CachingTestModule +import org.oppia.android.util.extensions.getProto import org.oppia.android.util.gcsresource.GcsResourceModule import org.oppia.android.util.locale.LocaleProdModule import org.oppia.android.util.logging.EventLoggingConfigurationModule @@ -901,6 +904,70 @@ class MarkChaptersCompletedFragmentTest { } } + @Test + fun testFragment_fragmentLoaded_verifyCorrectArgumentsPassed() { + launchMarkChaptersCompletedFragmentTestActivity( + internalProfileId, showConfirmationNotice = true + ).use { scenario -> + testCoroutineDispatchers.runCurrent() + scenario.onActivity { activity -> + + val fragment = activity.supportFragmentManager + .findFragmentById(R.id.mark_chapters_completed_container) as MarkChaptersCompletedFragment + val arguments = + checkNotNull(fragment.arguments) { + "Expected arguments to be passed to MarkChaptersCompletedFragment" + } + val args = arguments.getProto( + "MarkChaptersCompletedFragment.arguments", + MarkChaptersCompletedFragmentArguments.getDefaultInstance() + ) + val receivedInternalProfileId = args?.internalProfileId + val receivedShowConfirmationNotice = args?.showConfirmationNotice + + assertThat(receivedInternalProfileId).isEqualTo(internalProfileId) + assertThat(receivedShowConfirmationNotice).isEqualTo(true) + } + } + } + + @Test + fun testFragment_saveInstanceState_verifyCorrectStateRestored() { + launchMarkChaptersCompletedFragmentTestActivity( + internalProfileId, showConfirmationNotice = true + ).use { scenario -> + testCoroutineDispatchers.runCurrent() + onView(withId(R.id.mark_chapters_completed_all_check_box_container)).perform(click()) + var actualSelectedExplorationIds = ArrayList() + var actualSelectedExplorationTitles = ArrayList() + + scenario.onActivity { activity -> + var fragment = activity.supportFragmentManager + .findFragmentById(R.id.mark_chapters_completed_container) as MarkChaptersCompletedFragment + + actualSelectedExplorationIds = + fragment.markChaptersCompletedFragmentPresenter.serializableSelectedExplorationIds + actualSelectedExplorationTitles = + fragment.markChaptersCompletedFragmentPresenter.serializableSelectedExplorationTitles + } + + scenario.recreate() + + scenario.onActivity { activity -> + val newFragment = activity.supportFragmentManager + .findFragmentById(R.id.mark_chapters_completed_container) as MarkChaptersCompletedFragment + + val receivedSelectedExplorationIds = + newFragment.markChaptersCompletedFragmentPresenter.serializableSelectedExplorationIds + val receivedSelectedExplorationTitles = + newFragment.markChaptersCompletedFragmentPresenter.serializableSelectedExplorationTitles + + assertThat(receivedSelectedExplorationIds).isEqualTo(actualSelectedExplorationIds) + assertThat(receivedSelectedExplorationTitles).isEqualTo(actualSelectedExplorationTitles) + } + } + } + private fun launchMarkChaptersCompletedFragmentTestActivity( internalProfileId: Int, showConfirmationNotice: Boolean = false diff --git a/app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkStoriesCompletedFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkStoriesCompletedFragmentTest.kt index 19836a42087..f8ab0bbacef 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkStoriesCompletedFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkStoriesCompletedFragmentTest.kt @@ -37,6 +37,7 @@ import org.oppia.android.app.application.ApplicationInjectorProvider import org.oppia.android.app.application.ApplicationModule import org.oppia.android.app.application.ApplicationStartupListenerModule import org.oppia.android.app.application.testing.TestingBuildFlavorModule +import org.oppia.android.app.devoptions.markstoriescompleted.MarkStoriesCompletedFragment import org.oppia.android.app.devoptions.markstoriescompleted.testing.MarkStoriesCompletedTestActivity import org.oppia.android.app.model.ProfileId import org.oppia.android.app.player.state.itemviewmodel.SplitScreenInteractionModule @@ -99,6 +100,7 @@ import org.oppia.android.util.networking.NetworkConnectionUtilDebugModule import org.oppia.android.util.parser.html.HtmlParserEntityTypeModule import org.oppia.android.util.parser.image.GlideImageLoaderModule import org.oppia.android.util.parser.image.ImageParsingModule +import org.oppia.android.util.profile.CurrentUserProfileIdIntentDecorator.extractCurrentUserProfileId import org.robolectric.annotation.Config import org.robolectric.annotation.LooperMode import javax.inject.Inject @@ -469,6 +471,58 @@ class MarkStoriesCompletedFragmentTest { } } + @Test + fun testFragment_fragmentLoaded_verifyCorrectArgumentsPassed() { + launch( + createMarkStoriesCompletedTestActivityIntent(internalProfileId) + ).use { scenario -> + testCoroutineDispatchers.runCurrent() + scenario.onActivity { activity -> + + val fragment = activity.supportFragmentManager + .findFragmentById(R.id.mark_stories_completed_container) as MarkStoriesCompletedFragment + + val arguments = + checkNotNull(fragment.arguments) { + "Expected arguments to be passed to MarkStoriesCompletedFragment" + } + val profileId = arguments.extractCurrentUserProfileId() + val receivedInternalProfileId = profileId.internalId + + assertThat(receivedInternalProfileId).isEqualTo(internalProfileId) + } + } + } + + @Test + fun testFragment_saveInstanceState_verifyCorrectStateRestored() { + launch( + createMarkStoriesCompletedTestActivityIntent(internalProfileId) + ).use { scenario -> + testCoroutineDispatchers.runCurrent() + onView(withId(R.id.mark_stories_completed_all_check_box_container)).perform(click()) + var actualSelectedStoryIdList = ArrayList() + + scenario.onActivity { activity -> + var fragment = activity.supportFragmentManager + .findFragmentById(R.id.mark_stories_completed_container) as MarkStoriesCompletedFragment + actualSelectedStoryIdList = + fragment.markStoriesCompletedFragmentPresenter.selectedStoryIdList + } + + scenario.recreate() + + scenario.onActivity { activity -> + val newFragment = activity.supportFragmentManager + .findFragmentById(R.id.mark_stories_completed_container) as MarkStoriesCompletedFragment + val receivedSelectedStoryIdList = + newFragment.markStoriesCompletedFragmentPresenter.selectedStoryIdList + + assertThat(receivedSelectedStoryIdList).isEqualTo(actualSelectedStoryIdList) + } + } + } + private fun createMarkStoriesCompletedTestActivityIntent(internalProfileId: Int): Intent { return MarkStoriesCompletedTestActivity.createMarkStoriesCompletedTestIntent( context, internalProfileId diff --git a/app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkTopicsCompletedFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkTopicsCompletedFragmentTest.kt index dd17bbe74ba..91cc3ee71f2 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkTopicsCompletedFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkTopicsCompletedFragmentTest.kt @@ -37,6 +37,7 @@ import org.oppia.android.app.application.ApplicationInjectorProvider import org.oppia.android.app.application.ApplicationModule import org.oppia.android.app.application.ApplicationStartupListenerModule import org.oppia.android.app.application.testing.TestingBuildFlavorModule +import org.oppia.android.app.devoptions.marktopicscompleted.MarkTopicsCompletedFragment import org.oppia.android.app.devoptions.marktopicscompleted.testing.MarkTopicsCompletedTestActivity import org.oppia.android.app.model.ProfileId import org.oppia.android.app.player.state.itemviewmodel.SplitScreenInteractionModule @@ -99,6 +100,7 @@ import org.oppia.android.util.networking.NetworkConnectionUtilDebugModule import org.oppia.android.util.parser.html.HtmlParserEntityTypeModule import org.oppia.android.util.parser.image.GlideImageLoaderModule import org.oppia.android.util.parser.image.ImageParsingModule +import org.oppia.android.util.profile.CurrentUserProfileIdIntentDecorator.extractCurrentUserProfileId import org.robolectric.annotation.Config import org.robolectric.annotation.LooperMode import javax.inject.Inject @@ -450,6 +452,56 @@ class MarkTopicsCompletedFragmentTest { } } + @Test + fun testFragment_fragmentLoaded_verifyCorrectArgumentsPassed() { + launch( + createMarkTopicsCompletedTestActivityIntent(internalProfileId) + ).use { scenario -> + testCoroutineDispatchers.runCurrent() + scenario.onActivity { activity -> + + var fragment = activity.supportFragmentManager + .findFragmentById(R.id.mark_topics_completed_container) as MarkTopicsCompletedFragment + val arguments = + checkNotNull(fragment.arguments) { + "Expected arguments to be passed to MarkTopicsCompletedFragment" + } + val receivedProfileId = arguments.extractCurrentUserProfileId() + + assertThat(receivedProfileId).isEqualTo(profileId) + } + } + } + + @Test + fun testFragment_saveInstanceState_verifyCorrectStateRestored() { + launch( + createMarkTopicsCompletedTestActivityIntent(internalProfileId) + ).use { scenario -> + testCoroutineDispatchers.runCurrent() + onView(withId(R.id.mark_topics_completed_all_check_box_container)).perform(click()) + var actualSelectedTopicsList = ArrayList() + + scenario.onActivity { activity -> + var fragment = activity.supportFragmentManager + .findFragmentById(R.id.mark_topics_completed_container) as MarkTopicsCompletedFragment + actualSelectedTopicsList = + fragment.markTopicsCompletedFragmentPresenter.selectedTopicIdList + } + + scenario.recreate() + + scenario.onActivity { activity -> + val newFragment = activity.supportFragmentManager + .findFragmentById(R.id.mark_topics_completed_container) as MarkTopicsCompletedFragment + val restoredTopicIdList = + newFragment.markTopicsCompletedFragmentPresenter.selectedTopicIdList + + assertThat(restoredTopicIdList).isEqualTo(actualSelectedTopicsList) + } + } + } + private fun createMarkTopicsCompletedTestActivityIntent(internalProfileId: Int): Intent { return MarkTopicsCompletedTestActivity.createMarkTopicsCompletedTestIntent( context, internalProfileId diff --git a/app/src/sharedTest/java/org/oppia/android/app/help/HelpFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/help/HelpFragmentTest.kt index 508d72bdb2a..e378a684dd9 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/help/HelpFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/help/HelpFragmentTest.kt @@ -2,6 +2,7 @@ package org.oppia.android.app.help import android.app.Application import android.content.Intent +import android.widget.FrameLayout import androidx.appcompat.app.AppCompatActivity import androidx.drawerlayout.widget.DrawerLayout import androidx.recyclerview.widget.RecyclerView @@ -29,6 +30,7 @@ import androidx.test.espresso.matcher.ViewMatchers.withEffectiveVisibility import androidx.test.espresso.matcher.ViewMatchers.withId import androidx.test.espresso.matcher.ViewMatchers.withText import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat import dagger.Component import org.hamcrest.Matchers.equalTo import org.junit.After @@ -50,6 +52,7 @@ import org.oppia.android.app.devoptions.DeveloperOptionsModule import org.oppia.android.app.devoptions.DeveloperOptionsStarterModule import org.oppia.android.app.help.faq.FAQListActivity import org.oppia.android.app.help.thirdparty.ThirdPartyDependencyListActivity +import org.oppia.android.app.model.HelpFragmentArguments import org.oppia.android.app.model.PoliciesActivityParams import org.oppia.android.app.model.PolicyPage import org.oppia.android.app.model.ProfileId @@ -104,6 +107,7 @@ import org.oppia.android.testing.time.FakeOppiaClockModule import org.oppia.android.util.accessibility.AccessibilityTestModule import org.oppia.android.util.caching.AssetModule import org.oppia.android.util.caching.testing.CachingTestModule +import org.oppia.android.util.extensions.getProto import org.oppia.android.util.gcsresource.GcsResourceModule import org.oppia.android.util.locale.LocaleProdModule import org.oppia.android.util.logging.EventLoggingConfigurationModule @@ -1374,6 +1378,34 @@ class HelpFragmentTest { } } + @Test + fun testFragment_fragmentLoaded_verifyCorrectArgumentsPassed() { + launch( + createHelpActivityIntent( + internalProfileId = 0, + isFromNavigationDrawer = true + ) + ).use { scenario -> + testCoroutineDispatchers.runCurrent() + scenario.onActivity { activity -> + + var fragment = activity.supportFragmentManager + .findFragmentById(R.id.help_fragment_placeholder) as HelpFragment + val isMultipane = + activity.findViewById(R.id.multipane_options_container) != null + + val arguments = checkNotNull(fragment.arguments) { + "Expected arguments to be passed to HelpFragment" + } + val args = + arguments.getProto("HelpFragment.arguments", HelpFragmentArguments.getDefaultInstance()) + val receivedIsMultipane = args.isMultipane + + assertThat(receivedIsMultipane).isEqualTo(isMultipane) + } + } + } + private fun ActivityScenario.openNavigationDrawer() { onView(withContentDescription(R.string.drawer_open_content_description)) .check(matches(isCompletelyDisplayed())) diff --git a/app/src/sharedTest/java/org/oppia/android/app/options/AppLanguageFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/options/AppLanguageFragmentTest.kt index 62749ba3863..074985b485b 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/options/AppLanguageFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/options/AppLanguageFragmentTest.kt @@ -33,6 +33,7 @@ import org.oppia.android.app.application.testing.TestingBuildFlavorModule import org.oppia.android.app.devoptions.DeveloperOptionsModule import org.oppia.android.app.devoptions.DeveloperOptionsStarterModule import org.oppia.android.app.model.OppiaLanguage +import org.oppia.android.app.options.AppLanguageFragment.Companion.retrieveLanguageFromArguments import org.oppia.android.app.player.state.itemviewmodel.SplitScreenInteractionModule import org.oppia.android.app.recyclerview.RecyclerViewMatcher.Companion.atPositionOnView import org.oppia.android.app.shim.ViewBindingShimModule @@ -95,6 +96,7 @@ import org.oppia.android.util.networking.NetworkConnectionUtilDebugModule import org.oppia.android.util.parser.html.HtmlParserEntityTypeModule import org.oppia.android.util.parser.image.GlideImageLoaderModule import org.oppia.android.util.parser.image.ImageParsingModule +import org.oppia.android.util.profile.CurrentUserProfileIdIntentDecorator.extractCurrentUserProfileId import org.robolectric.annotation.Config import org.robolectric.annotation.LooperMode import javax.inject.Inject @@ -215,6 +217,50 @@ class AppLanguageFragmentTest { } } + @Test + fun testFragment_fragmentLoaded_verifyCorrectArgumentsPassed() { + launch(createAppLanguageActivityIntent(OppiaLanguage.ENGLISH)) + .use { scenario -> + testCoroutineDispatchers.runCurrent() + scenario.onActivity { activity -> + + val appLanguageFragment = activity.supportFragmentManager + .findFragmentById(R.id.app_language_fragment_container) as AppLanguageFragment + val recievedLanguage = appLanguageFragment.arguments?.retrieveLanguageFromArguments() + val receivedProfileId = + appLanguageFragment.arguments?.extractCurrentUserProfileId()?.internalId + + assertThat(recievedLanguage).isEqualTo(OppiaLanguage.ENGLISH) + assertThat(receivedProfileId).isEqualTo(internalProfileId) + } + } + } + + @Test + fun testFragment_saveInstanceState_verifyCorrectStateRestored() { + launch(createAppLanguageActivityIntent(OppiaLanguage.ENGLISH)) + .use { scenario -> + testCoroutineDispatchers.runCurrent() + + scenario.onActivity { activity -> + var appLanguageFragment = activity.supportFragmentManager + .findFragmentById(R.id.app_language_fragment_container) as AppLanguageFragment + appLanguageFragment.appLanguageFragmentPresenter.onLanguageSelected(OppiaLanguage.ARABIC) + } + + scenario.recreate() + + scenario.onActivity { activity -> + val newAppLanguageFragment = activity.supportFragmentManager + .findFragmentById(R.id.app_language_fragment_container) as AppLanguageFragment + val restoredLanguage = + newAppLanguageFragment.appLanguageFragmentPresenter.getLanguageSelected() + + assertThat(restoredLanguage).isEqualTo(OppiaLanguage.ARABIC) + } + } + } + private fun verifyKiswahiliIsSelected(appLanguageActivity: AppLanguageActivity?) { checkSelectedLanguage(index = KISWAHILI_BUTTON_INDEX, expectedLanguageName = "Kiswahili") assertThat(appLanguageActivity?.appLanguageActivityPresenter?.getLanguageSelected()?.name) diff --git a/app/src/sharedTest/java/org/oppia/android/app/options/AudioLanguageFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/options/AudioLanguageFragmentTest.kt index 7ad7462af04..b8aed771aaa 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/options/AudioLanguageFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/options/AudioLanguageFragmentTest.kt @@ -39,6 +39,7 @@ import org.oppia.android.app.model.AudioLanguage import org.oppia.android.app.model.AudioLanguage.BRAZILIAN_PORTUGUESE_LANGUAGE import org.oppia.android.app.model.AudioLanguage.ENGLISH_AUDIO_LANGUAGE import org.oppia.android.app.model.AudioLanguage.NIGERIAN_PIDGIN_LANGUAGE +import org.oppia.android.app.options.AudioLanguageFragment.Companion.retrieveLanguageFromArguments import org.oppia.android.app.player.state.itemviewmodel.SplitScreenInteractionModule import org.oppia.android.app.recyclerview.RecyclerViewMatcher.Companion.atPositionOnView import org.oppia.android.app.shim.ViewBindingShimModule @@ -117,12 +118,17 @@ class AudioLanguageFragmentTest { private const val NIGERIAN_PIDGIN_BUTTON_INDEX = 4 } - @get:Rule val initializeDefaultLocaleRule = InitializeDefaultLocaleRule() - @get:Rule val oppiaTestRule = OppiaTestRule() + @get:Rule + val initializeDefaultLocaleRule = InitializeDefaultLocaleRule() + @get:Rule + val oppiaTestRule = OppiaTestRule() - @Inject lateinit var context: Context - @Inject lateinit var profileTestHelper: ProfileTestHelper - @Inject lateinit var testCoroutineDispatchers: TestCoroutineDispatchers + @Inject + lateinit var context: Context + @Inject + lateinit var profileTestHelper: ProfileTestHelper + @Inject + lateinit var testCoroutineDispatchers: TestCoroutineDispatchers @After fun tearDown() { @@ -351,6 +357,52 @@ class AudioLanguageFragmentTest { } } + @Test + fun testFragment_fragmentLoaded_verifyCorrectArgumentsPassed() { + initializeTestApplicationComponent(enableOnboardingFlowV2 = true) + launch( + createDefaultAudioActivityIntent(ENGLISH_AUDIO_LANGUAGE) + ).use { scenario -> + testCoroutineDispatchers.runCurrent() + scenario.onActivity { activity -> + + val fragment = activity.supportFragmentManager + .findFragmentById(R.id.audio_language_fragment_container) as AudioLanguageFragment + val receivedAudioLanguage = fragment.arguments?.retrieveLanguageFromArguments() + + assertThat(ENGLISH_AUDIO_LANGUAGE).isEqualTo(receivedAudioLanguage) + } + } + } + + @Test + fun testFragment_saveInstanceState_verifyCorrectStateRestored() { + initializeTestApplicationComponent(enableOnboardingFlowV2 = true) + launch( + createDefaultAudioActivityIntent(ENGLISH_AUDIO_LANGUAGE) + ).use { scenario -> + testCoroutineDispatchers.runCurrent() + var language: AudioLanguage? = null + + scenario.onActivity { activity -> + var fragment = activity.supportFragmentManager + .findFragmentById(R.id.audio_language_fragment_container) as AudioLanguageFragment + language = fragment.audioLanguageFragmentPresenterV1.getLanguageSelected() + } + + scenario.recreate() + + scenario.onActivity { activity -> + val newfragment = activity.supportFragmentManager + .findFragmentById(R.id.audio_language_fragment_container) as AudioLanguageFragment + val restoredAudioLanguage = + newfragment.audioLanguageFragmentPresenterV1.getLanguageSelected() + + assertThat(restoredAudioLanguage).isEqualTo(language) + } + } + } + private fun launchActivityWithLanguage( audioLanguage: AudioLanguage ): ActivityScenario { diff --git a/app/src/sharedTest/java/org/oppia/android/app/player/audio/AudioFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/player/audio/AudioFragmentTest.kt index ed37473683f..382327eb863 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/player/audio/AudioFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/player/audio/AudioFragmentTest.kt @@ -23,6 +23,7 @@ import androidx.test.espresso.matcher.ViewMatchers.withContentDescription import androidx.test.espresso.matcher.ViewMatchers.withId import androidx.test.espresso.matcher.ViewMatchers.withText import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat import dagger.Component import org.hamcrest.Description import org.hamcrest.Matcher @@ -110,6 +111,7 @@ import org.oppia.android.util.networking.NetworkConnectionUtilDebugModule import org.oppia.android.util.parser.html.HtmlParserEntityTypeModule import org.oppia.android.util.parser.image.GlideImageLoaderModule import org.oppia.android.util.parser.image.ImageParsingModule +import org.oppia.android.util.profile.CurrentUserProfileIdIntentDecorator.extractCurrentUserProfileId import org.robolectric.annotation.Config import org.robolectric.annotation.LooperMode import javax.inject.Inject @@ -343,6 +345,24 @@ class AudioFragmentTest { } } + @Test + fun testFragment_fragmentLoaded_verifyCorrectArgumentsPassed() { + addMediaInfo() + launch( + createAudioFragmentTestIntent(internalProfileId) + ).use { scenario -> + testCoroutineDispatchers.runCurrent() + scenario.onActivity { activity -> + + val audioFragment = activity.supportFragmentManager + .findFragmentById(R.id.audio_fragment_placeholder) as AudioFragment + val receivedProfileId = audioFragment.arguments?.extractCurrentUserProfileId() + + assertThat(receivedProfileId).isEqualTo(profileId) + } + } + } + private fun withSeekBarPosition(position: Int) = object : TypeSafeMatcher() { override fun describeTo(description: Description) { description.appendText("SeekBar with progress same as $position") diff --git a/app/src/sharedTest/java/org/oppia/android/app/settings/profile/ProfileEditFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/settings/profile/ProfileEditFragmentTest.kt index 37992371629..811072493a6 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/settings/profile/ProfileEditFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/settings/profile/ProfileEditFragmentTest.kt @@ -45,6 +45,8 @@ import org.oppia.android.app.devoptions.DeveloperOptionsStarterModule import org.oppia.android.app.devoptions.markchapterscompleted.MarkChaptersCompletedActivity import org.oppia.android.app.devoptions.markchapterscompleted.MarkChaptersCompletedActivity.Companion.MARK_CHAPTERS_COMPLETED_ACTIVITY_PARAMS import org.oppia.android.app.model.MarkChaptersCompletedActivityParams +import org.oppia.android.app.model.ProfileEditActivityParams +import org.oppia.android.app.model.ProfileEditFragmentArguments import org.oppia.android.app.model.ProfileId import org.oppia.android.app.player.state.itemviewmodel.SplitScreenInteractionModule import org.oppia.android.app.shim.ViewBindingShimModule @@ -100,6 +102,8 @@ import org.oppia.android.util.accessibility.AccessibilityTestModule import org.oppia.android.util.caching.AssetModule import org.oppia.android.util.caching.testing.CachingTestModule import org.oppia.android.util.data.DataProviders.Companion.toLiveData +import org.oppia.android.util.extensions.getProto +import org.oppia.android.util.extensions.getProtoExtra import org.oppia.android.util.gcsresource.GcsResourceModule import org.oppia.android.util.locale.LocaleProdModule import org.oppia.android.util.logging.EventLoggingConfigurationModule @@ -473,6 +477,36 @@ class ProfileEditFragmentTest { assertThat(profile.allowInLessonQuickLanguageSwitching).isTrue() } + @Test + fun testFragment_fragmentLoaded_verifyCorrectArgumentsPassed() { + launchFragmentTestActivity(internalProfileId = 1).use { scenario -> + scenario.onActivity { activity -> + + val activityArgs = activity.intent.getProtoExtra( + ProfileEditActivity.PROFILE_EDIT_ACTIVITY_PARAMS_KEY, + ProfileEditActivityParams.getDefaultInstance() + ) + val isMultipane = activityArgs?.isMultipane ?: false + + val fragment = activity.supportFragmentManager + .findFragmentById(R.id.profile_edit_fragment_placeholder) as ProfileEditFragment + + val arguments = checkNotNull(fragment.arguments) { + "Expected variables to be passed to ProfileEditFragment" + } + val args = arguments.getProto( + ProfileEditFragment.PROFILE_EDIT_FRAGMENT_ARGUMENTS_KEY, + ProfileEditFragmentArguments.getDefaultInstance() + ) + val receivedInternalProfileId = args.internalProfileId + val receivedIsMultipane = args.isMultipane + + assertThat(receivedInternalProfileId).isEqualTo(1) + assertThat(receivedIsMultipane).isEqualTo(isMultipane) + } + } + } + private fun launchFragmentTestActivity(internalProfileId: Int) = launch( createProfileEditFragmentTestActivity(context, internalProfileId) diff --git a/app/src/sharedTest/java/org/oppia/android/app/thirdparty/LicenseListFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/thirdparty/LicenseListFragmentTest.kt index 1e2bc4791a7..5e6a627b086 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/thirdparty/LicenseListFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/thirdparty/LicenseListFragmentTest.kt @@ -19,6 +19,7 @@ import androidx.test.espresso.matcher.ViewMatchers.isRoot import androidx.test.espresso.matcher.ViewMatchers.withId import androidx.test.espresso.matcher.ViewMatchers.withText import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat import dagger.Component import org.hamcrest.Matchers.allOf import org.junit.After @@ -39,7 +40,9 @@ import org.oppia.android.app.application.testing.TestingBuildFlavorModule import org.oppia.android.app.devoptions.DeveloperOptionsModule import org.oppia.android.app.devoptions.DeveloperOptionsStarterModule import org.oppia.android.app.help.thirdparty.LicenseListActivity +import org.oppia.android.app.help.thirdparty.LicenseListFragment import org.oppia.android.app.help.thirdparty.LicenseTextViewerActivity +import org.oppia.android.app.model.LicenseListFragmentArguments import org.oppia.android.app.player.state.itemviewmodel.SplitScreenInteractionModule import org.oppia.android.app.recyclerview.RecyclerViewMatcher.Companion.atPosition import org.oppia.android.app.shim.ViewBindingShimModule @@ -87,6 +90,7 @@ import org.oppia.android.testing.time.FakeOppiaClockModule import org.oppia.android.util.accessibility.AccessibilityTestModule import org.oppia.android.util.caching.AssetModule import org.oppia.android.util.caching.testing.CachingTestModule +import org.oppia.android.util.extensions.getProto import org.oppia.android.util.gcsresource.GcsResourceModule import org.oppia.android.util.locale.LocaleProdModule import org.oppia.android.util.logging.EventLoggingConfigurationModule @@ -340,6 +344,32 @@ class LicenseListFragmentTest { } } + @Test + fun testFragment_fragmentLoaded_verifyCorrectArgumentsPassed() { + launch(createLicenseListActivity(2)).use { scenario -> + + testCoroutineDispatchers.runCurrent() + scenario.onActivity { activity -> + + var fragment = activity.supportFragmentManager + .findFragmentById(R.id.license_list_fragment_placeholder) as LicenseListFragment + + val arguments = checkNotNull(fragment.arguments) { + "Expected arguments to be passed to LicenseListFragment" + } + val args = arguments.getProto( + "LicenseListFragment.arguments", + LicenseListFragmentArguments.getDefaultInstance() + ) + val receivedDependencyIndex = args.dependencyIndex + val receivedIsMultipane = args.isMultipane + + assertThat(receivedDependencyIndex).isEqualTo(2) + assertThat(receivedIsMultipane).isEqualTo(false) + } + } + } + private fun createLicenseListActivity(dependencyIndex: Int): Intent { return LicenseListActivity.createLicenseListActivityIntent( ApplicationProvider.getApplicationContext(), diff --git a/app/src/sharedTest/java/org/oppia/android/app/topic/conceptcard/ConceptCardFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/topic/conceptcard/ConceptCardFragmentTest.kt index c8ec2292371..ac21622d80e 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/topic/conceptcard/ConceptCardFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/topic/conceptcard/ConceptCardFragmentTest.kt @@ -56,6 +56,7 @@ import org.oppia.android.app.application.testing.TestingBuildFlavorModule import org.oppia.android.app.devoptions.DeveloperOptionsModule import org.oppia.android.app.devoptions.DeveloperOptionsStarterModule import org.oppia.android.app.fragment.InjectableDialogFragment +import org.oppia.android.app.model.ConceptCardFragmentArguments import org.oppia.android.app.model.OppiaLanguage import org.oppia.android.app.model.ProfileId import org.oppia.android.app.model.WrittenTranslationLanguageSelection @@ -116,6 +117,7 @@ import org.oppia.android.util.accessibility.AccessibilityTestModule import org.oppia.android.util.caching.AssetModule import org.oppia.android.util.caching.LoadImagesFromAssets import org.oppia.android.util.caching.LoadLessonProtosFromAssets +import org.oppia.android.util.extensions.getProto import org.oppia.android.util.gcsresource.GcsResourceModule import org.oppia.android.util.locale.LocaleProdModule import org.oppia.android.util.logging.EventLoggingConfigurationModule @@ -127,6 +129,7 @@ import org.oppia.android.util.networking.NetworkConnectionUtilDebugModule import org.oppia.android.util.parser.html.HtmlParserEntityTypeModule import org.oppia.android.util.parser.image.GlideImageLoaderModule import org.oppia.android.util.parser.image.ImageParsingModule +import org.oppia.android.util.profile.CurrentUserProfileIdIntentDecorator.extractCurrentUserProfileId import org.robolectric.annotation.Config import org.robolectric.annotation.LooperMode import javax.inject.Inject @@ -492,6 +495,38 @@ class ConceptCardFragmentTest { } } + @Test + fun testFragment_fragmentLoaded_verifyCorrectArgumentsPassed() { + launchTestActivity().use { scenario -> + scenario.onActivity { activity -> + + ConceptCardFragment.bringToFrontOrCreateIfNew( + TEST_SKILL_ID_0, + profileId, + activity.supportFragmentManager + ) + val fragmentSkill0 = + activity.supportFragmentManager.fragments.filterIsInstance().single() + + val arguments = checkNotNull(fragmentSkill0.arguments) { + "Expected arguments to be passed to ConceptCardFragment" + } + val args = arguments.getProto( + ConceptCardFragment.CONCEPT_CARD_FRAGMENT_ARGUMENTS_KEY, + ConceptCardFragmentArguments.getDefaultInstance() + ) + val skillId = + checkNotNull(args.skillId) { + "Expected skillId to be passed to ConceptCardFragment" + } + val receivedProfileId = arguments.extractCurrentUserProfileId() + + assertThat(skillId).isEqualTo(TEST_SKILL_ID_0) + assertThat(receivedProfileId).isEqualTo(profileId) + } + } + } + private fun launchTestActivity(): ActivityScenario { val scenario = ActivityScenario.launch( ConceptCardFragmentTestActivity.createIntent(context, profileId) From 4730edb0263135e88dab08ab0f6dc724a1585a67 Mon Sep 17 00:00:00 2001 From: RD Rama Devi <122200035+Rd4dev@users.noreply.github.com> Date: Thu, 5 Sep 2024 22:52:56 +0530 Subject: [PATCH 4/9] Fix #1730 : Prevent binary files from being checked in using pre-commit hook (#5525) ## Explanation Fixes #1730 ### The PR includes - A pre-commit hook that identifies binary files in both - staged changes (for local checks before committing) - committed files (for CI checks after pushing to a PR) - If binary files are found, the commit/check is blocked, and the offending files are listed for removal. - The pre-commit hook is integrated via a setup script. - The same script is utilized for the CI pipeline. - The 'Pass' statement is only included in the CI checks to keep the local commit process cleaner. ### Local block as pre-commit hook when a binary file is detected ![image](https://github.com/user-attachments/assets/b953b2d7-55ec-46e6-a0b9-00967200509c) ### CI re-check if the PR includes a binary - [ Fail - [stack trace](https://github.com/Rd4dev/oppia-android/actions/runs/10715972847/job/29712474511#step:7:15) ] [ Pass - [stack trace](https://github.com/Rd4dev/oppia-android/actions/runs/10716040065/job/29712695824?pr=11#step:7:10) ] ![image](https://github.com/user-attachments/assets/64352473-86cb-49e0-b888-d3247286e9e5) ## 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)). --- .github/workflows/static_checks.yml | 11 +++++++++ scripts/pre-commit.sh | 35 +++++++++++++++++++++++++++++ scripts/setup.sh | 3 +++ 3 files changed, 49 insertions(+) create mode 100644 scripts/pre-commit.sh diff --git a/.github/workflows/static_checks.yml b/.github/workflows/static_checks.yml index 15dd3e28e76..ae04da9c0f4 100644 --- a/.github/workflows/static_checks.yml +++ b/.github/workflows/static_checks.yml @@ -107,6 +107,8 @@ jobs: CACHE_DIRECTORY: ~/.bazel_cache steps: - uses: actions/checkout@v2 + with: + fetch-depth: 0 - name: Set up Bazel uses: abhinavsingh/setup-bazel@v3 @@ -205,6 +207,15 @@ jobs: run: | bazel run //scripts:string_resource_validation_check -- $(pwd) + - name: Binary files check + # The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations, + # serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows. + if: ${{ !cancelled() }} + run: | + bash /home/runner/work/oppia-android/oppia-android/scripts/pre-commit.sh + echo "No binary files found in commit" + echo "BINARY FILES CHECK PASSED" + # Note that caching is intentionally not enabled for this check since licenses should always be # verified without any potential influence from earlier builds (i.e. always from a clean build to # ensure the results exactly match the current state of the repository). diff --git a/scripts/pre-commit.sh b/scripts/pre-commit.sh new file mode 100644 index 00000000000..26061ef5c09 --- /dev/null +++ b/scripts/pre-commit.sh @@ -0,0 +1,35 @@ +#!/bin/bash + +# Pre-commit hook to check for binary files. + +# Find the common ancestor between develop and the current branch +base_commit=$(git merge-base 'origin/develop' HEAD) + +# Get the list of staged changes (files ready to be committed) +staged_files=$(git diff --cached --name-only) + +# Get the list of changed files compared to the base commit +changed_files=$(git diff --name-only "$base_commit" HEAD) + +# Combine both lists of files, ensuring no duplicates +all_files=$(echo -e "$staged_files\n$changed_files" | sort -u) + +function checkForBinaries() { + binaryFilesCount=0 + + # Iterate over all files (both staged and changed) + for file in $all_files; do + if [ -f "$file" ] && file --mime "$file" | grep -q 'binary'; then + ((binaryFilesCount++)) + printf "\n\033[33m%s\033[0m" "$file" + fi + done + + if [[ "${binaryFilesCount}" -gt 0 ]]; then + printf "\n\nPlease remove the %d detected binary file(s)." "$binaryFilesCount" + printf "\nBINARY FILES CHECK FAILED" + exit 1 + fi +} + +checkForBinaries diff --git a/scripts/setup.sh b/scripts/setup.sh index 8c3ef595e1d..9f822095f32 100644 --- a/scripts/setup.sh +++ b/scripts/setup.sh @@ -13,6 +13,9 @@ # Move file from script folder to .git/hooks folder cp scripts/pre-push.sh .git/hooks/pre-push +# Copy the pre-commit hook from script to .git/hooks folder +cp scripts/pre-commit.sh .git/hooks/pre-commit + # Create a folder where all the set up files will be downloaded mkdir -p ../oppia-android-tools cd ../oppia-android-tools From 99935529ce0e27ed20bdcba6aba325c4108d7369 Mon Sep 17 00:00:00 2001 From: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com> Date: Mon, 9 Sep 2024 22:34:04 +0300 Subject: [PATCH 5/9] Add test for arabic inTextInputLayoutBindingAdaptersTest --- .../databinding/TextInputLayoutBindingAdaptersTest.kt | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/app/src/sharedTest/java/org/oppia/android/app/databinding/TextInputLayoutBindingAdaptersTest.kt b/app/src/sharedTest/java/org/oppia/android/app/databinding/TextInputLayoutBindingAdaptersTest.kt index 896f3db7104..38b1e28773e 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/databinding/TextInputLayoutBindingAdaptersTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/databinding/TextInputLayoutBindingAdaptersTest.kt @@ -152,6 +152,17 @@ class TextInputLayoutBindingAdaptersTest { } } + @Test + fun testBindingAdapters_setSelection_arabicLanguage_setsSelectionCorrectly() { + launchActivity().use { scenario -> + scenario?.onActivity { activity -> + val testView: AutoCompleteTextView = activity.findViewById(R.id.test_autocomplete_view) + TextInputLayoutBindingAdapters.setLanguageSelection(testView, OppiaLanguage.ARABIC, true) + assertThat(testView.text.toString()).isEqualTo(context.getString(R.string.arabic_localized_language_name)) + } + } + } + private fun launchActivity(): ActivityScenario? { val scenario = ActivityScenario.launch( From dea8c0620d7a44de266b93e37a8955a9db2788ef Mon Sep 17 00:00:00 2001 From: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com> Date: Mon, 9 Sep 2024 22:36:47 +0300 Subject: [PATCH 6/9] Refactor formatting --- .../android/domain/profile/ProfileManagementController.kt | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/domain/src/main/java/org/oppia/android/domain/profile/ProfileManagementController.kt b/domain/src/main/java/org/oppia/android/domain/profile/ProfileManagementController.kt index 2da7027a1b7..95438d0b9d0 100644 --- a/domain/src/main/java/org/oppia/android/domain/profile/ProfileManagementController.kt +++ b/domain/src/main/java/org/oppia/android/domain/profile/ProfileManagementController.kt @@ -1084,11 +1084,7 @@ class ProfileManagementController @Inject constructor( ) ) ProfileActionStatus.PROFILE_TYPE_UNKNOWN -> - AsyncResult.Failure( - UnknownProfileTypeException( - "ProfileType must be set" - ) - ) + AsyncResult.Failure(UnknownProfileTypeException("ProfileType must be set.")) } } From 47af3d1a6144bb1f5341ef1582576b0a4d12c073 Mon Sep 17 00:00:00 2001 From: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com> Date: Mon, 9 Sep 2024 22:43:13 +0300 Subject: [PATCH 7/9] Address reviewer comment --- .../onboarding/CreateProfileFragmentTest.kt | 91 +++++++++---------- 1 file changed, 44 insertions(+), 47 deletions(-) diff --git a/app/src/sharedTest/java/org/oppia/android/app/onboarding/CreateProfileFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/onboarding/CreateProfileFragmentTest.kt index 9de2bcb3b4c..fcab56b8f03 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/onboarding/CreateProfileFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/onboarding/CreateProfileFragmentTest.kt @@ -597,28 +597,26 @@ class CreateProfileFragmentTest { @Test fun testFragment_profileTypeArgumentMissing_showsUnknownProfileTypeError() { val intent = CreateProfileActivity.createProfileActivityIntent(context) - intent.apply { - // Not adding the profile type intent parameter to trigger the exception. - decorateWithUserProfileId(ProfileId.newBuilder().setInternalId(0).build()) + // Not adding the profile type intent parameter to trigger the exception. + intent.decorateWithUserProfileId(ProfileId.newBuilder().setInternalId(0).build()) - val scenario = ActivityScenario.launch(intent) - testCoroutineDispatchers.runCurrent() + val scenario = ActivityScenario.launch(intent) + testCoroutineDispatchers.runCurrent() - scenario.use { - onView(withId(R.id.create_profile_nickname_edittext)) - .perform( - editTextInputAction.appendText("John"), - closeSoftKeyboard() - ) + scenario.use { + onView(withId(R.id.create_profile_nickname_edittext)) + .perform( + editTextInputAction.appendText("John"), + closeSoftKeyboard() + ) - testCoroutineDispatchers.runCurrent() + testCoroutineDispatchers.runCurrent() - onView(withId(R.id.onboarding_navigation_continue)).perform(click()) - testCoroutineDispatchers.runCurrent() + onView(withId(R.id.onboarding_navigation_continue)).perform(click()) + testCoroutineDispatchers.runCurrent() - onView(withId(R.id.create_profile_nickname_error)) - .check(matches(withText(R.string.add_profile_error_missing_profile_type))) - } + onView(withId(R.id.create_profile_nickname_error)) + .check(matches(withText(R.string.add_profile_error_missing_profile_type))) } } @@ -637,20 +635,19 @@ class CreateProfileFragmentTest { private fun launchNewLearnerProfileActivity(): ActivityScenario? { - val intent = CreateProfileActivity.createProfileActivityIntent(context) - intent.apply { - decorateWithUserProfileId(ProfileId.newBuilder().setInternalId(0).build()) - putProtoExtra( - CREATE_PROFILE_PARAMS_KEY, - CreateProfileActivityParams.newBuilder() - .setProfileType(ProfileType.SOLE_LEARNER) - .build() - ) - val scenario = ActivityScenario.launch(intent) - testCoroutineDispatchers.runCurrent() - return scenario - } - } + val intent = CreateProfileActivity.createProfileActivityIntent(context) + intent.decorateWithUserProfileId(ProfileId.newBuilder().setInternalId(0).build()) + intent.putProtoExtra( + CREATE_PROFILE_PARAMS_KEY, + CreateProfileActivityParams.newBuilder() + .setProfileType(ProfileType.SOLE_LEARNER) + .build() + ) + val scenario = ActivityScenario.launch(intent) + testCoroutineDispatchers.runCurrent() + return scenario + } +} private fun setUpTestApplicationComponent() { ApplicationProvider.getApplicationContext().inject(this) @@ -692,24 +689,24 @@ class CreateProfileFragmentTest { @Component.Builder interface Builder : ApplicationComponent.Builder - fun inject(newLearnerProfileFragmentTest: CreateProfileFragmentTest) - } - - class TestApplication : Application(), ActivityComponentFactory, ApplicationInjectorProvider { - private val component: TestApplicationComponent by lazy { - DaggerCreateProfileFragmentTest_TestApplicationComponent.builder() - .setApplication(this) - .build() as TestApplicationComponent - } + fun inject(newLearnerProfileFragmentTest: CreateProfileFragmentTest) +} - fun inject(newLearnerProfileFragmentTest: CreateProfileFragmentTest) { - component.inject(newLearnerProfileFragmentTest) - } +class TestApplication : Application(), ActivityComponentFactory, ApplicationInjectorProvider { + private val component: TestApplicationComponent by lazy { + DaggerCreateProfileFragmentTest_TestApplicationComponent.builder() + .setApplication(this) + .build() as TestApplicationComponent + } - override fun createActivityComponent(activity: AppCompatActivity): ActivityComponent { - return component.getActivityComponentBuilderProvider().get().setActivity(activity).build() - } + fun inject(newLearnerProfileFragmentTest: CreateProfileFragmentTest) { + component.inject(newLearnerProfileFragmentTest) + } - override fun getApplicationInjector(): ApplicationInjector = component + override fun createActivityComponent(activity: AppCompatActivity): ActivityComponent { + return component.getActivityComponentBuilderProvider().get().setActivity(activity).build() } + + override fun getApplicationInjector(): ApplicationInjector = component +} } From cc7147abfb66c02397cd77c76ba351f02bc534db Mon Sep 17 00:00:00 2001 From: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com> Date: Mon, 9 Sep 2024 23:03:43 +0300 Subject: [PATCH 8/9] Cleanup language selection impl --- .../OnboardingAppLanguageViewModel.kt | 6 +- .../onboarding/OnboardingFragmentPresenter.kt | 6 +- .../translation/AppLanguageResourceHandler.kt | 22 ------ .../TextInputLayoutBindingAdaptersTest.kt | 4 +- .../onboarding/CreateProfileFragmentTest.kt | 78 ++++++++++--------- 5 files changed, 50 insertions(+), 66 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/onboarding/OnboardingAppLanguageViewModel.kt b/app/src/main/java/org/oppia/android/app/onboarding/OnboardingAppLanguageViewModel.kt index 869072e106f..312c4a9f08b 100644 --- a/app/src/main/java/org/oppia/android/app/onboarding/OnboardingAppLanguageViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/onboarding/OnboardingAppLanguageViewModel.kt @@ -13,8 +13,8 @@ class OnboardingAppLanguageViewModel @Inject constructor() : ObservableViewModel private val _languageSelectionLiveData = MutableLiveData() /** Get the list of app supported languages to be displayed in the language dropdown. */ - val supportedAppLanguagesList: LiveData> get() = _supportedAppLanguagesList - private val _supportedAppLanguagesList = MutableLiveData>() + val supportedAppLanguagesList: LiveData> get() = _supportedAppLanguagesList + private val _supportedAppLanguagesList = MutableLiveData>() /** Sets the app language selection. */ fun setSystemLanguageLivedata(language: OppiaLanguage) { @@ -22,7 +22,7 @@ class OnboardingAppLanguageViewModel @Inject constructor() : ObservableViewModel } /** Sets the list of app supported languages to be displayed in the language dropdown. */ - fun setSupportedAppLanguages(languageList: List) { + fun setSupportedAppLanguages(languageList: List) { _supportedAppLanguagesList.value = languageList } } diff --git a/app/src/main/java/org/oppia/android/app/onboarding/OnboardingFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/onboarding/OnboardingFragmentPresenter.kt index dcdf5ec1d53..68a20fcbf0d 100644 --- a/app/src/main/java/org/oppia/android/app/onboarding/OnboardingFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/onboarding/OnboardingFragmentPresenter.kt @@ -87,7 +87,7 @@ class OnboardingFragmentPresenter @Inject constructor( fragment.requireContext(), R.layout.onboarding_language_dropdown_item, R.id.onboarding_language_text_view, - languagesList + languagesList.map { appLanguageResourceHandler.computeLocalizedDisplayName(it) } ) onboardingLanguageDropdown.setAdapter(adapter) } @@ -205,9 +205,7 @@ class OnboardingFragmentPresenter @Inject constructor( { result -> when (result) { is AsyncResult.Success -> { - onboardingAppLanguageViewModel.setSupportedAppLanguages( - result.value.map { appLanguageResourceHandler.computeLocalizedDisplayName(it) } - ) + onboardingAppLanguageViewModel.setSupportedAppLanguages(result.value) } is AsyncResult.Failure -> { oppiaLogger.e( diff --git a/app/src/main/java/org/oppia/android/app/translation/AppLanguageResourceHandler.kt b/app/src/main/java/org/oppia/android/app/translation/AppLanguageResourceHandler.kt index 29b555b13a2..be2fa522409 100644 --- a/app/src/main/java/org/oppia/android/app/translation/AppLanguageResourceHandler.kt +++ b/app/src/main/java/org/oppia/android/app/translation/AppLanguageResourceHandler.kt @@ -186,18 +186,6 @@ class AppLanguageResourceHandler @Inject constructor( } } - /** - * Returns an [OppiaLanguage] from its human-readable, localized representation. - * It is expected that each input string is localized to the user's current locale, as per - * [computeLocalizedDisplayName]. - */ - fun getOppiaLanguageFromDisplayName(displayName: String): OppiaLanguage { - val localizedNameMap = OppiaLanguage.values() - .filter { it !in IGNORED_OPPIA_LANGUAGES } - .associateBy { computeLocalizedDisplayName(it) } - return localizedNameMap[displayName] ?: OppiaLanguage.ENGLISH - } - private fun getLocalizedDisplayName(languageCode: String, regionCode: String = ""): String { // TODO(#3791): Remove this dependency. val locale = Locale(languageCode, regionCode) @@ -205,14 +193,4 @@ class AppLanguageResourceHandler @Inject constructor( if (it.isLowerCase()) it.titlecase(locale) else it.toString() } } - - private companion object { - private val IGNORED_AUDIO_LANGUAGES = - listOf( - AudioLanguage.NO_AUDIO, AudioLanguage.AUDIO_LANGUAGE_UNSPECIFIED, AudioLanguage.UNRECOGNIZED - ) - - private val IGNORED_OPPIA_LANGUAGES = - listOf(OppiaLanguage.LANGUAGE_UNSPECIFIED, OppiaLanguage.UNRECOGNIZED) - } } diff --git a/app/src/sharedTest/java/org/oppia/android/app/databinding/TextInputLayoutBindingAdaptersTest.kt b/app/src/sharedTest/java/org/oppia/android/app/databinding/TextInputLayoutBindingAdaptersTest.kt index 38b1e28773e..844f2e70327 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/databinding/TextInputLayoutBindingAdaptersTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/databinding/TextInputLayoutBindingAdaptersTest.kt @@ -158,7 +158,9 @@ class TextInputLayoutBindingAdaptersTest { scenario?.onActivity { activity -> val testView: AutoCompleteTextView = activity.findViewById(R.id.test_autocomplete_view) TextInputLayoutBindingAdapters.setLanguageSelection(testView, OppiaLanguage.ARABIC, true) - assertThat(testView.text.toString()).isEqualTo(context.getString(R.string.arabic_localized_language_name)) + assertThat(testView.text.toString()).isEqualTo( + context.getString(R.string.arabic_localized_language_name) + ) } } } diff --git a/app/src/sharedTest/java/org/oppia/android/app/onboarding/CreateProfileFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/onboarding/CreateProfileFragmentTest.kt index fcab56b8f03..c59489c20c9 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/onboarding/CreateProfileFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/onboarding/CreateProfileFragmentTest.kt @@ -134,13 +134,20 @@ import javax.inject.Singleton qualifiers = "port-xxhdpi" ) class CreateProfileFragmentTest { - @get:Rule val initializeDefaultLocaleRule = InitializeDefaultLocaleRule() - @get:Rule val oppiaTestRule = OppiaTestRule() - @Inject lateinit var testCoroutineDispatchers: TestCoroutineDispatchers - @Inject lateinit var context: Context - @Inject lateinit var editTextInputAction: EditTextInputAction - @Inject lateinit var testGlideImageLoader: TestGlideImageLoader - @Inject lateinit var profileTestHelper: ProfileTestHelper + @get:Rule + val initializeDefaultLocaleRule = InitializeDefaultLocaleRule() + @get:Rule + val oppiaTestRule = OppiaTestRule() + @Inject + lateinit var testCoroutineDispatchers: TestCoroutineDispatchers + @Inject + lateinit var context: Context + @Inject + lateinit var editTextInputAction: EditTextInputAction + @Inject + lateinit var testGlideImageLoader: TestGlideImageLoader + @Inject + lateinit var profileTestHelper: ProfileTestHelper @Before fun setUp() { @@ -635,19 +642,18 @@ class CreateProfileFragmentTest { private fun launchNewLearnerProfileActivity(): ActivityScenario? { - val intent = CreateProfileActivity.createProfileActivityIntent(context) - intent.decorateWithUserProfileId(ProfileId.newBuilder().setInternalId(0).build()) - intent.putProtoExtra( - CREATE_PROFILE_PARAMS_KEY, - CreateProfileActivityParams.newBuilder() - .setProfileType(ProfileType.SOLE_LEARNER) - .build() - ) - val scenario = ActivityScenario.launch(intent) - testCoroutineDispatchers.runCurrent() - return scenario - } -} + val intent = CreateProfileActivity.createProfileActivityIntent(context) + intent.decorateWithUserProfileId(ProfileId.newBuilder().setInternalId(0).build()) + intent.putProtoExtra( + CREATE_PROFILE_PARAMS_KEY, + CreateProfileActivityParams.newBuilder() + .setProfileType(ProfileType.SOLE_LEARNER) + .build() + ) + val scenario = ActivityScenario.launch(intent) + testCoroutineDispatchers.runCurrent() + return scenario + } private fun setUpTestApplicationComponent() { ApplicationProvider.getApplicationContext().inject(this) @@ -689,24 +695,24 @@ class CreateProfileFragmentTest { @Component.Builder interface Builder : ApplicationComponent.Builder - fun inject(newLearnerProfileFragmentTest: CreateProfileFragmentTest) -} - -class TestApplication : Application(), ActivityComponentFactory, ApplicationInjectorProvider { - private val component: TestApplicationComponent by lazy { - DaggerCreateProfileFragmentTest_TestApplicationComponent.builder() - .setApplication(this) - .build() as TestApplicationComponent + fun inject(newLearnerProfileFragmentTest: CreateProfileFragmentTest) } - fun inject(newLearnerProfileFragmentTest: CreateProfileFragmentTest) { - component.inject(newLearnerProfileFragmentTest) - } + class TestApplication : Application(), ActivityComponentFactory, ApplicationInjectorProvider { + private val component: TestApplicationComponent by lazy { + DaggerCreateProfileFragmentTest_TestApplicationComponent.builder() + .setApplication(this) + .build() as TestApplicationComponent + } - override fun createActivityComponent(activity: AppCompatActivity): ActivityComponent { - return component.getActivityComponentBuilderProvider().get().setActivity(activity).build() - } + fun inject(newLearnerProfileFragmentTest: CreateProfileFragmentTest) { + component.inject(newLearnerProfileFragmentTest) + } - override fun getApplicationInjector(): ApplicationInjector = component -} + override fun createActivityComponent(activity: AppCompatActivity): ActivityComponent { + return component.getActivityComponentBuilderProvider().get().setActivity(activity).build() + } + + override fun getApplicationInjector(): ApplicationInjector = component + } } From fac0a9547ce9937bc0da5c3489c8cf4a6e49389e Mon Sep 17 00:00:00 2001 From: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com> Date: Tue, 10 Sep 2024 08:17:28 +0300 Subject: [PATCH 9/9] Nit --- .../android/domain/profile/ProfileManagementControllerTest.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/domain/src/test/java/org/oppia/android/domain/profile/ProfileManagementControllerTest.kt b/domain/src/test/java/org/oppia/android/domain/profile/ProfileManagementControllerTest.kt index cfdc893cc52..287239d6e72 100644 --- a/domain/src/test/java/org/oppia/android/domain/profile/ProfileManagementControllerTest.kt +++ b/domain/src/test/java/org/oppia/android/domain/profile/ProfileManagementControllerTest.kt @@ -1518,7 +1518,7 @@ class ProfileManagementControllerTest { ) val failure = monitorFactory.waitForNextFailureResult(updateProvider) - assertThat(failure).hasMessageThat().isEqualTo("ProfileType must be set") + assertThat(failure).hasMessageThat().isEqualTo("ProfileType must be set.") } @Test @@ -1616,7 +1616,7 @@ class ProfileManagementControllerTest { ) val failure = monitorFactory.waitForNextFailureResult(updateProvider) - assertThat(failure).hasMessageThat().isEqualTo("ProfileType must be set") + assertThat(failure).hasMessageThat().isEqualTo("ProfileType must be set.") } private fun addTestProfiles() {