From 5ad95d0cb356215d1b3882fbfb8177f97ad79eb9 Mon Sep 17 00:00:00 2001 From: Rd Date: Sun, 11 Aug 2024 02:30:25 +0530 Subject: [PATCH 01/64] Introducing new wiki page for code coverage --- wiki/Oppia-Android-Code-Coverage.md | 87 +++++++++++++++++++++++++++++ wiki/_Sidebar.md | 3 + 2 files changed, 90 insertions(+) create mode 100644 wiki/Oppia-Android-Code-Coverage.md diff --git a/wiki/Oppia-Android-Code-Coverage.md b/wiki/Oppia-Android-Code-Coverage.md new file mode 100644 index 00000000000..8225cbd9cf4 --- /dev/null +++ b/wiki/Oppia-Android-Code-Coverage.md @@ -0,0 +1,87 @@ +## Table of Contents + +- [Overview](#overview) +- [Understanding code coverage](#understanding-code-coverage) +- [Why is Code Coverage Important?](#why-is-code-coverage-important) +- [How to use the code coverage tool?](#how-to-use-the-code-coverage-tool) + - [Code Coverage in CI Environment](#code-coverage-in-ci-environment) + - [Code Coverage in local Development](#code-coverage-in-local-development) + - [Generating Html Reports](#generating-html-reports) +- [Limitations of the code coverage tool](#limitations-of-the-code-coverage-tool) + +# Overview +In software engineering, code coverage, also called test coverage, is a percentage measure of the degree to which the source code of a program is executed when a particular test suite is run. A program with high code coverage has more of its source code executed during testing, which suggests it has a lower chance of containing undetected software bugs compared to a program with low code coverage. + +# Understanding code coverage +Code coverage measures the extent to which the code is tested by the automated tests. It indicates whether all parts of the code are examined to ensure they function correctly. + +Let's take a look at how code coverage works with a simple Kotlin function. + +Consider a Kotlin function designed to determine if a number is positive or negative: + +```kotlin +fun checkSign(n: Int): String { + return if (n >= 0) { + "Positive" + } else { + "Negative" + } +} +``` + +A test is created to verify that this function works correctly: +Please refer to the [Oppia Android testing documentation](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Testing) to learn more about writing tests. + +``` +import org.junit.Test +import kotlin.test.assertEquals + +class checkSignTest { + @Test + fun testCheckSign_withPositiveInteger_returnsPositive() { + assertEquals("Positive", checkSign(4)) + } +} +``` + +This test checks whether passing a positive integer '+4' `checkSign(4)` correctly returns "Positive" as output. +However, it doesn’t test how the function handles negative numbers. This means that only a portion of the function’s behavior is being tested, leaving the handling of negative numbers unverified. + +For thorough testing, the tests should also check how the function responds to negative inputs, this can be done by adding a test case by passing a negative integer to expect a return of "Negative" as output. + +```diff +import org.junit.Test +import kotlin.test.assertEquals + +class checkSignTest { + @Test + fun testCheckSign_withPositiveInteger_returnsPositive() { + assertEquals("Positive", checkSign(4)) + } + ++ @Test ++ fun testCheckSign_withNegativeInteger_returnsNegative() { ++ assertEquals("Negative", checkSign(-7)) ++ } +} +``` + +# Why is code coverage important? +### 1. Minimizes the Risk of Bugs +High code coverage means your code is thoroughly tested. This helps find and fix bugs early. + +### 2. Maintains Code Stability +When you make changes to your code, high coverage helps ensure that those changes don’t break existing functionality. + +### 3. Facilitates Continuous Integration and Deployment +With high code coverage, you can confidently integrate and deploy code changes more frequently. Automated tests act as a safety check, allowing you to release updates automatically with reduced risk. + +### 4. Encourages Comprehensive Testing +Striving for high code coverage encourages to write tests for all parts of the code, including edge cases and less common scenarios, while preparing the application to handle unexpected conditions gracefully. + +### 5. Provides Confidence in Code Quality +Achieving a high percentage of code coverage gives confidence in the quality of the code. It also helps in maintaining a high standard of code quality over time. + +# +**The Goal is to reach 100% Code Coverage.** +# diff --git a/wiki/_Sidebar.md b/wiki/_Sidebar.md index 16a5e8e93a1..da510d653a3 100644 --- a/wiki/_Sidebar.md +++ b/wiki/_Sidebar.md @@ -25,6 +25,9 @@ * 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) + * Code Coverage + * [Oppia Android Code Coverage](https://github.com/oppia/oppia-android-workflow/wiki/Oppia-Android-Code-Coverage) + * [Writing effective tests](https://github.com/oppia/oppia-android/wiki/) * [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) * [RTL Guidelines](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines) From dae2add3fa22da2bed0c69b75205b0825c553e92 Mon Sep 17 00:00:00 2001 From: Rd Date: Tue, 13 Aug 2024 03:28:26 +0530 Subject: [PATCH 02/64] How to use code coverage tool - wiki additions --- wiki/Oppia-Android-Code-Coverage.md | 303 +++++++++++++++++++++++++++- 1 file changed, 298 insertions(+), 5 deletions(-) diff --git a/wiki/Oppia-Android-Code-Coverage.md b/wiki/Oppia-Android-Code-Coverage.md index 8225cbd9cf4..88a714fc2f7 100644 --- a/wiki/Oppia-Android-Code-Coverage.md +++ b/wiki/Oppia-Android-Code-Coverage.md @@ -1,12 +1,14 @@ ## Table of Contents - [Overview](#overview) -- [Understanding code coverage](#understanding-code-coverage) +- [Understanding Code Coverage](#understanding-code-coverage) - [Why is Code Coverage Important?](#why-is-code-coverage-important) - [How to use the code coverage tool?](#how-to-use-the-code-coverage-tool) - - [Code Coverage in CI Environment](#code-coverage-in-ci-environment) - - [Code Coverage in local Development](#code-coverage-in-local-development) - - [Generating Html Reports](#generating-html-reports) + - [Continuous Itegration Checks on Pull Request](#1-continuous-integration-checks-on-pull-requests) + - [Understanding the CI Coverage Report](#11-understanding-the-ci-coverage-report) + - [Local Command Line Interface Tools](#2-local-command-line-interface-cli-tools) + - [Understanding the Html Reports](#21-understanding-the-ci-coverage-report) +- [Increasing Code Coverage Metrics](#increasing-code-coverage-metrics) - [Limitations of the code coverage tool](#limitations-of-the-code-coverage-tool) # Overview @@ -66,6 +68,10 @@ class checkSignTest { } ``` +# +>The ultimate Goal is to reach 100% Code Coverage. +# + # Why is code coverage important? ### 1. Minimizes the Risk of Bugs High code coverage means your code is thoroughly tested. This helps find and fix bugs early. @@ -82,6 +88,293 @@ Striving for high code coverage encourages to write tests for all parts of the c ### 5. Provides Confidence in Code Quality Achieving a high percentage of code coverage gives confidence in the quality of the code. It also helps in maintaining a high standard of code quality over time. +# How to use the code coverage tool? + +Oppia supports code coverage analysis through two primary methods: +1. Continuous Integration (CI) Checks on Pull Requests (PRs) +2. Local Command-Line Interface (CLI) Tools + +## 1. Continuous Integration Checks on Pull Requests + +Once a pull request (PR) is created, the Continuous Integration (CI) system automatically triggers a coverage check on all modified files. + +Note: The coverage checks are initiated only after the unit tests have successfully passed. + +![Screenshot (1596)](https://github.com/user-attachments/assets/5bacbb09-cf75-4811-b24c-84692f34916e) + +Following the completion of the coverage analysis, a detailed coverage analysis report will be uploaded as a comment on your PR. + +![image](https://github.com/user-attachments/assets/f0b5ae9d-9e64-4431-b493-c35eae94d2c1) + +## 1.1 Understanding the CI Coverage Report + + +Let's look at a sample coverage report to understand the details it provides. + +## Coverage Report + +### Results +Number of files assessed: 5
+Overall Coverage: **94.26%**
+Coverage Analysis: **FAIL** :x:
+## + +### Failure Cases + +| File | Failure Reason | Status | +|------|----------------|:------:| +| File.kt | No appropriate test file found for File.kt. | :x: | + +### Failing coverage + +| File | Coverage | Lines Hit | Status | Min Required | +|------|:--------:|----------:|:------:|:------------:| +|
Fail1.ktscripts/src/java/org/oppia/android/scripts/coverage/Fail1.kt
| 51.38% | 148 / 288 | :x: | 70% | +|
Fail2.ktutility/src/main/java/org/oppia/android/util/Fail2.kt
| 77.78% | 7 / 9 | :x: | 80% _*_ | +|
Fail3.ktdomain/src/main/java/org/oppia/android/domain/classify/Fail3.kt
| 10.00% | 1 / 10 | :x: | 30% _*_ | + +>**_*_** represents tests with custom overridden pass/fail coverage thresholds + +### Passing coverage + +
+Files with passing code coverage
+ +| 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% | +
+ +### Exempted coverage +
Files exempted from coverage
+ +| File | Failure Reason | +|------|----------------| +|
TestExemptedFile.ktapp/src/main/java/org/oppia/android/app/activity/TestExemptedFile.kt
| This file is exempted from having a test file; skipping coverage check. | +|
SourceIncompatible.ktapp/src/main/java/org/oppia/android/app/activity/SourceIncompatible.kt
| This file is incompatible with code coverage tooling; skipping coverage check. | + +
+ +# + +The coverage report header provides an overview of the coverage analysis: + +### 1. Report Overview Section + +# + +### Results +Number of files assessed: 5
+Overall Coverage: **94.26%**
+Coverage Analysis: **FAIL** :x:
+ +# + +- **Number of Files Assessed:**
This displays the number of Kotlin files included in the coverage analysis. Files with other extensions, such as .xml, .json, or .md, are excluded from this analysis. + +- **Overall Coverage:**
This reflects the average code coverage percentage for the files modified in the current pull request. It provides a snapshot of how thoroughly the new code is tested. + +- **Coverage Analysis:**
This indicates whether the pull request passes the coverage analysis. A check mark denotes a passing result, while an X mark indicates a failure. + +### 2. Report Details Section + +# + +**2.a. Failure Cases** + +### Failure Cases + +| File | Failure Reason | Status | +|------|----------------|:------:| +| File.kt | No appropriate test file found for File.kt. | :x: | + +# + +This section lists files that failed to acquire coverage data. Failures may occur due to various reasons, including: + +- The absence of an appropriate test file for the respective source file, preventing coverage analysis. +- Bazel failing to collect coverage data for the source file. +- Bazel lacking a reference to the required source file in the collected data. +- Other potential reasons are still under exploration. + The table has two columns: + +1. **File:** Displays the file for which the error occurred (Clicking the drop-down reveals the file's path in the codebase). +2. **Failure Reason:** Describes the reason for the failure. + +Note: If this table or section is not present in the coverage report, it indicates that no changes exhibited failure + +# + +**2.b. Failing coverages** + +### Failing coverage + +| File | Coverage | Lines Hit | Status | Min Required | +|------|:--------:|----------:|:------:|:------------:| +|
Fail1.ktscripts/src/java/org/oppia/android/scripts/coverage/Fail1.kt
| 51.38% | 148 / 288 | :x: | 70% | +|
Fail2.ktutility/src/main/java/org/oppia/android/util/Fail2.kt
| 77.78% | 7 / 9 | :x: | 80% _*_ | +|
Fail3.ktdomain/src/main/java/org/oppia/android/domain/classify/Fail3.kt
| 10.00% | 1 / 10 | :x: | 30% _*_ | + +>**_*_** represents tests with custom overridden pass/fail coverage thresholds + +# + +This section highlights files that have failed to meet the minimum coverage percentage. Any file that does not meet the minimum threshold percentage is considered to have a failing status. The minimum threshold is set to a standard value (currently as 70%) and displayed alongside the status for each file. + +Files with overridden coverage thresholds are indicated by an asterisk (*) and include the specific required percentage. + +Note: Files listed may have met the standard minimum threshold but still fail if they do not achieve the required overridden coverage percentage. These files are strictly obligated to meet their specific required percentage and do not consider the minimum threshold. + +For a complete list of files with overridden coverage thresholds, refer to the [test_file_exemptions.textproto](https://github.com/oppia/oppia-android/blob/develop/scripts/assets/test_file_exemptions.textproto) file. + +The data presented includes: + +1. **File:** Name of the file that failed to meet the coverage threshold (Clicking the drop-down reveals the file's path in the codebase). +2. **Coverage Percentage:** Percentage of coverage achieved for the file. +3. **Lines Covered:** Number of lines covered by the test cases, shown in "lines hit / lines found" format. +4. **Status:** Indicates whether the coverage status is a pass or fail. +5. **Required Percentage:** Minimum percentage required to pass the coverage check for the file. + +# + +**2.c. Passing coverages** + +### Passing coverage + +
+Files with passing code coverage
+ +| File | Coverage | Lines Hit | Status | Min Required | +|------|:--------:|----------:|:------:|:------------:| +|
Pass.ktutility/src/main/java/org/oppia/android/util/Pass.kt
| 94.26% | 197 / 209 | :white_check_mark: | 70% | +
+ +# + +This section highlights files that have met the minimum coverage requirements. Files with overridden coverage thresholds are marked with an asterisk (*), and their specific required percentages are provided. + +These files have meet their overridden minimum coverage to achieve a passing status. + +For a complete list of files with overridden coverage thresholds, refer to the [test_file_exemptions.textproto](https://github.com/oppia/oppia-android/blob/develop/scripts/assets/test_file_exemptions.textproto) file. + +The data presented includes: + +1. **File:** Name of the file that met the coverage threshold (Clicking the drop-down reveals the file's path in the codebase). +2. **Coverage Percentage:** Percentage of coverage achieved for the file. +3. **Lines Covered:** Number of lines covered by the test cases, shown in "lines hit / lines found" format. +4. **Status:** Indicates whether the coverage status is a pass or fail. +5. **Required Percentage:** Minimum percentage required to pass the coverage check for the file. + +**2.d. Exempted coverages** + +### Exempted coverage +
Files exempted from coverage
+ +| File | Failure Reason | +|------|----------------| +|
TestExemptedFile.ktapp/src/main/java/org/oppia/android/app/activity/TestExemptedFile.kt
| This file is exempted from having a test file; skipping coverage check. | +|
SourceIncompatible.ktapp/src/main/java/org/oppia/android/app/activity/SourceIncompatible.kt
| This file is incompatible with code coverage tooling; skipping coverage check. | + +
+ +# + +Certain files are exempt from coverage checks. These exemptions include: + +1. **Test File Exemptions:** Files that do not have corresponding test files are exempt 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. + +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) + +This section appears at the bottom of the report, as a drop-down. It includes a table listing the exemptions as, + +1. **File:** Displays the file name that is exempted (Clicking the drop-down reveals the file's path in the codebase). +2. **Failure Reason:** Describes the specific reason for its exemption. + +# + +With this report, you can review the coverage percentages for various files and identify which files pass or fail the coverage check. + +# + +## 2. Local Command-Line Interface (CLI) Tools + +While the CI check provides an overview of coverage, you might want to visualize how tests cover specific files locally, including which lines are covered and which are not. For this, Oppia's local command-line coverage tooling is useful. + +Note: Follow these [Bazel setup instructions](https://github.com/oppia/oppia-android/wiki/Oppia-Bazel-Setup-Instructions) if Bazel isn't yet set up on your device: + +Oppia Android allows you to generate coverage reports in HTML format using the command: + +### Run Coverage + +``` +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: + +``` +bazel run //scripts:run_coverage -- $(pwd) utility/src/main/java/org/oppia/android/util/math/MathTokenizer.kt +``` + +By default, this will generate an HTML report in the coverage_reports directory. For the given file, the report will be saved as **coverage_reports/utility/src/main/java/org/oppia/android/util/math/MathTokenizer/coverage.html** + +A list of files can be provided as an input to generate coverage reports for all the files. + +``` +bazel run //scripts:run_coverage -- $(pwd) +utility/src/main/java/org/oppia/android/util/math/MathTokenizer.kt +utility/src/main/java/org/oppia/android/util/parser/math/MathModel.kt +``` + +### Process Timeout + +The default wait time for a process to complete is 300 seconds (5 minutes). If the process does not finish within this period, you may encounter a "Process Did Not Finish Within Intended Time" error. To extend the wait time, you can modify the timeout setting by adding the flag --processTimeout=15 (The Time unit is minutes). + +``` +bazel run //scripts:run_coverage -- $(pwd) +utility/src/main/java/org/oppia/android/util/parser/math/MathModel.kt --processTimeout=15 +``` + +## 2.1 Understanding the CI Coverage Report + +Let's examine the coverage.html report located at coverage_reports/utility/src/main/java/org/oppia/android/util/math/MathTokenizer/coverage.html to understand its contents. + +**Header Section:** At the top of the report, you'll find information including: + +![image](https://github.com/user-attachments/assets/08c4f0a0-51b5-4d8e-a2f3-ff13d8b6cb2f) + # -**The Goal is to reach 100% Code Coverage.** + +- **File Path:** The relative path of the file to the oppia-android codebase. +- **Coverage Percentage:** The percentage of code covered by tests. +- **Line Coverage Data:** Details on the coverage of individual lines. +- **Main Content:** + +Visual Coverage Representation: The report visually highlights which lines are covered and which are not. + +![image](https://github.com/user-attachments/assets/0e36fde3-639b-4874-b809-59d33827388d) + + # + +- **Red Lines:** Indicate lines not covered by test cases. +- **Green Lines:** Represent lines that are covered by test cases. + +These generated html reports are be used to identify areas of your code that may need additional testing. + +## Increasing Code Coverage Metrics + +While figuring out which lines are covered and which are not, you can increase code coverage by ensuring that each line is thoroughly tested. To enhance code coverage effectiveness, locate the test files associated with the source file (typically found in the same package but within the test directories). + +Note: Some files in the app module may have multiple test files, located in the sharedTest and test packages, all testing a single source file. (eg: The file StateFragment.kt has 2 test files StateFragmentTest.kt under sharedTest and StateFragmentLocalTest.kt under test folder) + +By identifying and adding appropriate test scenarios to cover previously uncovered lines, you will boost the coverage percentage and improve the overall quality and reliability of your code. + +## Limitations of the code coverage tool + + From a2966e845d491b2a2287ef49396039cbdf370c98 Mon Sep 17 00:00:00 2001 From: Rd Date: Tue, 13 Aug 2024 03:51:04 +0530 Subject: [PATCH 03/64] Limitations section code coverage --- wiki/Oppia-Android-Code-Coverage.md | 31 ++++++++++++----------------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/wiki/Oppia-Android-Code-Coverage.md b/wiki/Oppia-Android-Code-Coverage.md index 88a714fc2f7..d8ca85b6a91 100644 --- a/wiki/Oppia-Android-Code-Coverage.md +++ b/wiki/Oppia-Android-Code-Coverage.md @@ -68,29 +68,25 @@ class checkSignTest { } ``` -# ->The ultimate Goal is to reach 100% Code Coverage. -# - # Why is code coverage important? -### 1. Minimizes the Risk of Bugs -High code coverage means your code is thoroughly tested. This helps find and fix bugs early. +- **Minimizes the Risk of Bugs:** + High code coverage means your code is thoroughly tested. This helps find and fix bugs early. -### 2. Maintains Code Stability -When you make changes to your code, high coverage helps ensure that those changes don’t break existing functionality. +- **Maintains Code Stability:** + When you make changes to your code, high coverage helps ensure that those changes don’t break existing functionality. -### 3. Facilitates Continuous Integration and Deployment -With high code coverage, you can confidently integrate and deploy code changes more frequently. Automated tests act as a safety check, allowing you to release updates automatically with reduced risk. +- **Facilitates Continuous Integration and Deployment:** + With high code coverage, you can confidently integrate and deploy code changes more frequently. Automated tests act as a safety check, allowing you to release updates automatically with reduced risk. -### 4. Encourages Comprehensive Testing -Striving for high code coverage encourages to write tests for all parts of the code, including edge cases and less common scenarios, while preparing the application to handle unexpected conditions gracefully. +- **Encourages Comprehensive Testing:** + Striving for high code coverage encourages to write tests for all parts of the code, including edge cases and less common scenarios, while preparing the application to handle unexpected conditions gracefully. -### 5. Provides Confidence in Code Quality -Achieving a high percentage of code coverage gives confidence in the quality of the code. It also helps in maintaining a high standard of code quality over time. +- **Provides Confidence in Code Quality:** + Achieving a high percentage of code coverage gives confidence in the quality of the code. It also helps in maintaining a high standard of code quality over time. # How to use the code coverage tool? -Oppia supports code coverage analysis through two primary methods: +Oppia Android supports code coverage analysis through two primary methods: 1. Continuous Integration (CI) Checks on Pull Requests (PRs) 2. Local Command-Line Interface (CLI) Tools @@ -292,11 +288,8 @@ This section appears at the bottom of the report, as a drop-down. It includes a 1. **File:** Displays the file name that is exempted (Clicking the drop-down reveals the file's path in the codebase). 2. **Failure Reason:** Describes the specific reason for its exemption. -# - With this report, you can review the coverage percentages for various files and identify which files pass or fail the coverage check. -# ## 2. Local Command-Line Interface (CLI) Tools @@ -377,4 +370,6 @@ By identifying and adding appropriate test scenarios to cover previously uncover ## 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. +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. From c6ebcffb0c0713df9d199ef8a90b3c164d00fb66 Mon Sep 17 00:00:00 2001 From: Rd Date: Tue, 13 Aug 2024 06:42:06 +0530 Subject: [PATCH 04/64] Adding page for Writing test with good behavioural coverage --- ...ng-tests-with-good-behavioural-coverage.md | 147 ++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 wiki/Writing-tests-with-good-behavioural-coverage.md diff --git a/wiki/Writing-tests-with-good-behavioural-coverage.md b/wiki/Writing-tests-with-good-behavioural-coverage.md new file mode 100644 index 00000000000..5cd49900279 --- /dev/null +++ b/wiki/Writing-tests-with-good-behavioural-coverage.md @@ -0,0 +1,147 @@ +## Table of Contents + +- [Writing Tests with Good Behavioral Coverage](##writing-tests-with-good-behavioral-coverage) +- [Why is Behavioral Coverage Necessary?](#why-is-behavioral-coverage-necessary) +- [Writing Effective Tests](#writing-effective-tests) + - [Understand the Requirements](#1-understanding-the-requirements) + - [Writing Clear and Descriptive Test Cases](#2-writing-clear-and-descriptive-test-cases) + - [Covering Different Scenarios](#3-covering-different-scenarios) + - [Exception Handling](#4-exception-handling) + + +# Writing Tests with Good Behavioral Coverage + +Writing tests with good behavioral coverage involves ensuring that your code is tested thoroughly to meet its requirements and behave correctly in various scenarios. This approach aims to verify not only that your code works but also that it handles different inputs and edge cases as expected. + +# Why is Behavioral Coverage Necessary? + +Behavioral coverage is crucial because it helps ensure that your code functions correctly under all expected conditions, not just the typical use cases. By covering various scenarios, you can: + +- Detect Bugs Early: Identify and fix issues before they affect users. +- Improve Code Quality: Ensure that the code meets its requirements and handles edge cases. +- Maintain Robustness: Ensure that changes or additions to the codebase do not introduce new issues. + +For more details on testing methodologies specific to Oppia Android, please refer to the Oppia Testing Wiki. + +# Writing Effective Tests + +## 1. Understanding the Requirements + +Before writing tests thoroughly review documentation, user stories, or specifications for the functionality you're testing. +**Example:** +If you're testing a function that checks if a number is positive or negative, make sure it returns "Positive" for numbers greater than or equal to zero and "Negative" for numbers less than zero. + +``` +@Test +fun testCheckSign_forPositiveInput_returnsPositive() { + assertEquals("Positive", checkSign(5)) +} + +@Test +fun testCheckNumber_forNegativeInput_returnsNegative() { + assertEquals("Negative", checkSign(-3)) +} +``` + +# 2. Writing Clear and Descriptive Test Cases + +Each test case should: + +- Clearly describe the scenario and expected outcome. +- Use descriptive names for your test methods. + +Naming Convention: +``` +testAction_withOneCondition_withSecondCondition_hasExpectedOutcome +``` + +Example: +``` +testCheckSign_forPositiveInput_returnsPositive() +testCheckSign_forNegativeInput_returnsNegative() +testCheckSign_forZeroInput_returnsNeitherPositiveNorNegative() +``` + +## 3. Covering Different Scenarios + +Ensure tests cover: + +- Positive Cases: Valid inputs that should pass. +- Negative Cases: Invalid inputs or edge cases. +- Boundary Cases: Inputs at the edge of acceptable ranges. + +``` +@Test +fun testCheckSign_forZeroInput_returnsNeitherPositiveNorNegative() { + assertEquals("Neither Positive Nor Negative", checkSign(0)) // Boundary value +} + +@Test +fun testCheckSign_forPositiveBoundaryInput_returnsPositive() { + assertEquals("Positive", checkSign(1)) // Just above the boundary value +} + +@Test +fun testCheckSign_forNegativeBoundaryInput_returnsNegative() { + assertEquals("Negative", checkSign(-1)) // Just below the boundary value +} +``` + +## 4. Exception Handling + +Exception Handling is a critical aspect of testing that ensures your code can handle and recover from error conditions gracefully. + +- Ensure Error Conditions Are Managed Gracefully: + - You need to verify that your application responds correctly when invalid or unexpected inputs are provided. For instance, if your function is designed to throw an exception when it receives null, you should test that it does so correctly. + +- Verify Correct Exception Types: + - Testing should confirm that the correct type of exception is thrown. This ensures that your error handling logic is specific and accurate. + +``` +@Test +fun testCheckSign_forNullInput_throwsIllegalArgumentException() { + assertThrows { + checkSign(null) + } +} +``` + +- Check for Proper Exception Messages: + - It's important to ensure that exceptions include meaningful and informative messages that help diagnose issues. This can be tested by verifying the content of the exception message. + +``` +@Test +fun testCheckSign_forNullInput_throwsIllegalArgumentException() { + val exception = assertThrows { + checkNumber(null) + } + assertThat(exception).contains("Value cannot be null") +} +``` + +- Ensure Exceptions Are Thrown at Correct Times: + - Ensure that exceptions are thrown only under the right conditions and not during normal operations. + +``` +@Test +fun testCheckSign_forNullInput_throwsIllegalArgumentException() { + // Normal case exception should not be thrown + assertThrows { + checkSign(null) + } +} + +@Test +fun testCheckSign_forNullInput_throwsIllegalArgumentException() { + // Null case exception should be thrown + val exception = assertThrows { + checkNumber(null) + } + assertThat(exception).contains("Value cannot be null") +} +``` + +- Include Edge Cases: + - Consider edge cases where exceptions might be thrown, such as boundary values or extreme input scenarios. + +### WIP From 51393a49069bb14832eaebdba6178695f103437c Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 14 Aug 2024 20:32:21 +0530 Subject: [PATCH 05/64] Rewrote Writing tests with good behavioural coverage wiki page --- ...ng-tests-with-good-behavioural-coverage.md | 1236 ++++++++++++++++- 1 file changed, 1177 insertions(+), 59 deletions(-) diff --git a/wiki/Writing-tests-with-good-behavioural-coverage.md b/wiki/Writing-tests-with-good-behavioural-coverage.md index 5cd49900279..b6c9417c776 100644 --- a/wiki/Writing-tests-with-good-behavioural-coverage.md +++ b/wiki/Writing-tests-with-good-behavioural-coverage.md @@ -1,49 +1,136 @@ ## Table of Contents -- [Writing Tests with Good Behavioral Coverage](##writing-tests-with-good-behavioral-coverage) -- [Why is Behavioral Coverage Necessary?](#why-is-behavioral-coverage-necessary) +- [What is Behavioral Coverage?](#what-is-behavioral-coverage) +- [Understanding Behavioral Coverage and its Importance](#understanding-behavioral-coverage-and-its-importance) - [Writing Effective Tests](#writing-effective-tests) - - [Understand the Requirements](#1-understanding-the-requirements) - - [Writing Clear and Descriptive Test Cases](#2-writing-clear-and-descriptive-test-cases) - - [Covering Different Scenarios](#3-covering-different-scenarios) - - [Exception Handling](#4-exception-handling) + - [Understand the Requirements](#1-understanding-the-requirements) + - [Writing Clear and Descriptive Test Cases](#2-writing-clear-and-descriptive-test-cases) + - [Focusing on Specific Test Cases](#3-focusing-on-specific-test-cases) + - [Covering Different Scenarios](#4-covering-different-scenarios) + - [Covering All Branches, Paths, and Conditions](#5-covering-all-branches-paths-and-conditions) + - [Exception and Error Handling](#6-exception-and-error-handling) +- [Guidelines for Testing Public API](#guidelines-for-testing-public-api) +- [How to Map a Line of Code to Its Corresponding Behaviors?](#how-to-map-a-line-of-code-to-its-corresponding-behaviors) +# What is Behavioral Coverage? -# Writing Tests with Good Behavioral Coverage +Behavioral coverage refers to the practice of testing various behaviors or functional aspects of your code to ensure it operates correctly under different scenarios. Instead of merely executing lines of code, it focuses on validating that all specified behaviors, edge cases, and conditions are correctly handled by the code. -Writing tests with good behavioral coverage involves ensuring that your code is tested thoroughly to meet its requirements and behave correctly in various scenarios. This approach aims to verify not only that your code works but also that it handles different inputs and edge cases as expected. +## Understanding Behavioral Coverage and its Importance -# Why is Behavioral Coverage Necessary? +Focusing on behavioral coverage is essential because: -Behavioral coverage is crucial because it helps ensure that your code functions correctly under all expected conditions, not just the typical use cases. By covering various scenarios, you can: +- Multiple Behaviors per Line: A single line of code can produce multiple behaviors depending on input values and conditions. +- Comprehensive Testing: High line coverage doesn’t guarantee all behaviors are tested. Behavioral coverage ensures every scenario, including edge cases and exceptions, is validated. +- Quality Over Quantity: Achieving high line coverage might give a false sense of security. Testing all possible behaviors ensures higher quality and robustness. -- Detect Bugs Early: Identify and fix issues before they affect users. -- Improve Code Quality: Ensure that the code meets its requirements and handles edge cases. -- Maintain Robustness: Ensure that changes or additions to the codebase do not introduce new issues. +Let's understand it better with a sample function. Consider this function that validates a name: -For more details on testing methodologies specific to Oppia Android, please refer to the Oppia Testing Wiki. +``` +fun getName(name: String? = " ") = + name?.takeIf { it.all { char -> char.isLetterOrWhitespace() } } + ?: throw IllegalArgumentException("Invalid name") +``` + +### Line Coverage Testing + +A basic test case for line coverage might look like this: + +``` +@Test fun testValidName() { + // Tests line coverage by hitting the line where name is accessed + assertThat(getName("Alice"), equalTo("Alice")) +} +``` + +**Line Coverage Result:** This test covers the line of code but doesn’t test all scenarios. + +### Behavioural Coverage Testing + +To ensure behavioral coverage, the test needs to verify various conditions: + +``` +@Test fun testGetName_withDefaultValue_result(getName()) { + // Default value when no name is provided + assertThat(getName(), equalTo(" ")) +} + +@Test fun testGetName_withNullName_throwsException() { + // Exception for null value + assertThrows { getName(null) } +} + +@Test fun testGetName_withSpecialCharacters_throwsException() { + // Exception for special characters + assertThrows { getName("!@#$%^&*()") } +} + +@Test fun testGetName_withEmptyName_result(getName("")) { + // Empty string should use default value + assertThat(getName(""), equalTo(" ")) +} + +@Test fun testGetName_withWhitespaceName_result(getName(" ")) { + // Whitespace name + assertThat(getName(" "), equalTo(" ")) +} +``` + +**Behavioural Coverage Result:** These tests ensure that all potential behaviors, including edge cases and exceptions, are covered, providing a more thorough validation. + +### Quality > Percentage + +While line coverage might reach 100% with a single test, it doesn’t ensure that all scenarios are tested. Behavioral coverage ensures quality by validating all possible scenarios and edge cases, which is more important for reliable software. + +For more details on testing methodologies specific to Oppia Android, please refer to the [Oppia Android Testing](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Testing). # Writing Effective Tests +Writing effective tests is crucial for ensuring that your code behaves correctly under various conditions. Good testing practices help you identify issues early, maintain high code quality, and ensure that changes or new features do not break existing functionality. This guide focuses on how to write tests that not only check if your code runs but also verify that it performs as expected in different scenarios. + ## 1. Understanding the Requirements -Before writing tests thoroughly review documentation, user stories, or specifications for the functionality you're testing. -**Example:** -If you're testing a function that checks if a number is positive or negative, make sure it returns "Positive" for numbers greater than or equal to zero and "Negative" for numbers less than zero. +Before you start writing tests, it's essential to thoroughly understand the requirements or specifications for the functionality you are testing. This ensures that your tests accurately reflect what the code is supposed to do. + +# + +**Example User Story and Specification:** + +| **Aspect** | **Details** | +|--------------------------|-----------------------------------------------| +| **User Story** | As a shopper, I want to check if an item’s price is within my budget. If the price is above my budget, I shouldn’t buy it. If it’s within my budget, I should buy it. | +| **Function Name** | `shouldBuyItem` | +| **Inputs** | - `price`: A double representing the item's price.
- `budget`: A double representing the maximum amount you’re willing to spend. | +| **Output** | A boolean indicating whether the item should be bought. | +| **Behavior** | - Return `true` if the price is less than or equal to the budget.
- Return `false` if the price is greater than the budget. | + +**Requested Feature Code:** + +The function to determine if an item should be bought based on the price and budget is, + +``` +fun shouldBuyItem(price: Double, budget: Double): Boolean { + return price <= budget +} +``` + +**Respective Test Case:** + +To ensure that the shouldBuyItem function works correctly, we can add the following test cases, ``` @Test -fun testCheckSign_forPositiveInput_returnsPositive() { - assertEquals("Positive", checkSign(5)) +fun testShouldBuyItem_withPriceWithinBudget_returnsTrue() { + assertTrue(shouldBuyItem(50.0, 100.0)) } @Test -fun testCheckNumber_forNegativeInput_returnsNegative() { - assertEquals("Negative", checkSign(-3)) +fun testShouldBuyItem_withPriceAboveBudget_returnsFalse() { + assertFalse(shouldBuyItem(150.0, 100.0)) } ``` -# 2. Writing Clear and Descriptive Test Cases +## 2. Writing Clear and Descriptive Test Cases Each test case should: @@ -62,86 +149,1117 @@ testCheckSign_forNegativeInput_returnsNegative() testCheckSign_forZeroInput_returnsNeitherPositiveNorNegative() ``` -## 3. Covering Different Scenarios +## 3. Focusing on Specific Test Cases + +When testing a function that performs multiple actions, it's crucial to write test cases that focus on individual aspects of the function. This approach allows you to isolate and identify issues more effectively if a test fails, rather than dealing with a more complex failure analysis. + +### Why Focus on Specific Test Cases? +Testing one thing at a time helps: + +- Identify Issues More Easily: If a test fails, you know precisely which aspect of the function is problematic. +- Improve Test Clarity: Each test case has a clear purpose, making tests easier to read and maintain. +- Isolate Failures: Helps in pinpointing issues related to a specific behavior or output. + +Consider a function that manages a food order process. This function does the following: + +1. Lists the food items. +2. Calculates the total price. +3. Display the order. +4. Checks if the payment has been made and provides the corresponding message. + +``` +fun processOrder(order: List, paymentMade: Boolean): String { + // List the food items + val itemList = order.joinToString(", ") + + // Calculate total price (mocked here for simplicity) + val totalPrice = order.size * 10.0 + + // Display order + println("Order: $itemList") + println("Total: $totalPrice") + + // Payment status + return if (paymentMade) "Payment successful" else "Payment pending" +} +``` + +**Potential output with payment made:** + +``` +Order: Pizza, Burger +Total: 20.0 +Result: Payment successful +``` + +### Testing the Entire Functionality + +**Single Test Case:** + +``` +@Test +fun testProcessOrder_allSteps_returnsCorrectMessage() { + val result = processOrder(listOf("Pizza", "Burger"), paymentMade = true) + assertThat(result).isEqualTo("Payment successful") +} +``` + +**Difficulties in Testing All Aspects Together:** + +- Complex Failure Diagnosis: If this test fails, you need to diagnose whether the issue lies in listing items, calculating the total, displaying the order, or payment status. +- Less Focused: It does not target individual aspects of the function, making it harder to identify which specific action failed. + +### Testing Specific Aspects + +**Test Case 1: Testing Listing items** + +``` +@Test +fun testProcessOrder_listsItems_correctly() { + processOrder(listOf("Pizza", "Burger"), paymentMade = true) + val output = outContent.toString().trim() + assertThat(output, containsString("Order: Pizza, Burger")) +} +``` + +**Test Case 2: Calculates Total** + +``` +@Test +fun testProcessOrder_calculatesTotal_correctly() { + processOrder(listOf("Pizza", "Burger"), paymentMade = true) + val output = outContent.toString().trim() + assertThat(output, containsString("Total: 20.0")) +} +``` + +**Test Case 3: Payment Success** -Ensure tests cover: +``` +@Test +fun testProcessOrder_paymentSuccessful() { + processOrder(listOf("Pizza", "Burger"), paymentMade = true) + val output = outContent.toString().trim() + assertThat(output, containsString("Payment successful")) +} +``` -- Positive Cases: Valid inputs that should pass. -- Negative Cases: Invalid inputs or edge cases. -- Boundary Cases: Inputs at the edge of acceptable ranges. +**Test Case 4: Payment Pending** ``` @Test -fun testCheckSign_forZeroInput_returnsNeitherPositiveNorNegative() { - assertEquals("Neither Positive Nor Negative", checkSign(0)) // Boundary value +fun testProcessOrder_paymentPending() { + processOrder(listOf("Pizza", "Burger"), paymentMade = false) + val output = outContent.toString().trim() + assertThat(output, containsString("Payment pending")) +} +``` + +**Benefits of specific test cases:** + +- Clear Purpose: Each test case has a single, well-defined objective, making the tests more readable and maintainable. +- Easier Debugging: Focusing on one aspect makes it easier to pinpoint and fix issues. +- Improved Coverage: Ensures that each individual functionality of the method is tested thoroughly. + +## 4. Covering Different Scenarios + +To ensure robust testing, it's crucial to cover various scenarios your function might encounter. This involves testing the function with a range of inputs, including positive numbers, negative numbers, zero, and edge cases. Each scenario should be tested to verify that your function behaves correctly across different conditions. + +Consider a function `checkSign` that takes an integer and returns a string indicating whether the number is positive, negative, or zero. + +``` +fun checkSign(number: Int): String { + return when { + number > 0 -> "Positive" + number < 0 -> "Negative" + else -> "Zero" + } } +``` +### Testing different scenerios + +Positive Number: Verifies that the function correctly identifies positive numbers. + +``` @Test -fun testCheckSign_forPositiveBoundaryInput_returnsPositive() { - assertEquals("Positive", checkSign(1)) // Just above the boundary value +fun testCheckNumber_forPositiveInput_returnsPositive() { + assertThat(checkNumber(5), equalTo("Positive")) } +``` +Negative Number: Ensures that negative numbers are correctly classified. + +``` @Test -fun testCheckSign_forNegativeBoundaryInput_returnsNegative() { - assertEquals("Negative", checkSign(-1)) // Just below the boundary value +fun testCheckNumber_forNegativeInput_returnsNegative() { + assertThat(checkNumber(-3), equalTo("Negative")) } ``` -## 4. Exception Handling +Zero: Checks that zero is handled correctly. + +``` +@Test +fun testCheckNumber_forZeroInput_returnsZero() { + assertThat(checkNumber(0), equalTo("Zero")) +} +``` -Exception Handling is a critical aspect of testing that ensures your code can handle and recover from error conditions gracefully. +Maximum Value: Tests the function with Int.MAX_VALUE to ensure it handles the upper boundary. -- Ensure Error Conditions Are Managed Gracefully: - - You need to verify that your application responds correctly when invalid or unexpected inputs are provided. For instance, if your function is designed to throw an exception when it receives null, you should test that it does so correctly. +``` +@Test +fun testCheckNumber_forMaxValue_returnsPositive() { + assertThat(checkNumber(Int.MAX_VALUE), equalTo("Positive")) +} +``` -- Verify Correct Exception Types: - - Testing should confirm that the correct type of exception is thrown. This ensures that your error handling logic is specific and accurate. +Minimum Value: Tests the function with Int.MIN_VALUE to ensure it handles the lower boundary. ``` @Test -fun testCheckSign_forNullInput_throwsIllegalArgumentException() { - assertThrows { - checkSign(null) +fun testCheckNumber_forMinValue_returnsNegative() { + assertThat(checkNumber(Int.MIN_VALUE), equalTo("Negative")) +} +``` + +## 5. Covering All Branches, Paths, and Conditions + +Testing all branches, paths, and conditions within your code is essential to ensure that every possible execution path is verified. This approach helps in identifying edge cases and logic errors that could otherwise go unnoticed. Effective testing covers all possible scenarios, including combinations of conditions and branching logic. + +Let's see the function to evaluate a user's access level based on their age and membership status. + +``` +fun evaluateAccess(age: Int, isMember: Boolean): String { + var result: String + + if (age >= 18 && isMember) { + result = "Access granted" + } else if (age >= 18 && !isMember) { + result = "Membership required" + } else { + result = "Access denied" } + + return result +} +``` + +The different scenarios and the expected outcomes are, + +| Scenario | Description | Expected Outcome | +|------------------|---------------------------------------|----------------------------| +| Adult Member | `age >= 18` and `isMember = true` | Returns "Access granted" | +| Adult Non-Member | `age >= 18` and `isMember = false` | Returns "Membership required" | +| Minor Member | `age < 18` and `isMember = true` | Returns "Access denied" | +| Minor Non-Member | `age < 18` and `isMember = false` | Returns "Access denied" | + +Testing needs to be performed to cover all branches, paths and conditions. + +``` +@Test +fun testEvaluateAccess_adultMember() { + assertThat(evaluateAccess(25, true), equalTo("Access granted")) +} + +@Test +fun testEvaluateAccess_adultNonMember() { + assertThat(evaluateAccess(30, false), equalTo("Membership required")) +} + +@Test +fun testEvaluateAccess_minorMember() { + assertThat(evaluateAccess(16, true), equalTo("Access denied")) +} + +@Test +fun testEvaluateAccess_minorNonMember() { + assertThat(evaluateAccess(15, false), equalTo("Access denied")) +} +``` + +Testing all branches and conditions ensures that your function can handle all possible scenarios, making it more reliable and easier to maintain. + +## 6. Exception and Error Handling + +**Exception Handling:** + +Exceptions are unexpected conditions or events that disrupt the normal flow of a program. They handle situations not part of the regular program flow, like invalid user input or file operation issues. + +Examples: +- `NullPointerException` (accessing a null reference) +- `IllegalArgumentException` (invalid function argument) +- `IOException` (I/O operation failure) + +This allows to catch and handle issues, allowing the program to continue running or fail in a controlled manner. + +For instance, to handle division by zero error. + +``` +fun divide(a: Int, b: Int): Int { + if (b == 0) throw ArithmeticException("Division by zero") + return a / b +} +``` + +**Error Handling:** + +Errors are more severe issues indicating fundamental problems with the program's environment or state, often beyond the program's control. In Java/Kotlin, these are represented by classes extending Error, like `OutOfMemoryError` or `StackOverflowError`. + +This allows to manage situations where the application may not continue running properly, ensuring the program does not enter an unstable state. + +For instance, + +``` +fun simulateError() { + val arr = IntArray(Int.MAX_VALUE) // May cause OutOfMemoryError +} +``` + +### Testing Exceptions + +Ensure that the code handles invalid or unexpected inputs gracefully and verify that appropriate errors or exceptions are triggered. + +**Functionality:** + +``` +fun divideNumbers(numerator: Int, denominator: Int): Int { + if (denominator == 0) throw IllegalArgumentException("Denominator cannot be zero") + return numerator / denominator } ``` -- Check for Proper Exception Messages: - - It's important to ensure that exceptions include meaningful and informative messages that help diagnose issues. This can be tested by verifying the content of the exception message. +**Test Case:** ``` @Test -fun testCheckSign_forNullInput_throwsIllegalArgumentException() { +fun testDivideNumbers_forZeroDenominator_throwsIllegalArgumentException() { val exception = assertThrows { - checkNumber(null) + divideNumbers(10, 0) } - assertThat(exception).contains("Value cannot be null") + assertThat(exception).contains("Denominator cannot be zero") } ``` -- Ensure Exceptions Are Thrown at Correct Times: - - Ensure that exceptions are thrown only under the right conditions and not during normal operations. +Testing exceptions and error handling is vital to ensure that applications behave correctly under error conditions, provide meaningful feedback, and maintain reliability. Without these tests, applications are prone to unpredictable failures, poor user experiences, and potential security issues. + +### Verifying Correct Exception Types + +Ensure that the correct type of exception is thrown in response to error conditions. This confirms that your error handling logic is specific and accurate. ``` @Test -fun testCheckSign_forNullInput_throwsIllegalArgumentException() { - // Normal case exception should not be thrown +fun testDivideNumbers_forZeroDenominator_throwsIllegalArgumentException() { assertThrows { - checkSign(null) + divideNumbers(10, 0) + } +} +``` + +### Checking for Proper Exception Messages + +It's crucial that exceptions contain meaningful messages that help diagnose issues. Verify that the exception messages are informative and relevant. + +``` +@Test +fun testDivideNumbers_forZeroDenominator_containsCorrectMessage() { + val exception = assertThrows { + divideNumbers(10, 0) + } + assertThat(exception.message).contains("Denominator cannot be zero") +} +``` + +### Ensuring Exceptions Are Thrown at Correct Times + +Exceptions should be thrown only under specific conditions, not during normal operations. Verify that exceptions are correctly managed according to the context. + +``` +@Test +fun testDivideNumbers_forValidInputs_returnsExpectedResult() { + val result = divideNumbers(10, 2) + assertThat(result).isEqualTo(5) +} +``` + +### Testing Edge Cases + +Test edge cases where exceptions might be thrown, such as boundary values or extreme input scenarios. + +**Function with Edge Case Handling:** + +``` +fun calculateDiscount(price: Double, discountPercent: Double): Double { + if (price < 0 || discountPercent < 0) { + throw IllegalArgumentException("Price and discount cannot be negative") + } + return price - (price * discountPercent / 100) +} +``` + +**Test Case:** + +``` +@Test +fun testCalculateDiscount_forNegativePrice_throwsIllegalArgumentException() { + val exception = assertThrows { + calculateDiscount(-100.0, 10.0) + } + assertThat(exception.message).contains("Price and discount cannot be negative") +} + +@Test +fun testCalculateDiscount_forNegativeDiscount_throwsIllegalArgumentException() { + val exception = assertThrows { + calculateDiscount(100.0, -10.0) + } + assertThat(exception.message).contains("Price and discount cannot be negative") +} +``` + +# Guidelines for Testing Public API + +A public API (Application Programming Interface) refers to the set of methods, properties, and functionalities exposed by a class or module for use by external code. It defines how other parts of a system or external systems can interact with the functionality provided by that class or module. + +Public APIs are essential because they provide a way to interact with the functionality of a class or module without exposing its internal workings. They define how external code can use the functionality offered by the class or module, ensuring that interactions are safe and predictable while keeping the internal implementation hidden and secure. + +Let's consider the following example for a public API to withdraw money from the BankAccount. + +``` +class BankAccount( + private var balance: Double, + private val username: String, + private val password: String +) { + + // Public method to withdraw money + fun withdraw( + requestedUsername: String, // Username provided for the withdrawal + requestedPassword: String, // Password provided for the withdrawal + file: File? = null // Optional passbook file to upload to note transactions + amount: Double, // Amount to withdraw + needReceipt: Boolean = false // Flag to indicate if a receipt is needed, defaults to false + ) { + // Verify user credentials + // Validate withdrawal amount + // Perform the withdrawal operation + // Print a receipt if needed + println("Withdrawing $amount for $requestedUsername") + if (needReceipt) { + printReceipt(amount) + } + + // Process the file if provided + file?.let { + processFile(it) + } + } + + private fun isValidUser(requestedUsername: String, requestedPassword: String): Boolean { + return true + } + + private fun isValidWithdrawal(amount: Double): Boolean { + return true + } + + private fun performWithdrawal(amount: Double) { + println("Withdrew $amount. New balance is $balance") + } + + private fun printReceipt(amount: Double) { + println("Receipt: Withdrew $amount. Current balance: $balance") + } + + private fun processFile(file: File) { + println("Processing file: ${file.name}") + } +} +``` + +The **`withdraw`** method serves as the single public entry point for withdrawing money from the account. It handles user validation, amount checking, optional file upload and printing of the receipt. By keeping the internal methods private, the class ensures that the operations are performed in a controlled manner while hiding the complexity of these operations from the user. + +## Testing Public API + +Testing a public API involves verifying that its methods and functionalities work as expected under various conditions. To ensure comprehensive coverage, it's important to focus on the behavior of the API rather than just individual lines of code. + +### 1. Checking Pre-Conditions + +Ensure the necessary setup, such as initializing bank data, is completed before running tests. + +**Test:** + +``` +@Test +fun testWithdraw_noBankData_initializationError() { + val account = BankAccount(null, null, null) // Initialize with null values + + val exception = assertThrows { + account.withdraw("user", "password", 200.0) + } + assertThat(exception.message).contains("Bank data not initialized") +} +``` + +This test ensures that if the bank data is not set up properly, the system throws an appropriate exception. This is important to verify that the API behaves correctly when initialization is incomplete. + +### 2. Testing Valid Data + +Verify that the API correctly processes valid input data. + +``` +@Test +fun testWithdraw_validData_outputsCorrectBalance() { + val account = BankAccount(1000.0, "user", "password") + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) + + account.withdraw("user", "password", 200.0) + + val outputLines = output.toString().trim() + assertThat(outputLines).isEqualTo("Withdrew 200.0. New balance is 800.0") + System.setOut(System.out) +} +``` + +### 3. Testing Invalid Data + +Ensure the API handles incorrect or invalid data properly. + +``` +@Test +fun testWithdraw_invalidUserName_throwsException() { + val account = BankAccount(1000.0, "user", "password") + + val exception = assertThrows { + account.withdraw("invalidUser", "password", 200.0) } + assertThat(exception).contains("Invalid credentials") } @Test -fun testCheckSign_forNullInput_throwsIllegalArgumentException() { - // Null case exception should be thrown +fun testWithdraw_invalidPassword_throwsException() { + val account = BankAccount(1000.0, "user", "password") + val exception = assertThrows { - checkNumber(null) + account.withdraw("user", "invalidPassword", 200.0) } - assertThat(exception).contains("Value cannot be null") + assertThat(exception).contains("Invalid credentials") } ``` -- Include Edge Cases: - - Consider edge cases where exceptions might be thrown, such as boundary values or extreme input scenarios. +### 4. Testing Edge Cases + +Verify the API's behavior with edge cases and boundary conditions. + +``` +@Test +fun testWithdraw_emptyUsername_throwsException() { + val account = BankAccount(1000.0, "user", "password") + + val exception = assertThrows { + account.withdraw("", "password", 200.0) + } + assertThat(exception.message).contains("Username cannot be empty") +} + +@Test +fun testWithdraw_emptyBalance_throwsException() { + val account = BankAccount(0.0, "user", "password") + + val exception = assertThrows { + account.withdraw("user", "password", 200.0) + } + assertThat(exception.message).contains("Insufficient balance") +} + +@Test +fun testWithdraw_emptyAmount_throwsException() { + val account = BankAccount(1000.0, "user", "password") + + val exception = assertThrows { + account.withdraw("user", "password", 0.0) + } + assertThat(exception.message).contains("Invalid withdrawal amount") +} + +@Test +fun testWithdraw_amountGreaterThanBalance_throwsException() { + val account = BankAccount(1000.0, "user", "password") + + val exception = assertThrows { + account.withdraw("user", "password", 1500.0) + } + assertThat(exception.message).contains("Invalid withdrawal amount") +} +``` + +### 5. Testing Default Values + +Verify the API's behaviour with different receipt parameter values. + +``` +@Test +fun testWithdraw_withDefaultReceipt_noReceiptPrinted() { + val account = BankAccount(1000.0, "user", "password") + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) + + account.withdraw("user", "password", 200.0) + + val outputLines = output.toString().trim() + assertThat(outputLines).doesNotContain("Receipt: Withdrew 200.0. Current balance: 800.0") + System.setOut(System.out) +} + +@Test +fun testWithdraw_withNeedReceipt_receiptPrinted() { + val account = BankAccount(1000.0, "user", "password") + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) + + account.withdraw("user", "password", 200.0, needReceipt = true) + + val outputLines = output.toString().trim() + assertThat(outputLines).contains("Receipt: Withdrew 200.0. Current balance: 800.0") + System.setOut(System.out) +} + +@Test +fun testWithdraw_withoutNeedReceipt_noReceiptPrinted() { + val account = BankAccount(1000.0, "user", "password") + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) + + account.withdraw("user", "password", 200.0, needReceipt = false) + + val outputLines = output.toString().trim() + assertThat(outputLines).doesNotContain("Receipt: Withdrew 200.0. Current balance: 800.0") + System.setOut(System.out) +} +``` + +These tests check if the receipt value is correctly processed. They ensure that receipts are printed when requested and not printed otherwise. + +### 6. Testing Validity of File + +Testing how an API handles file inputs is crucial because: + +1. File Format Validation: Ensures that only acceptable file formats are processed, avoiding potential errors or security issues. +2. File Existence: Confirms that the API properly handles cases where files are missing, preventing unexpected failures. +3. File Processing: Validates that the API processes files correctly when they are valid, ensuring proper functionality. + +**Test:** + +a. Testing with Invalid File Format + +``` +@Test +fun testWithdraw_withInvalidFileFormat_throwsException() { + val account = BankAccount(1000.0, "user", "password") + val invalidFile = File("invalidFile.txt") // File with an invalid format + + val exception = assertThrows { + account.withdraw("user", "password", 200.0, file = invalidFile) + } + assertThat(exception).contains("Invalid file format") +} +``` + +b. Testing with Unavailable File + +``` +@Test +fun testWithdraw_withUnavailableFile_throwsException() { + val account = BankAccount(1000.0, "user", "password") + val unavailableFile = File("nonExistentFile.pdf") // File that does not exist + + val exception = assertThrows { + account.withdraw("user", "password", 200.0, file = unavailableFile) + } + assertThat(exception).contains("File not found") +} +``` + +c. Testing with Valid File + +``` +@Test +fun testWithdraw_withValidFile_processesFile() { + val account = BankAccount(1000.0, "user", "password") + val validFile = File("passbook.pdf") + validFile.createNewFile() // Ensure the file exists for the test + + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) + + account.withdraw("user", "password", 200.0, file = validFile) + + assertThat(output.toString().trim()).isEqualTo("Processing file: passbook.pdf") + System.setOut(System.out) +} +``` + +These tests collectively cover critical aspects of file handling, ensuring robust and reliable API functionality. + +### 7. Testing Absence of Unwanted Output + +In addition to validating correct handling of valid and invalid files, it's also important to ensure that unwanted output or behavior does not occur. + +**Test:** + +a. Ensure No Processing Message for Missing File + +``` +@Test +fun testWithdraw_withNoFile_producesNoFileProcessingOutput() { + val account = BankAccount(1000.0, "user", "password") + + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) + + account.withdraw("user", "password", 200.0) // No file provided + + assertThat(output.toString().trim()).doesNotContain("Processing file") + System.setOut(System.out) +} +``` + +This test ensures that no unwanted methods, such as file processing, are called when the file parameter is not provided, thereby verifying that the API behaves as expected without unnecessary actions. + +b. Ensure No Receipt Message for Default Value + +``` +@Test +fun testWithdraw_withDefaultReceipt_noReceiptPrinted() { + val account = BankAccount(1000.0, "user", "password") + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) + + account.withdraw("user", "password", 200.0) // Using default receipt flag (false) + + assertThat(output.toString().trim()).doesNotContain("Receipt:") + System.setOut(System.out) +} +``` + +This test verifies that no receipt message is output when the default receipt flag is in use, ensuring that the API respects the default behavior and does not produce unintended output. + +### 8. 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. + +**a. To verify correctness of output:** + +Verifies that after withdrawing $200, the balance is updated to $800. This checks that the core functionality of updating the balance works correctly. + +``` +@Test +fun testWithdraw_withSufficientBalance_updatesBalance() { + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) + + account.withdraw("user", "password", 200.0) + + assertThat(output.toString().trim()).isEqualTo("Withdrew 200.0. New balance is 800.0") + System.setOut(System.out) +} +``` + +**b. To verify with receipt:** + +Ensures that when a receipt is requested, it includes the correct balance details of the withdrawal. + +``` +@Test +fun testWithdraw_withReceipt_generatesReceipt() { + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) + + account.withdraw("user", "password", 200.0, needReceipt = true) + + assertThat(output.toString().trim()).contains("Receipt: Withdrew 200.0. Current balance: 800.0") + System.setOut(System.out) +} +``` + +**c. To verify with passbook details:** + +Confirms balance statement with the passbook file being updated when a file is provided. + +``` +@Test +fun testWithdraw_withPassbook_updatesPassbook() { + val passbookFile = File("passbook.pdf") + passbookFile.createNewFile() + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) + + account.withdraw("user", "password", 200.0, file = passbookFile) + + // Read the passbook file and check its contents + val fileContents = passbookFile.readText() + assertThat(fileContents).contains("Withdrew 200.0. New balance is 800.0") + + System.setOut(System.out) +} +``` + +**d. To verify based on the log message:** + +Validates that the correct message about the withdrawal is logged. + +``` +@Test +fun testWithdraw_withSufficientBalance_logsCorrectMessage() { + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) + + account.withdraw("user", "password", 200.0) + + assertThat(output.toString().trim()).contains("New balance is 800.0") + System.setOut(System.out) +} +``` + +**e. To verify with the balance is updated correctly:** + +Ensures that the balance is updated correctly after a withdrawal. + +``` +@Test +fun testWithdraw_withSufficientBalance_updatesBalanceCorrectly() { + account.withdraw("user", "password", 200.0) + assertThat(account.balance).isEqualTo(800.0) +} +``` + +These tests cover various aspects of the withdrawal functionality, ensuring that the balance updates correctly with receipts, passbooks and output messages. Although the core functionality of withdrawing funds and updating the balance is consistent, it can be observed in multiple ways. Each test focuses on a specific verification method while ultimately validating the same core functionality. + +## Testing Practices for Effective Validation + +The following best practices are meant to assist in the organization and execution of tests. By following these guidelines, you can ensure that your testing process is efficient, thorough, and well-structured. + +### 1. Use of Helper Functions + +Helper functions are crucial for avoiding repetitive code, especially for pre-setup tasks that are used across multiple test cases. In this scenario, initializing the bank data or ensuring the proper state of the BankAccount object can be encapsulated in helper functions. + +``` +// Helper function to initialize a BankAccount with default data +fun createDefaultBankAccount(): BankAccount { + return BankAccount(1000.0, "user", "password") +} +``` + +This helps to reduce redundancy and maintain consistency across multiple test cases by centralizing repetitive setup tasks. + +### 2. Setup and Teardown using `@Before` and `@After` + +Using `@Before` and `@After` annotations ensures that common setup and cleanup tasks are automatically executed before and after each test case, maintaining a clean and consistent test environment. + +**Example:** + +``` +class BankAccountTests { + + private lateinit var account: BankAccount + + @Before + fun setUp() { + // Initialize a BankAccount instance before each test + account = createDefaultBankAccount() + } + + @After + fun cleanup() { + // Clean up any resources or data after each test + + // Restore the original system output stream after test + System.setOut(System.out) + } + + // Test cases here +} +``` + +### 3. Descriptive Test Names + +Naming test functions descriptively helps in identifying the purpose and scope of each test. Use names that reflect the specific behavior being tested. + +``` +@Test +fun testWithdraw_withValidData_updatesBalance() { + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) + + account.withdraw("user", "password", 200.0) + + // Check that the withdrawal output is correct and no exceptions are thrown + assertThat(output.toString().trim()).contains("Withdrew 200.0. New balance is 800.0") + System.setOut(System.out) +} + +@Test +fun testWithdraw_withNoFile_producesNoFileProcessingOutput() { + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) + + account.withdraw("user", "password", 200.0) + + // Ensure no file processing message is output when no file is provided + assertThat(output.toString().trim()).doesNotContain("Processing file") + System.setOut(System.out) +} + +@Test +fun testWithdraw_withInvalidFileFormat_throwsException() { + val invalidFile = File("invalidFile.txt") + + // Verify that an invalid file format results in an appropriate exception + val exception = assertThrows { + account.withdraw("user", "password", 200.0, file = invalidFile) + } + assertThat(exception.message).contains("Invalid file format") +} + +@Test +fun testWithdraw_withUnavailableFile_throwsException() { + val unavailableFile = File("nonExistentFile.pdf") + + // Verify that attempting to use a non-existent file results in an exception + val exception = assertThrows { + account.withdraw("user", "password", 200.0, file = unavailableFile) + } + assertThat(exception.message).contains("File not found") +} + +@Test +fun testWithdraw_withDefaultReceipt_noReceiptPrinted() { + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) + + account.withdraw("user", "password", 200.0) + + // Ensure that no receipt is printed when the default receipt flag is used + assertThat(output.toString().trim()).doesNotContain("Receipt:") + System.setOut(System.out) +} +``` + +**Importance of Specific Naming and Conditions:** + +- Clarity: Specific names and conditions make tests easier to understand and manage. +- Focus: Helps pinpoint exact scenarios being tested, improving test coverage. +- Debugging: Clear names and conditions aid in quickly identifying the cause of failures. +- Documentation: Serves as self-documentation, providing insight into test purpose and scope. +- Maintenance: Simplifies updates and modifications by clearly defining what each test covers. + +# How to Map a Line of Code to Its Corresponding Behaviors? + +Understanding how to map a line of code to its corresponding behaviors is essential for improving code coverage and writing effective tests. Here’s a structured approach to locate and cover lines of code: + +Let's use our Bank API code from previous examples to understand how to map uncovered lines of code to their corresponding behaviors and effectively write tests to cover them. + +Consider that our test suite covers the code as follows: + +**Test:** + +``` +@Test +fun testWithdraw_withValidCredentials_printsWithdrawMessage() { + val account = BankAccount(1000.0, "user", "password") + val file = File("file.pdf") + + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) + + account.withdraw("user", "password", 200.0, file = file) + + assertThat(output.toString().trim()).contains("Receipt: Withdrew 200.0. Current balance: 800.0") + System.setOut(System.out) +} +``` + +This validates the code behaviour to function properly with valid inputs. + +### 1. Identify the Line of Code + +You can utilize the Oppia Android code coverage tool to assess the coverage for this code. This will generate an HTML report that helps you visualize the lines covered in green highlight and those not covered in red highlight. + +![image](https://github.com/user-attachments/assets/53bde5dd-3f96-4454-8ed8-159fb090f20e) + +Analyzing the report reveals that the line, + +``` +println("Receipt: Withdrew $amount. Current balance: $balance") +``` + +and its function call `printReceipt` are marked in red, indicating that this line was never executed by the test case. This suggests that the functionality is not covered by the current tests, potentially exposing it to issues or regressions if the code is modified in the future. The green highlights indicate the lines of code that are covered by test cases. + +For more information on how to utilize the code coverage analysis tool, please refer to the [Oppia Android Code Coverage](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage) page. + +### 2. Map to the Line of Code + +**1. Locate the Uncovered Line:** + +Locate to the corresponding line number of the uncovered line in the source file. That would locate to these lines: + +BankAccount.kt + +``` +private fun printReceipt(amount: Double) { + println("Receipt: Withdrew $amount. Current balance: $balance") +} +``` + +Flow Diagram + +```mermaid +graph TD + F[printReceipt] --> G[print - Receipt: Withdrew $400] +``` + +**2. Traceback to the calling point:** + +Next, trace the uncovered line back to where it is called in the code. This helps to understand why it wasn’t executed by the test case. + +``` +fun withdraw( + requestedUsername: String, + requestedPassword: String, + amount: Double, + needReceipt: Boolean = false, // Defaults to false + file: File? = null +) { + // Other code here + + if (needReceipt) { --------------------. + printReceipt(amount) : + } : +} : + : +... : + < -----------------` +private fun printReceipt(amount: Double) { + println("Receipt: Withdrew $amount. Current balance: $balance") +} +``` + +2.1 The Conditional Call + +Identify the condition that controls whether the line of code is executed. Here it is the condition to have the value of **needReceipt** set to **true** to call the `printReceipt` method. + +``` +if (needReceipt) { + printReceipt(amount) +} +``` + +Flow Diagram + +```mermaid +graph TD + D[Condition - if needReceipt is true] --> E[Method Call - printReceipt] + E --> F[printReceipt] + F --> G[print - Receipt: Withdrew $400] +``` + +2.2 Determine the Origin of the Conditional Value + +Next, trace where this conditional value is set in the method. This helps to identify the requirement of the condition to set a passing value and access the method call. + +``` +fun withdraw( + requestedUsername: String, + requestedPassword: String, + amount: Double, + needReceipt: Boolean = false, // Defaults to false ---------. + file: File? = null : +) { : + : + if (needReceipt) { <-----------------------` + printReceipt(amount) + } +} +``` + +Flow Diagram + +```mermaid +graph TD + B[needReceipt Default to false] + D[Condition - if needReceipt is true] --> E[Method Call - printReceipt] + E --> F[printReceipt] + F --> G[print - Receipt: Withdrew $400] +``` + +It can be seen that the **needReceipt** value is passed as a parameter while having a **default value** of **false**. Since the value was **never set to true** in our test case, + +``` +@Test +fun testWithdraw_withValidCredentials_printsWithdrawMessage() { + ... + + account.withdraw("user", "password", 200.0, file = file) + + ... +} +``` + +it defaulted to being false thereby never meeting the condition to perform the `printReceipt`. + +2.3 Trace Back to the Method Call + +Identify the method or function that influences the needReceipt parameter and trace it to the public API where it is used. By understanding this connection, you can modify the needReceipt parameter’s default value in withdraw to affect the behavior of the code. + +``` +fun withdraw( + requestedUsername: String, + requestedPassword: String, + amount: Double, + needReceipt: Boolean = false, // Defaults to false + file: File? = null +) { } +``` + +Flow Diagram + +```mermaid +graph TD + A[Public API - withdraw] --> B[needReceipt Default to false] + A[Public API - withdraw] --> C[needReceipt set to true] + C --> D[Condition - if needReceipt is true] + D --> E[Method Call - printReceipt] + E --> F[printReceipt] + F --> G[print - Receipt: Withdrew $400] +``` + +**3. Add Test Case to Cover the Line:** + +To ensure that the `printReceipt` method is covered, we need to add a test case that sets the **needReceipt** parameter to **true**. + +Test: + +``` +@Test +fun testWithdraw_withReceipt_printsReceipt() { + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) + + // Call withdraw with needReceipt set to true + account.withdraw("user", "password", 200.0, needReceipt = true) + + // Verify that receipt was printed + assertThat(output.toString().trim()).contains("Receipt: Withdrew 200.0. Current balance: 800.0") + System.setOut(System.out) +} +``` + +This test ensures that the `printReceipt` method is invoked and behaves as intended. By covering this line, you verify that the receipt functionality is properly executed, which is crucial for ensuring complete test coverage and the reliability of the feature. + +Performing a code coverage analysis with the added test case would generate a report as below: + +![image](https://github.com/user-attachments/assets/cc4b8ca5-5d10-48a1-893e-19e06c7bff95) -### WIP +By following these steps, you can effectively map an uncovered line of code to its calling point and understand why it was not covered. Adjusting your test cases to trigger the conditions required for the line to be executed will help ensure comprehensive test coverage and minimize the risk of regression issues. \ No newline at end of file From 9630d7faccdb15c6fb0661cfadf1a41272b1ea35 Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 14 Aug 2024 22:39:20 +0530 Subject: [PATCH 06/64] Updates as per reviews part 1 --- wiki/Oppia-Android-Code-Coverage.md | 40 ++++++++++--------- ...ng-tests-with-good-behavioural-coverage.md | 36 ++++++++--------- wiki/_Sidebar.md | 3 +- 3 files changed, 40 insertions(+), 39 deletions(-) diff --git a/wiki/Oppia-Android-Code-Coverage.md b/wiki/Oppia-Android-Code-Coverage.md index d8ca85b6a91..7082f30337c 100644 --- a/wiki/Oppia-Android-Code-Coverage.md +++ b/wiki/Oppia-Android-Code-Coverage.md @@ -36,12 +36,12 @@ Please refer to the [Oppia Android testing documentation](https://github.com/opp ``` import org.junit.Test -import kotlin.test.assertEquals +import com.google.common.truth.Truth.assertThat class checkSignTest { @Test fun testCheckSign_withPositiveInteger_returnsPositive() { - assertEquals("Positive", checkSign(4)) + assertThat(checkSign(4)).isEqualTo("Positive") } } ``` @@ -53,17 +53,17 @@ For thorough testing, the tests should also check how the function responds to n ```diff import org.junit.Test -import kotlin.test.assertEquals +import com.google.common.truth.Truth.assertThat class checkSignTest { @Test fun testCheckSign_withPositiveInteger_returnsPositive() { - assertEquals("Positive", checkSign(4)) + assertThat(checkSign(4)).isEqualTo("Positive") } + @Test + fun testCheckSign_withNegativeInteger_returnsNegative() { -+ assertEquals("Negative", checkSign(-7)) ++ assertThat(checkSign(-7)).isEqualTo("Negative") + } } ``` @@ -192,12 +192,14 @@ This section lists files that failed to acquire coverage data. Failures may occu - Bazel failing to collect coverage data for the source file. - Bazel lacking a reference to the required source file in the collected data. - Other potential reasons are still under exploration. - The table has two columns: + + The table has three columns: 1. **File:** Displays the file for which the error occurred (Clicking the drop-down reveals the file's path in the codebase). 2. **Failure Reason:** Describes the reason for the failure. +3. **Status:** Indicates that the coverage status is a failure. -Note: If this table or section is not present in the coverage report, it indicates that no changes exhibited failure +Note: If this table or section is not present in the coverage report, it indicates that no changes exhibited failure. # @@ -215,7 +217,7 @@ Note: If this table or section is not present in the coverage report, it indicat # -This section highlights files that have failed to meet the minimum coverage percentage. Any file that does not meet the minimum threshold percentage is considered to have a failing status. The minimum threshold is set to a standard value (currently as 70%) and displayed alongside the status for each file. +This section highlights files that have failed to meet the minimum coverage percentage. Any file that does not meet the minimum threshold percentage is considered to have a failing status. The minimum threshold is configured to a standard value, as specified in [CoverageReporter.kt](https://github.com/oppia/oppia-android/blob/develop/scripts/src/java/org/oppia/android/scripts/coverage/reporter/CoverageReporter.kt), and this value is shown alongside the status for each file. Files with overridden coverage thresholds are indicated by an asterisk (*) and include the specific required percentage. @@ -277,13 +279,13 @@ The data presented includes: Certain files are exempt from coverage checks. These exemptions include: -1. **Test File Exemptions:** Files that do not have corresponding test files are exempt from coverage checks. Since no test files are available for these sources, coverage analysis cannot be performed, and these files are therefore skipped. +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. 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) -This section appears at the bottom of the report, as a drop-down. It includes a table listing the exemptions as, +This section appears at the bottom of the report, as a drop-down. It includes a table listing the exemptions with columns: 1. **File:** Displays the file name that is exempted (Clicking the drop-down reveals the file's path in the codebase). 2. **Failure Reason:** Describes the specific reason for its exemption. @@ -295,13 +297,13 @@ With this report, you can review the coverage percentages for various files and While the CI check provides an overview of coverage, you might want to visualize how tests cover specific files locally, including which lines are covered and which are not. For this, Oppia's local command-line coverage tooling is useful. -Note: Follow these [Bazel setup instructions](https://github.com/oppia/oppia-android/wiki/Oppia-Bazel-Setup-Instructions) if Bazel isn't yet set up on your device: +Note: Follow these [Bazel setup instructions](https://github.com/oppia/oppia-android/wiki/Oppia-Bazel-Setup-Instructions) if Bazel isn't yet set up in your local development environment. Oppia Android allows you to generate coverage reports in HTML format using the command: ### Run Coverage -``` +```sh bazel run //scripts:run_coverage -- ``` @@ -310,15 +312,15 @@ bazel run //scripts:run_coverage -- Date: Wed, 14 Aug 2024 23:05:47 +0530 Subject: [PATCH 07/64] Trying to fix the failure of accessing evaluate job if files are skipped As download artifact was accessed even without anything being uploaded causing an error --- .github/workflows/code_coverage.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 07824dfb179..d8f73f0ba31 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -259,6 +259,7 @@ jobs: name: Evaluate Code Coverage Reports runs-on: ubuntu-20.04 needs: code_coverage_run + if: ${{ needs.compute_changed_files.outputs.can_skip_files != 'true' }} env: CACHE_DIRECTORY: ~/.bazel_cache steps: @@ -308,12 +309,13 @@ jobs: publish_coverage_report: name: Publish Code Coverage Report needs: evaluate-code-coverage-reports + if: ${{ needs.compute_changed_files.outputs.can_skip_files != 'true' && !cancelled()}} permissions: pull-requests: write # 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() }} runs-on: ubuntu-latest steps: - name: Download Generated Markdown Report From fd3675d194b9c1f63eeea19e2dd770293b33512a Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 14 Aug 2024 23:18:58 +0530 Subject: [PATCH 08/64] Check coverage status pass with skip files present --- .github/workflows/code_coverage.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index d8f73f0ba31..02d9908f12d 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -343,5 +343,5 @@ jobs: run: exit 1 - name: Check that coverage status is passed - if: ${{ needs.evaluate-code-coverage-reports.result != 'success' }} + if: ${{ needs.compute_changed_files.outputs.can_skip_files != 'true' && needs.evaluate-code-coverage-reports.result != 'success' }} run: exit 1 From e97b1abe555b08c633c5300f01b7ea7dd1e1f359 Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 01:21:31 +0530 Subject: [PATCH 09/64] Updates as per reviews part 2 with addition to coverage summary, unit centric philosophy sections --- wiki/Oppia-Android-Code-Coverage.md | 106 +++++++++++++++++++++++++--- 1 file changed, 95 insertions(+), 11 deletions(-) diff --git a/wiki/Oppia-Android-Code-Coverage.md b/wiki/Oppia-Android-Code-Coverage.md index 7082f30337c..ea80425eaff 100644 --- a/wiki/Oppia-Android-Code-Coverage.md +++ b/wiki/Oppia-Android-Code-Coverage.md @@ -9,6 +9,7 @@ - [Local Command Line Interface Tools](#2-local-command-line-interface-cli-tools) - [Understanding the Html Reports](#21-understanding-the-ci-coverage-report) - [Increasing Code Coverage Metrics](#increasing-code-coverage-metrics) +- [Unit-Centric Coverage Philosophy](#unit-centric-coverage-philosophy) - [Limitations of the code coverage tool](#limitations-of-the-code-coverage-tool) # Overview @@ -192,7 +193,7 @@ This section lists files that failed to acquire coverage data. Failures may occu - Bazel failing to collect coverage data for the source file. - Bazel lacking a reference to the required source file in the collected data. - Other potential reasons are still under exploration. - + The table has three columns: 1. **File:** Displays the file for which the error occurred (Clicking the drop-down reveals the file's path in the codebase). @@ -339,36 +340,119 @@ utility/src/main/java/org/oppia/android/util/parser/math/MathModel.kt --processT Let's examine the example coverage.html report (generated using the sample command in the previous section) to understand its contents. The report should be located at ``coverage_reports/utility/src/main/java/org/oppia/android/util/math/MathTokenizer/coverage.html``. -**Header Section:** At the top of the report, you'll find information including: - -![image](https://github.com/user-attachments/assets/08c4f0a0-51b5-4d8e-a2f3-ff13d8b6cb2f) +The report is divided into two sections: +- Coverage Overview +- Line-by-Line Coverage Breakdown # +### Coverage Overview + +![image](https://github.com/user-attachments/assets/e366af1f-2f64-4f14-a7e4-f7a592688c6c) + +In the top section of the report, you'll find the following details: + - **File Path:** The relative path of the file to the oppia-android codebase. - **Coverage Percentage:** The percentage of code covered by tests. - **Line Coverage Data:** Details on the coverage of individual lines. - **Main Content:** -Visual Coverage Representation: The report visually highlights the coverage status of source code lines. +# -![image](https://github.com/user-attachments/assets/0e36fde3-639b-4874-b809-59d33827388d) +### Line-by-Line Coverage Breakdown - -# +The subsequent section of the report visually represents the coverage status of each line of the source code. - **Red Lines:** Indicate lines not covered by test cases. - **Green Lines:** Represent lines that are covered by test cases. +![image](https://github.com/user-attachments/assets/c1d55cf8-94bf-4ab5-a02b-c95264e854db) + These generated html reports are be used to identify areas of your code that may need additional testing. ## Increasing Code Coverage Metrics -While figuring out which lines are covered and which are not, you can increase code coverage by ensuring that each line is thoroughly tested. To enhance code coverage effectiveness, locate the test files associated with the source file (typically found in the same package but within the test directories). +To improve code coverage, start by identifying which lines are covered and which are not. To effectively increase coverage, trace the uncovered lines back to their calling functions, and ensure these are executed and tested in your test suites. The corresponding test files for the source code are usually located in the same package within the test directories. Add tests that cover all relevant behavioral scenarios for the uncovered lines to achieve comprehensive testing. + +For more guidance on best practices, refer to the [Writing Tests with Good Behavioural Coverage](https://github.com/oppia/oppia-android/wiki/Writing-Tests-With-Good-Behavioural-Coverage) wiki page. + +Note: Some files in the app module may have multiple test files, located in the sharedTest and test packages, all testing a single source file. For example: [StateFragment.kt](https://github.com/oppia/oppia-android/blob/develop/app/src/main/java/org/oppia/android/app/player/state/StateFragment.kt) has 2 test files [StateFragmentTest.kt](https://github.com/oppia/oppia-android/blob/develop/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt) under ``app/src/sharedTest`` and [StateFragmentLocalTest.kt](https://github.com/oppia/oppia-android/blob/develop/app/src/test/java/org/oppia/android/app/player/state/StateFragmentLocalTest.kt) under ``app/src/test``. + +## Unit-Centric Coverage Philosophy + +Oppia Android's approach to code coverage is focused on analyzing each source unit within the context of its own tests. This means that only the tests specifically written for a particular source file are considered when determining that file's coverage. Incidental coverage—when a source file is indirectly tested through another test file—is not counted towards the coverage metrics of the original file. + +This approach ensures that coverage percentages are more meaningful, as they reflect deliberate and thorough testing of each source file. This makes it essential to write comprehensive tests for every source unit to achieve accurate and reliable coverage metrics. + +### Example Scenario + +To clarify the concept, consider the following example with source files and their corresponding test files: + +| **Source File** | **Tested By** | **Test File** | +|------------------------------|:-------------:|--------------------------------| +| `OppiaCommentBot.kt` | -> |`OppiaCommentBotTest.kt` | +| `UploadReviewComment.kt` | -> |`UploadReviewCommentTest.kt` | + +**OppiaCommentBot.kt (Source file):** + +It manages the logic for uploading comments and interacting with the Oppia project’s comment system. + +``` +class OppiaCommentBot { + fun uploadComment(comment: String): String { + // Uploads a comment to a repository + return "Comment uploaded: $comment" + } +} +``` + +**OppiaCommentBotTest.kt (Test file for OppiaCommentBot):** + +``` +import org.junit.Test +import com.google.common.truth.Truth.assertThat + +class OppiaCommentBotTest { + @Test + fun testUploadComment_returnsExpectedMessage() { + val bot = OppiaCommentBot() + val result = bot.uploadComment("Great job!") + assertThat(result).isEqualTo("Comment uploaded: Great job!") + } +} +``` + +**UploadReviewComment.kt (Another Source file):** + +It handles the creation and submission of review comments, utilizing features from OppiaCommentBot.kt. + +``` +class UploadReviewComment { + fun createAndUploadReview(comment: String) { + // generates review + val bot = OppiaCommentBot() + bot.uploadComment(review) + } +} +``` + +**UploadReviewCommentTest.kt (Test file for UploadReviewComment):** + +``` +import org.junit.Test + +class UploadReviewCommentTest { + @Test + fun testCreateAndUploadReview_callsUploadComment() { + val review = UploadReviewComment() + review.createAndUploadReview("Needs revision") + } +} +``` -Note: Some files in the app module may have multiple test files, located in the sharedTest and test packages, all testing a single source file. (eg: The file StateFragment.kt has 2 test files StateFragmentTest.kt under sharedTest and StateFragmentLocalTest.kt under test folder) +In this example, the `OppiaCommentBot` class is used in both `UploadReviewComment.kt` and `OppiaCommentBotTest.kt`. However, the code coverage tool only considers the tests in `OppiaCommentBotTest.kt` when calculating the coverage for `OppiaCommentBot.kt`. Tests in `UploadReviewCommentTest.kt` that indirectly call `OppiaCommentBot` do not count toward its coverage. -By identifying and adding appropriate test scenarios to cover previously uncovered lines, you will boost the coverage percentage and improve the overall quality and reliability of your code. +It’s essential to ensure that each source file is directly tested by its own corresponding test file to accurately reflect the unit's coverage. This approach helps maintain a high standard of code quality and ensures that the coverage metrics genuinely represent the code’s reliability. ## Limitations of the code coverage tool From 15e1033976ea754a94b2ad01fb28fe3af19b0d75 Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 01:58:05 +0530 Subject: [PATCH 10/64] Testing from forked repo to see if permissions work --- .../oppia/android/scripts/coverage/reporter/CoverageReporter.kt | 1 + 1 file changed, 1 insertion(+) 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 b27ec9283ce..2d668e6bf05 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 @@ -690,3 +690,4 @@ private fun loadTestFileExemptionsProto( }.build() } } + From 1cad5c645f83ddd06d39de052144aaef8318ce3e Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 02:38:11 +0530 Subject: [PATCH 11/64] Replacing pull request with pull request target --- .github/workflows/code_coverage.yml | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 02d9908f12d..89e48967989 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -5,7 +5,9 @@ name: Code Coverage # Controls when the action will run. Triggers the workflow on pull request # events or push events in the develop branch. on: - pull_request: + pull_request_target +#on: +# pull_request: push: branches: # Push events on develop branch @@ -16,19 +18,19 @@ concurrency: cancel-in-progress: true jobs: - check_unit_tests_completed: - name: Check unit test completed - runs-on: ubuntu-latest - steps: - - name: Wait for unit tests to checks - uses: ArcticLampyrid/action-wait-for-workflow@v1.2.0 - with: - workflow: unit_tests.yml - sha: auto +# check_unit_tests_completed: +# name: Check unit test completed +# runs-on: ubuntu-latest +# steps: +# - name: Wait for unit tests to checks +# uses: ArcticLampyrid/action-wait-for-workflow@v1.2.0 +# with: +# workflow: unit_tests.yml +# sha: auto compute_changed_files: name: Compute changed files - needs: check_unit_tests_completed +# needs: check_unit_tests_completed runs-on: ubuntu-20.04 outputs: matrix: ${{ steps.compute-file-matrix.outputs.matrix }} From f9b4b0da8051d560ed8794be01a78f6528ff3277 Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 02:56:27 +0530 Subject: [PATCH 12/64] On pull request target with opened, synchronize and reopened --- .github/workflows/code_coverage.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 89e48967989..3d736216c60 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -5,7 +5,8 @@ name: Code Coverage # Controls when the action will run. Triggers the workflow on pull request # events or push events in the develop branch. on: - pull_request_target + pull_request_target: + types: [opened, synchronize, reopened] #on: # pull_request: push: From 6ec17904baf406332ea71408ddab7299ea4c8b89 Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 03:07:33 +0530 Subject: [PATCH 13/64] Trying to fix syntax error with workflow --- .github/workflows/code_coverage.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 3d736216c60..a495d567aad 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -7,8 +7,6 @@ name: Code Coverage on: pull_request_target: types: [opened, synchronize, reopened] -#on: -# pull_request: push: branches: # Push events on develop branch From a6ce3753dd7101a1e39708bc9c3a91f256221aa7 Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 03:12:47 +0530 Subject: [PATCH 14/64] Remove comments --- .github/workflows/code_coverage.yml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index a495d567aad..3e29262cff4 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -1,15 +1,10 @@ -# Contains jobs corresponding to code coverage. - name: Code Coverage -# Controls when the action will run. Triggers the workflow on pull request -# events or push events in the develop branch. on: pull_request_target: types: [opened, synchronize, reopened] push: branches: - # Push events on develop branch - develop concurrency: From 195e34d289a52222cb9eeb59733543ebbaa15703 Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 03:21:51 +0530 Subject: [PATCH 15/64] With just pr target set --- .github/workflows/code_coverage.yml | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 3e29262cff4..9a1759f6d8f 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -1,11 +1,17 @@ +# Contains jobs corresponding to code coverage. + name: Code Coverage +# Controls when the action will run. Triggers the workflow on pull request +# events or push events in the develop branch. on: pull_request_target: types: [opened, synchronize, reopened] - push: - branches: - - develop + +# push: +# branches: +# # Push events on develop branch +# - develop concurrency: group: ${{ github.workflow }}-${{ github.ref }} From 3bc2729e56c94a7df61f38a4b543bba0a351740c Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 03:23:15 +0530 Subject: [PATCH 16/64] Only on push --- .github/workflows/code_coverage.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 9a1759f6d8f..d22cd4661cd 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -5,13 +5,13 @@ name: Code Coverage # Controls when the action will run. Triggers the workflow on pull request # events or push events in the develop branch. on: - pull_request_target: - types: [opened, synchronize, reopened] +# pull_request_target: +# types: [opened, synchronize, reopened] -# push: -# branches: -# # Push events on develop branch -# - develop + push: + branches: + # Push events on develop branch + - develop concurrency: group: ${{ github.workflow }}-${{ github.ref }} From 3f9ee1e326b1f3137e21916fb9f1a652e9dc8a6f Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 03:27:34 +0530 Subject: [PATCH 17/64] Reverting to the pull_request --- .github/workflows/code_coverage.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index d22cd4661cd..7ef46de6680 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -4,10 +4,11 @@ name: Code Coverage # Controls when the action will run. Triggers the workflow on pull request # events or push events in the develop branch. -on: +#on: # pull_request_target: # types: [opened, synchronize, reopened] - +on: + pull_request: push: branches: # Push events on develop branch From 5c993dc2ffe079a2750287173ead2afe907c4127 Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 03:37:13 +0530 Subject: [PATCH 18/64] With just pr target --- .github/workflows/code_coverage.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 7ef46de6680..e2d9bafd359 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -8,11 +8,11 @@ name: Code Coverage # pull_request_target: # types: [opened, synchronize, reopened] on: - pull_request: - push: - branches: - # Push events on develop branch - - develop + pull_request:_target: +# push: +# branches: +# # Push events on develop branch +# - develop concurrency: group: ${{ github.workflow }}-${{ github.ref }} From a11422f2b7480aad849815491420dd1b16f09819 Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 08:36:08 +0530 Subject: [PATCH 19/64] Pull request target --- .github/workflows/code_coverage.yml | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index e2d9bafd359..bdda7ec0ce3 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -4,15 +4,10 @@ name: Code Coverage # Controls when the action will run. Triggers the workflow on pull request # events or push events in the develop branch. -#on: -# pull_request_target: -# types: [opened, synchronize, reopened] on: - pull_request:_target: -# push: -# branches: -# # Push events on develop branch -# - develop + pull_request_target: + branches: + - develop concurrency: group: ${{ github.workflow }}-${{ github.ref }} From 65345eee6daf330bc2c209adbb8f41c3557046a9 Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 08:42:43 +0530 Subject: [PATCH 20/64] Adding push trigger --- .github/workflows/code_coverage.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index bdda7ec0ce3..ab25fb794c1 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -6,8 +6,9 @@ name: Code Coverage # events or push events in the develop branch. on: pull_request_target: - branches: - - develop + push: + branches: + - develop concurrency: group: ${{ github.workflow }}-${{ github.ref }} From f9455dcc91f249764055e30b5d121c0dc1a4037b Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 08:46:11 +0530 Subject: [PATCH 21/64] Moving the nested pull --- .github/workflows/code_coverage.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index ab25fb794c1..0b008bc2559 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -6,9 +6,11 @@ name: Code Coverage # events or push events in the develop branch. on: pull_request_target: - push: - branches: - - develop + branches: + - develop + push: + branches: + - develop concurrency: group: ${{ github.workflow }}-${{ github.ref }} From dad785859a098aace0e43a1699f99ccbc5e41519 Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 08:59:39 +0530 Subject: [PATCH 22/64] Pull Request target labels --- .github/workflows/code_coverage.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 0b008bc2559..5e6669fc358 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -6,8 +6,7 @@ name: Code Coverage # events or push events in the develop branch. on: pull_request_target: - branches: - - develop + types: [opened, synchronize, reopened] push: branches: - develop From d6ed539cb62730c045ce3efe74bc9cd59f291609 Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 09:02:07 +0530 Subject: [PATCH 23/64] Removing push trigger --- .github/workflows/code_coverage.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 5e6669fc358..9af841cea5d 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -7,9 +7,6 @@ name: Code Coverage on: pull_request_target: types: [opened, synchronize, reopened] - push: - branches: - - develop concurrency: group: ${{ github.workflow }}-${{ github.ref }} From bf845b73e662f56456a091e38a666b9b7a8eae72 Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 09:07:42 +0530 Subject: [PATCH 24/64] Adding Permissions to write --- .github/workflows/code_coverage.yml | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 9af841cea5d..251617b8292 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -4,9 +4,18 @@ name: Code Coverage # Controls when the action will run. Triggers the workflow on pull request # events or push events in the develop branch. +#on: +# pull_request_target: +# types: [opened, synchronize, reopened] on: - pull_request_target: - types: [opened, synchronize, reopened] + pull_request: + push: + branches: + # Push events on develop branch + - develop + +permissions: + pull-requests: write concurrency: group: ${{ github.workflow }}-${{ github.ref }} From b8ca56b8e9cae368acd49dab309d62cb2a5e0b07 Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 09:26:02 +0530 Subject: [PATCH 25/64] To try with manual triggers --- .github/workflows/code_coverage.yml | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 251617b8292..0f6b3794558 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -2,17 +2,20 @@ name: Code Coverage +on: + workflow_dispatch: + # Controls when the action will run. Triggers the workflow on pull request # events or push events in the develop branch. #on: # pull_request_target: # types: [opened, synchronize, reopened] -on: - pull_request: - push: - branches: - # Push events on develop branch - - develop +#on: +# pull_request: +# push: +# branches: +# # Push events on develop branch +# - develop permissions: pull-requests: write From 8c67fff0cc2db4f3cec4bfc870e994ac6fec1488 Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 09:42:45 +0530 Subject: [PATCH 26/64] Do versions make any changes --- .github/workflows/code_coverage.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 0f6b3794558..fd4aba8d633 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -333,7 +333,7 @@ jobs: name: final-coverage-report - name: Upload Coverage Report as PR Comment - uses: peter-evans/create-or-update-comment@v4 + uses: peter-evans/create-or-update-comment@v1 with: issue-number: ${{ github.event.pull_request.number }} body-path: 'CoverageReport.md' From 813ff66e85e6d08b1aade7319fd13118a816cc13 Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 09:57:40 +0530 Subject: [PATCH 27/64] Manually providing the issue number --- .github/workflows/code_coverage.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index fd4aba8d633..05c3111c737 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -335,7 +335,7 @@ jobs: - name: Upload Coverage Report as PR Comment uses: peter-evans/create-or-update-comment@v1 with: - issue-number: ${{ github.event.pull_request.number }} + issue-number: 5483 body-path: 'CoverageReport.md' # Reference: https://github.community/t/127354/7. From a5aeb129af03e8b86d95e876d26c42acb1befb56 Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 15 Aug 2024 10:26:27 +0530 Subject: [PATCH 28/64] Pull Request target --- .github/workflows/code_coverage.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 05c3111c737..8dab2b5b51d 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -4,6 +4,9 @@ name: Code Coverage on: workflow_dispatch: + pull_request_target: + types: [ opened, synchronize, reopened ] + # Controls when the action will run. Triggers the workflow on pull request # events or push events in the develop branch. @@ -335,13 +338,13 @@ jobs: - name: Upload Coverage Report as PR Comment uses: peter-evans/create-or-update-comment@v1 with: - issue-number: 5483 + issue-number: ${{ github.event.pull_request.number }} body-path: 'CoverageReport.md' # 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: [ compute_changed_files, code_coverage_run, publish_coverage_report, 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() }} From b58db863ef1f6b6f0e96465c261de7728baec4db Mon Sep 17 00:00:00 2001 From: Rd Date: Fri, 16 Aug 2024 22:49:05 +0530 Subject: [PATCH 29/64] Addressing Review part 1 - Exception messages and fixes Save lobby since prioritizing fixing Fork restriction fixes --- wiki/Oppia-Android-Code-Coverage.md | 58 +- ...ng-tests-with-good-behavioural-coverage.md | 942 ++++++++++-------- 2 files changed, 558 insertions(+), 442 deletions(-) diff --git a/wiki/Oppia-Android-Code-Coverage.md b/wiki/Oppia-Android-Code-Coverage.md index ea80425eaff..b230df5e784 100644 --- a/wiki/Oppia-Android-Code-Coverage.md +++ b/wiki/Oppia-Android-Code-Coverage.md @@ -24,11 +24,11 @@ Consider a Kotlin function designed to determine if a number is positive or nega ```kotlin fun checkSign(n: Int): String { - return if (n >= 0) { - "Positive" - } else { - "Negative" - } + return if (n >= 0) { + "Positive" + } else { + "Negative" + } } ``` @@ -397,28 +397,28 @@ To clarify the concept, consider the following example with source files and the It manages the logic for uploading comments and interacting with the Oppia project’s comment system. -``` +```kotlin class OppiaCommentBot { - fun uploadComment(comment: String): String { - // Uploads a comment to a repository - return "Comment uploaded: $comment" - } + fun uploadComment(comment: String): String { + // Uploads a comment to a repository + return "Comment uploaded: $comment" + } } ``` **OppiaCommentBotTest.kt (Test file for OppiaCommentBot):** -``` +```kotlin import org.junit.Test import com.google.common.truth.Truth.assertThat class OppiaCommentBotTest { - @Test - fun testUploadComment_returnsExpectedMessage() { - val bot = OppiaCommentBot() - val result = bot.uploadComment("Great job!") - assertThat(result).isEqualTo("Comment uploaded: Great job!") - } + @Test + fun testUploadComment_returnsExpectedMessage() { + val bot = OppiaCommentBot() + val result = bot.uploadComment("Great job!") + assertThat(result).isEqualTo("Comment uploaded: Great job!") + } } ``` @@ -426,27 +426,27 @@ class OppiaCommentBotTest { It handles the creation and submission of review comments, utilizing features from OppiaCommentBot.kt. -``` +```kotlin class UploadReviewComment { - fun createAndUploadReview(comment: String) { - // generates review - val bot = OppiaCommentBot() - bot.uploadComment(review) - } + fun createAndUploadReview(comment: String) { + // generates review + val bot = OppiaCommentBot() + bot.uploadComment(review) + } } ``` **UploadReviewCommentTest.kt (Test file for UploadReviewComment):** -``` +```kotlin import org.junit.Test class UploadReviewCommentTest { - @Test - fun testCreateAndUploadReview_callsUploadComment() { - val review = UploadReviewComment() - review.createAndUploadReview("Needs revision") - } + @Test + fun testCreateAndUploadReview_callsUploadComment() { + val review = UploadReviewComment() + review.createAndUploadReview("Needs revision") + } } ``` diff --git a/wiki/Writing-tests-with-good-behavioural-coverage.md b/wiki/Writing-tests-with-good-behavioural-coverage.md index b2c113d57d3..0fb4795967b 100644 --- a/wiki/Writing-tests-with-good-behavioural-coverage.md +++ b/wiki/Writing-tests-with-good-behavioural-coverage.md @@ -26,20 +26,20 @@ Focusing on behavioral coverage is essential because: Let's understand it better with a sample function. Consider this function that validates a name: -``` +```kotlin fun getName(name: String? = " ") = - name?.takeIf { it.all { char -> char.isLetterOrWhitespace() } } - ?: throw IllegalArgumentException("Invalid name") + name?.takeIf { it.all { char -> char.isLetterOrWhitespace() } } + ?: throw IllegalArgumentException("Invalid name") ``` ### Line Coverage Testing A basic test case for line coverage might look like this: -``` +```kotlin @Test fun testValidName() { - // Tests line coverage by hitting the line where name is accessed - assertThat(getName("Alice")).isEqualTo("Alice") + // Tests line coverage by hitting the line where name is accessed + assertThat(getName("Alice"), equalTo("Alice")) } ``` @@ -49,30 +49,30 @@ A basic test case for line coverage might look like this: To ensure behavioral coverage, the test needs to verify various conditions: -``` +```kotlin @Test fun testGetName_withDefaultValue_result(getName()) { - // Default value when no name is provided - assertThat(getName()).isEqualTo(" ") + // Default value when no name is provided + assertThat(getName(), equalTo(" ")) } @Test fun testGetName_withNullName_throwsException() { - // Exception for null value - assertThrows { getName(null) } + // Exception for null value + assertThrows { getName(null) } } @Test fun testGetName_withSpecialCharacters_throwsException() { - // Exception for special characters - assertThrows { getName("!@#$%^&*()") } + // Exception for special characters + assertThrows { getName("!@#$%^&*()") } } @Test fun testGetName_withEmptyName_result(getName("")) { - // Empty string should use default value - assertThat(getName("")).isEqualTo(" ") + // Empty string should use default value + assertThat(getName(""), equalTo(" ")) } @Test fun testGetName_withWhitespaceName_result(getName(" ")) { - // Whitespace name - assertThat(getName(" ")).isEqualTo(" ") + // Whitespace name + assertThat(getName(" "), equalTo(" ")) } ``` @@ -82,7 +82,7 @@ To ensure behavioral coverage, the test needs to verify various conditions: While line coverage might reach 100% with a single test, it doesn’t ensure that all scenarios are tested. Behavioral coverage ensures quality by validating all possible scenarios and edge cases, which is more important for reliable software. -For more details on testing methodologies specific to Oppia Android, please refer to the [Oppia Android Testing wiki page](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Testing). +For more details on testing methodologies specific to Oppia Android, please refer to the [Oppia Android Testing](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Testing). # Writing Effective Tests @@ -108,9 +108,9 @@ Before you start writing tests, it's essential to thoroughly understand the requ The function to determine if an item should be bought based on the price and budget is, -``` +```kotlin fun shouldBuyItem(price: Double, budget: Double): Boolean { - return price <= budget + return price <= budget } ``` @@ -118,15 +118,15 @@ fun shouldBuyItem(price: Double, budget: Double): Boolean { To ensure that the shouldBuyItem function works correctly, we can add the following test cases, -``` +```kotlin @Test fun testShouldBuyItem_withPriceWithinBudget_returnsTrue() { - assertTrue(shouldBuyItem(50.0, 100.0)) + assertThat(shouldBuyItem(50.0, 100.0)).isTrue() } @Test fun testShouldBuyItem_withPriceAboveBudget_returnsFalse() { - assertFalse(shouldBuyItem(150.0, 100.0)) + assertThat(shouldBuyItem(150.0, 100.0)).isFalse() } ``` @@ -138,12 +138,12 @@ Each test case should: - Use descriptive names for your test methods. Naming Convention: -``` +```kotlin testAction_withOneCondition_withSecondCondition_hasExpectedOutcome ``` Example: -``` +```kotlin testCheckSign_forPositiveInput_returnsPositive() testCheckSign_forNegativeInput_returnsNegative() testCheckSign_forZeroInput_returnsNeitherPositiveNorNegative() @@ -167,20 +167,20 @@ Consider a function that manages a food order process. This function does the fo 3. Display the order. 4. Checks if the payment has been made and provides the corresponding message. -``` +```kotlin fun processOrder(order: List, paymentMade: Boolean): String { - // List the food items - val itemList = order.joinToString(", ") + // List the food items + val itemList = order.joinToString(", ") - // Calculate total price (mocked here for simplicity) - val totalPrice = order.size * 10.0 + // Calculate total price (mocked here for simplicity) + val totalPrice = order.size * 10.0 - // Display order - println("Order: $itemList") - println("Total: $totalPrice") + // Display order + println("Order: $itemList") + println("Total: $totalPrice") - // Payment status - return if (paymentMade) "Payment successful" else "Payment pending" + // Payment status + return if (paymentMade) "Payment successful" else "Payment pending" } ``` @@ -196,11 +196,11 @@ Result: Payment successful **Single Test Case:** -``` +```kotlin @Test fun testProcessOrder_allSteps_returnsCorrectMessage() { - val result = processOrder(listOf("Pizza", "Burger"), paymentMade = true) - assertThat(result).isEqualTo("Payment successful") + val result = processOrder(listOf("Pizza", "Burger"), paymentMade = true) + assertThat(result).isEqualTo("Payment successful") } ``` @@ -213,45 +213,45 @@ fun testProcessOrder_allSteps_returnsCorrectMessage() { **Test Case 1: Testing Listing items** -``` +```kotlin @Test -fun testProcessOrder_listsItems_correctly() { - processOrder(listOf("Pizza", "Burger"), paymentMade = true) - val output = outContent.toString().trim() - assertThat(output, containsString("Order: Pizza, Burger")) +fun testProcessOrder_providedWithList_displaysListOfItems() { + processOrder(listOf("Pizza", "Burger"), paymentMade = true) + val output = outContent.toString().trim() + assertThat(output, containsString("Order: Pizza, Burger")) } ``` **Test Case 2: Calculates Total** -``` +```kotlin @Test -fun testProcessOrder_calculatesTotal_correctly() { - processOrder(listOf("Pizza", "Burger"), paymentMade = true) - val output = outContent.toString().trim() - assertThat(output).contains("Total: 20.0") +fun testProcessOrder_forListItems_calculatesCorrectTotalPrice() { + processOrder(listOf("Pizza", "Burger"), paymentMade = true) + val output = outContent.toString().trim() + assertThat(output, containsString("Total: 20.0")) } ``` **Test Case 3: Payment Success** -``` +```kotlin @Test -fun testProcessOrder_paymentSuccessful() { - processOrder(listOf("Pizza", "Burger"), paymentMade = true) - val output = outContent.toString().trim() - assertThat(output).contains("Payment successful") +fun testProcessOrder_whenPaymentMade_displaysPaymentSuccess() { + processOrder(listOf("Pizza", "Burger"), paymentMade = true) + val output = outContent.toString().trim() + assertThat(output, containsString("Payment successful")) } ``` **Test Case 4: Payment Pending** -``` +```kotlin @Test -fun testProcessOrder_paymentPending() { - processOrder(listOf("Pizza", "Burger"), paymentMade = false) - val output = outContent.toString().trim() - assertThat(output).contains("Payment pending") +fun testProcessOrder_whenNotPaymentMade_displaysPaymentPending() { + processOrder(listOf("Pizza", "Burger"), paymentMade = false) + val output = outContent.toString().trim() + assertThat(output, containsString("Payment pending")) } ``` @@ -267,13 +267,13 @@ To ensure robust testing, it's crucial to cover various scenarios your function Consider a function `checkSign` that takes an integer and returns a string indicating whether the number is positive, negative, or zero. -``` +```kotlin fun checkSign(number: Int): String { - return when { - number > 0 -> "Positive" - number < 0 -> "Negative" - else -> "Zero" - } + return when { + number > 0 -> "Positive" + number < 0 -> "Negative" + else -> "Zero" + } } ``` @@ -281,46 +281,46 @@ fun checkSign(number: Int): String { Positive Number: Verifies that the function correctly identifies positive numbers. -``` +```kotlin @Test fun testCheckNumber_forPositiveInput_returnsPositive() { - assertThat(checkNumber(5)).isEqualTo("Positive") + assertThat(checkNumber(5), equalTo("Positive")) } ``` Negative Number: Ensures that negative numbers are correctly classified. -``` +```kotlin @Test fun testCheckNumber_forNegativeInput_returnsNegative() { - assertThat(checkNumber(-3)).isEqualTo("Negative") + assertThat(checkNumber(-3), equalTo("Negative")) } ``` Zero: Checks that zero is handled correctly. -``` +```kotlin @Test fun testCheckNumber_forZeroInput_returnsZero() { - assertThat(checkNumber(0)).isEqualTo("Zero") + assertThat(checkNumber(0), equalTo("Zero")) } ``` Maximum Value: Tests the function with Int.MAX_VALUE to ensure it handles the upper boundary. -``` +```kotlin @Test fun testCheckNumber_forMaxValue_returnsPositive() { - assertThat(checkNumber(Int.MAX_VALUE)).isEqualTo("Positive") + assertThat(checkNumber(Int.MAX_VALUE), equalTo("Positive")) } ``` Minimum Value: Tests the function with Int.MIN_VALUE to ensure it handles the lower boundary. -``` +```kotlin @Test fun testCheckNumber_forMinValue_returnsNegative() { - assertThat(checkNumber(Int.MIN_VALUE)).isEqualTo("Negative") + assertThat(checkNumber(Int.MIN_VALUE), equalTo("Negative")) } ``` @@ -330,19 +330,19 @@ Testing all branches, paths, and conditions within your code is essential to ens Let's see the function to evaluate a user's access level based on their age and membership status. -``` +```kotlin fun evaluateAccess(age: Int, isMember: Boolean): String { - var result: String - - if (age >= 18 && isMember) { - result = "Access granted" - } else if (age >= 18 && !isMember) { - result = "Membership required" - } else { - result = "Access denied" - } + var result: String + + if (age >= 18 && isMember) { + result = "Access granted" + } else if (age >= 18 && !isMember) { + result = "Membership required" + } else { + result = "Access denied" + } - return result + return result } ``` @@ -357,25 +357,25 @@ The different scenarios and the expected outcomes are, Testing needs to be performed to cover all branches, paths and conditions. -``` +```kotlin @Test -fun testEvaluateAccess_adultMember() { - assertThat(evaluateAccess(25, true)).isEqualTo("Access granted") +fun testEvaluateAccess_forAdultMember_grantsAccess() { + assertThat(evaluateAccess(25, true), equalTo("Access granted")) } @Test -fun testEvaluateAccess_adultNonMember() { - assertThat(evaluateAccess(30, false)).isEqualTo("Membership required") +fun testEvaluateAccess_forAdultNonMember_requiresMembership() { + assertThat(evaluateAccess(30, false), equalTo("Membership required")) } @Test -fun testEvaluateAccess_minorMember() { - assertThat(evaluateAccess(16, true)).isEqualTo("Access denied") +fun testEvaluateAccess_forMinorMember_deniesAccess() { + assertThat(evaluateAccess(16, true), equalTo("Access denied")) } @Test -fun testEvaluateAccess_minorNonMember() { - assertThat(evaluateAccess(15, false)).isEqualTo("Access denied") +fun testEvaluateAccess_forminorNonMember_deniesAccess() { + assertThat(evaluateAccess(15, false), equalTo("Access denied")) } ``` @@ -396,10 +396,10 @@ This allows to catch and handle issues, allowing the program to continue running For instance, to handle division by zero error. -``` +```kotlin fun divide(a: Int, b: Int): Int { - if (b == 0) throw ArithmeticException("Division by zero") - return a / b + if (b == 0) throw ArithmeticException("Division by zero") + return a / b } ``` @@ -411,9 +411,9 @@ This allows to manage situations where the application may not continue running For instance, -``` +```kotlin fun simulateError() { - val arr = IntArray(Int.MAX_VALUE) // May cause OutOfMemoryError + val arr = IntArray(Int.MAX_VALUE) // May cause OutOfMemoryError } ``` @@ -423,98 +423,192 @@ Ensure that the code handles invalid or unexpected inputs gracefully and verify **Functionality:** -``` +```kotlin fun divideNumbers(numerator: Int, denominator: Int): Int { - if (denominator == 0) throw IllegalArgumentException("Denominator cannot be zero") - return numerator / denominator + if (denominator == 0) throw IllegalArgumentException("Denominator cannot be zero") + return numerator / denominator } ``` **Test Case:** -``` +```kotlin @Test fun testDivideNumbers_forZeroDenominator_throwsIllegalArgumentException() { - val exception = assertThrows { - divideNumbers(10, 0) - } - assertThat(exception).contains("Denominator cannot be zero") + val exception = assertThrows { + divideNumbers(10, 0) + } + assertThat(exception).contains("Denominator cannot be zero") } ``` Testing exceptions and error handling is vital to ensure that applications behave correctly under error conditions, provide meaningful feedback, and maintain reliability. Without these tests, applications are prone to unpredictable failures, poor user experiences, and potential security issues. -### Verifying Correct Exception Types +### 1. Verifying Correct Exception Types Ensure that the correct type of exception is thrown in response to error conditions. This confirms that your error handling logic is specific and accurate. -``` +```kotlin @Test fun testDivideNumbers_forZeroDenominator_throwsIllegalArgumentException() { - assertThrows { - divideNumbers(10, 0) - } + assertThrows { + divideNumbers(10, 0) + } } ``` -### Checking for Proper Exception Messages +### 2. Checking for Proper Exception Messages -It's crucial that exceptions contain meaningful messages that help diagnose issues. Verify that the exception messages are informative and relevant. +When writing tests, it's crucial to ensure that exceptions thrown by your code contain meaningful and descriptive messages. These messages play a vital role in diagnosing issues, providing clarity on what went wrong and why it happened. +Let's consider a TicketBooking class that has a function to reserve a seat. This function checks if seats are available using Kotlin's check() function. If no seats are available, it throws an exception with a specific message. + +**Functionality:** + +```kotlin +class TicketBooking { + fun reserveSeat(seatsAvailable: Int) { + check(seatsAvailable > 0) { + "No seats are available. Please check other bookings for available seats." + } + // Additional code to reserve a seat + } +} ``` + +In this case, when the **seatsAvailable** becomes 0, the `check()` function will throw an `IllegalStateException` with the message "No seats are available. Please check other bookings for available seats." + +**Test:** + +To verify that the exception is thrown with the correct message, we write a test case: + +```kotlin @Test -fun testDivideNumbers_forZeroDenominator_containsCorrectMessage() { - val exception = assertThrows { - divideNumbers(10, 0) +fun testBookTickets_withUnavailableSeats_throwsException() { + val booking = TicketBooking() + val exception = assertThrows { + booking.reserveSeat(0) + } + assertThat(exception).contains("No seats are available. Please check other bookings for available seats.") +} +``` + +This test case checks that when no seats are available, the reserveSeat() function throws an `IllegalStateException` with the appropriate message. + +### Why verify Exception Messages? + +Verifying exception messages is crucial for ensuring the correctness and usefulness of your tests. This is especially important when dealing with generic exceptions, where multiple checks might throw the same type of exception. + +To better understand this, let's extend the `TicketBooking` class with an additional function that checks the payment status before confirming a booking. This function uses `check()` to verify if the payment was successful. If not, it throws an exception with a specific message. + +**Extended Functionality:** + +```kotlin +class TicketBooking { + fun reserveSeat(seatsAvailable: Int) { + check(seatsAvailable > 0) { + "No seats are available. Please check other bookings for available seats." + } + // Additional code to reserve a seat + } + + fun confirmPayment(isPaymentSuccessful: Boolean) { + check(isPaymentSuccessful) { + "Payment not successful. Please try again." } - assertThat(exception.message).contains("Denominator cannot be zero") + // Additional code to confirm payment + } } ``` -### Ensuring Exceptions Are Thrown at Correct Times +In this scenario, the `confirmPayment()` function throws an `IllegalStateException` if the payment is not successful, with the message "Payment not successful. Please try again." -Exceptions should be thrown only under specific conditions, not during normal operations. Verify that exceptions are correctly managed according to the context. +### Importance of Specific Error Messages + +Imagine if both checks in the `reserveSeat()` and `confirmPayment()` functions used generic messages like "Error occured" as: + +```kotlin +check(seatsAvailable > 0) { "Error occurred" } +check(isPaymentSuccessful) { "Error occurred" } +``` + +In this case, when an exception is thrown, it becomes very challenging to determine the exact cause of the error. Did the error occur because there were no seats available, or because the payment was not successful? This ambiguity can make debugging difficult and reduce the effectiveness of your tests. + +That is why it is necessary to test that each exception throws a specific error message relevant to its particular scenario—to help accurately diagnose where things went wrong. Consider the following test cases: + +**Test for Seat Availability:** + +```kotlin +@Test +fun testBookTickets_withUnavailableSeats_throwsException() { + val booking = TicketBooking() + val exception = assertThrows { + booking.reserveSeat(0) + } + assertThat(exception).contains("No seats are available. Please check other bookings for available seats.") +} +``` + +This test case ensures that when no seats are available, the correct exception with the appropriate message is thrown. + +**Test for Payment Status:** + +```kotlin +@Test +fun testConfirmPayment_withUnsuccessfulPayment_throwsException() { + val booking = TicketBooking() + val exception = assertThrows { + booking.confirmPayment(false) + } + assertThat(exception).contains("Payment not successful. Please try again.") +} ``` + +### 3. Ensuring Exceptions Are Thrown at Correct Times + +Exceptions should be thrown only under specific conditions, not during normal operations. Verify that exceptions are correctly managed according to the context. + +```kotlin @Test fun testDivideNumbers_forValidInputs_returnsExpectedResult() { - val result = divideNumbers(10, 2) - assertThat(result).isEqualTo(5) + val result = divideNumbers(10, 2) + assertThat(result).isEqualTo(5) } ``` -### Testing Edge Cases +### 4. Testing Edge Cases Test edge cases where exceptions might be thrown, such as boundary values or extreme input scenarios. **Function with Edge Case Handling:** -``` +```kotlin fun calculateDiscount(price: Double, discountPercent: Double): Double { - if (price < 0 || discountPercent < 0) { - throw IllegalArgumentException("Price and discount cannot be negative") - } - return price - (price * discountPercent / 100) + if (price < 0 || discountPercent < 0) { + throw IllegalArgumentException("Price and discount cannot be negative") + } + return price - (price * discountPercent / 100) } ``` **Test Case:** -``` +```kotlin @Test fun testCalculateDiscount_forNegativePrice_throwsIllegalArgumentException() { - val exception = assertThrows { - calculateDiscount(-100.0, 10.0) - } - assertThat(exception.message).contains("Price and discount cannot be negative") + val exception = assertThrows { + calculateDiscount(-100.0, 10.0) + } + assertThat(exception).contains("Price and discount cannot be negative") } @Test fun testCalculateDiscount_forNegativeDiscount_throwsIllegalArgumentException() { - val exception = assertThrows { - calculateDiscount(100.0, -10.0) - } - assertThat(exception.message).contains("Price and discount cannot be negative") + val exception = assertThrows { + calculateDiscount(100.0, -10.0) + } + assertThat(exception).contains("Price and discount cannot be negative") } ``` @@ -526,55 +620,55 @@ Public APIs are essential because they provide a way to interact with the functi Let's consider the following example for a public API to withdraw money from the BankAccount. -``` +```kotlin class BankAccount( - private var balance: Double, - private val username: String, - private val password: String + private var balance: Double, + private val username: String, + private val password: String ) { - // Public method to withdraw money - fun withdraw( - requestedUsername: String, // Username provided for the withdrawal - requestedPassword: String, // Password provided for the withdrawal - file: File? = null // Optional passbook file to upload to note transactions - amount: Double, // Amount to withdraw - needReceipt: Boolean = false // Flag to indicate if a receipt is needed, defaults to false - ) { - // Verify user credentials - // Validate withdrawal amount - // Perform the withdrawal operation - // Print a receipt if needed - println("Withdrawing $amount for $requestedUsername") - if (needReceipt) { - printReceipt(amount) - } - - // Process the file if provided - file?.let { - processFile(it) - } + // Public method to withdraw money + fun withdraw( + requestedUsername: String, // Username provided for the withdrawal + requestedPassword: String, // Password provided for the withdrawal + file: File? = null // Optional passbook file to upload to note transactions + amount: Double, // Amount to withdraw + needReceipt: Boolean = false // Flag to indicate if a receipt is needed, defaults to false + ) { + // Verify user credentials + // Validate withdrawal amount + // Perform the withdrawal operation + // Print a receipt if needed + println("Withdrawing $amount for $requestedUsername") + if (needReceipt) { + printReceipt(amount) } - private fun isValidUser(requestedUsername: String, requestedPassword: String): Boolean { - return true + // Process the file if provided + file?.let { + processFile(it) } + } - private fun isValidWithdrawal(amount: Double): Boolean { - return true - } + private fun isValidUser(requestedUsername: String, requestedPassword: String): Boolean { + return true + } - private fun performWithdrawal(amount: Double) { - println("Withdrew $amount. New balance is $balance") - } + private fun isValidWithdrawal(amount: Double): Boolean { + return true + } - private fun printReceipt(amount: Double) { - println("Receipt: Withdrew $amount. Current balance: $balance") - } + private fun performWithdrawal(amount: Double) { + println("Withdrew $amount. New balance is $balance") + } - private fun processFile(file: File) { - println("Processing file: ${file.name}") - } + private fun printReceipt(amount: Double) { + println("Receipt: Withdrew $amount. Current balance: $balance") + } + + private fun processFile(file: File) { + println("Processing file: ${file.name}") + } } ``` @@ -590,15 +684,15 @@ Ensure the necessary setup, such as initializing bank data, is completed before **Test:** -``` +```kotlin @Test fun testWithdraw_noBankData_initializationError() { - val account = BankAccount(null, null, null) // Initialize with null values - - val exception = assertThrows { - account.withdraw("user", "password", 200.0) - } - assertThat(exception.message).contains("Bank data not initialized") + val account = BankAccount(null, null, null) // Initialize with null values + + val exception = assertThrows { + account.withdraw("user", "password", 200.0) + } + assertThat(exception).contains("Bank data not initialized") } ``` @@ -608,18 +702,18 @@ This test ensures that if the bank data is not set up properly, the system throw Verify that the API correctly processes valid input data. -``` +```kotlin @Test fun testWithdraw_validData_outputsCorrectBalance() { - val account = BankAccount(1000.0, "user", "password") - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) + val account = BankAccount(1000.0, "user", "password") + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) - account.withdraw("user", "password", 200.0) + account.withdraw("user", "password", 200.0) - val outputLines = output.toString().trim() - assertThat(outputLines).isEqualTo("Withdrew 200.0. New balance is 800.0") - System.setOut(System.out) + val outputLines = output.toString().trim() + assertThat(outputLines).isEqualTo("Withdrew 200.0. New balance is 800.0") + System.setOut(System.out) } ``` @@ -627,25 +721,25 @@ fun testWithdraw_validData_outputsCorrectBalance() { Ensure the API handles incorrect or invalid data properly. -``` +```kotlin @Test fun testWithdraw_invalidUserName_throwsException() { - val account = BankAccount(1000.0, "user", "password") + val account = BankAccount(1000.0, "user", "password") - val exception = assertThrows { - account.withdraw("invalidUser", "password", 200.0) - } - assertThat(exception).contains("Invalid credentials") + val exception = assertThrows { + account.withdraw("invalidUser", "password", 200.0) + } + assertThat(exception).contains("Invalid credentials") } @Test fun testWithdraw_invalidPassword_throwsException() { - val account = BankAccount(1000.0, "user", "password") + val account = BankAccount(1000.0, "user", "password") - val exception = assertThrows { - account.withdraw("user", "invalidPassword", 200.0) - } - assertThat(exception).contains("Invalid credentials") + val exception = assertThrows { + account.withdraw("user", "invalidPassword", 200.0) + } + assertThat(exception).contains("Invalid credentials") } ``` @@ -653,45 +747,45 @@ fun testWithdraw_invalidPassword_throwsException() { Verify the API's behavior with edge cases and boundary conditions. -``` +```kotlin @Test fun testWithdraw_emptyUsername_throwsException() { - val account = BankAccount(1000.0, "user", "password") + val account = BankAccount(1000.0, "user", "password") - val exception = assertThrows { - account.withdraw("", "password", 200.0) - } - assertThat(exception.message).contains("Username cannot be empty") + val exception = assertThrows { + account.withdraw("", "password", 200.0) + } + assertThat(exception).contains("Username cannot be empty") } @Test fun testWithdraw_emptyBalance_throwsException() { - val account = BankAccount(0.0, "user", "password") + val account = BankAccount(0.0, "user", "password") - val exception = assertThrows { - account.withdraw("user", "password", 200.0) - } - assertThat(exception.message).contains("Insufficient balance") + val exception = assertThrows { + account.withdraw("user", "password", 200.0) + } + assertThat(exception).contains("Insufficient balance") } @Test fun testWithdraw_emptyAmount_throwsException() { - val account = BankAccount(1000.0, "user", "password") + val account = BankAccount(1000.0, "user", "password") - val exception = assertThrows { - account.withdraw("user", "password", 0.0) - } - assertThat(exception.message).contains("Invalid withdrawal amount") + val exception = assertThrows { + account.withdraw("user", "password", 0.0) + } + assertThat(exception).contains("Invalid withdrawal amount") } @Test fun testWithdraw_amountGreaterThanBalance_throwsException() { - val account = BankAccount(1000.0, "user", "password") + val account = BankAccount(1000.0, "user", "password") - val exception = assertThrows { - account.withdraw("user", "password", 1500.0) - } - assertThat(exception.message).contains("Invalid withdrawal amount") + val exception = assertThrows { + account.withdraw("user", "password", 1500.0) + } + assertThat(exception).contains("Invalid withdrawal amount") } ``` @@ -699,44 +793,44 @@ fun testWithdraw_amountGreaterThanBalance_throwsException() { Verify the API's behaviour with different receipt parameter values. -``` +```kotlin @Test fun testWithdraw_withDefaultReceipt_noReceiptPrinted() { - val account = BankAccount(1000.0, "user", "password") - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) + val account = BankAccount(1000.0, "user", "password") + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) - account.withdraw("user", "password", 200.0) + account.withdraw("user", "password", 200.0) - val outputLines = output.toString().trim() - assertThat(outputLines).doesNotContain("Receipt: Withdrew 200.0. Current balance: 800.0") - System.setOut(System.out) + val outputLines = output.toString().trim() + assertThat(outputLines).doesNotContain("Receipt: Withdrew 200.0. Current balance: 800.0") + System.setOut(System.out) } @Test fun testWithdraw_withNeedReceipt_receiptPrinted() { - val account = BankAccount(1000.0, "user", "password") - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) + val account = BankAccount(1000.0, "user", "password") + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) - account.withdraw("user", "password", 200.0, needReceipt = true) + account.withdraw("user", "password", 200.0, needReceipt = true) - val outputLines = output.toString().trim() - assertThat(outputLines).contains("Receipt: Withdrew 200.0. Current balance: 800.0") - System.setOut(System.out) + val outputLines = output.toString().trim() + assertThat(outputLines).contains("Receipt: Withdrew 200.0. Current balance: 800.0") + System.setOut(System.out) } @Test fun testWithdraw_withoutNeedReceipt_noReceiptPrinted() { - val account = BankAccount(1000.0, "user", "password") - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) + val account = BankAccount(1000.0, "user", "password") + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) - account.withdraw("user", "password", 200.0, needReceipt = false) + account.withdraw("user", "password", 200.0, needReceipt = false) - val outputLines = output.toString().trim() - assertThat(outputLines).doesNotContain("Receipt: Withdrew 200.0. Current balance: 800.0") - System.setOut(System.out) + val outputLines = output.toString().trim() + assertThat(outputLines).doesNotContain("Receipt: Withdrew 200.0. Current balance: 800.0") + System.setOut(System.out) } ``` @@ -754,50 +848,50 @@ Testing how an API handles file inputs is crucial because: a. Testing with Invalid File Format -``` +```kotlin @Test fun testWithdraw_withInvalidFileFormat_throwsException() { - val account = BankAccount(1000.0, "user", "password") - val invalidFile = File("invalidFile.txt") // File with an invalid format + val account = BankAccount(1000.0, "user", "password") + val invalidFile = File("invalidFile.txt") // File with an invalid format - val exception = assertThrows { - account.withdraw("user", "password", 200.0, file = invalidFile) - } - assertThat(exception).contains("Invalid file format") + val exception = assertThrows { + account.withdraw("user", "password", 200.0, file = invalidFile) + } + assertThat(exception).contains("Invalid file format") } ``` b. Testing with Unavailable File -``` +```kotlin @Test fun testWithdraw_withUnavailableFile_throwsException() { - val account = BankAccount(1000.0, "user", "password") - val unavailableFile = File("nonExistentFile.pdf") // File that does not exist + val account = BankAccount(1000.0, "user", "password") + val unavailableFile = File("nonExistentFile.pdf") // File that does not exist - val exception = assertThrows { - account.withdraw("user", "password", 200.0, file = unavailableFile) - } - assertThat(exception).contains("File not found") + val exception = assertThrows { + account.withdraw("user", "password", 200.0, file = unavailableFile) + } + assertThat(exception).contains("File not found") } ``` c. Testing with Valid File -``` +```kotlin @Test fun testWithdraw_withValidFile_processesFile() { - val account = BankAccount(1000.0, "user", "password") - val validFile = File("passbook.pdf") - validFile.createNewFile() // Ensure the file exists for the test + val account = BankAccount(1000.0, "user", "password") + val validFile = File("passbook.pdf") + validFile.createNewFile() // Ensure the file exists for the test - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) - account.withdraw("user", "password", 200.0, file = validFile) + account.withdraw("user", "password", 200.0, file = validFile) - assertThat(output.toString().trim()).isEqualTo("Processing file: passbook.pdf") - System.setOut(System.out) + assertThat(output.toString().trim()).isEqualTo("Processing file: passbook.pdf") + System.setOut(System.out) } ``` @@ -811,18 +905,18 @@ In addition to validating correct handling of valid and invalid files, it's also a. Ensure No Processing Message for Missing File -``` +```kotlin @Test fun testWithdraw_withNoFile_producesNoFileProcessingOutput() { - val account = BankAccount(1000.0, "user", "password") + val account = BankAccount(1000.0, "user", "password") - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) - account.withdraw("user", "password", 200.0) // No file provided + account.withdraw("user", "password", 200.0) // No file provided - assertThat(output.toString().trim()).doesNotContain("Processing file") - System.setOut(System.out) + assertThat(output.toString().trim()).doesNotContain("Processing file") + System.setOut(System.out) } ``` @@ -830,17 +924,17 @@ This test ensures that no unwanted methods, such as file processing, are called b. Ensure No Receipt Message for Default Value -``` +```kotlin @Test fun testWithdraw_withDefaultReceipt_noReceiptPrinted() { - val account = BankAccount(1000.0, "user", "password") - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) + val account = BankAccount(1000.0, "user", "password") + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) - account.withdraw("user", "password", 200.0) // Using default receipt flag (false) + account.withdraw("user", "password", 200.0) // Using default receipt flag (false) - assertThat(output.toString().trim()).doesNotContain("Receipt:") - System.setOut(System.out) + assertThat(output.toString().trim()).doesNotContain("Receipt:") + System.setOut(System.out) } ``` @@ -854,16 +948,16 @@ When testing a single outcome like a successful withdrawal, you can use multiple Verifies that after withdrawing $200, the balance is updated to $800. This checks that the core functionality of updating the balance works correctly. -``` +```kotlin @Test fun testWithdraw_withSufficientBalance_updatesBalance() { - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) - account.withdraw("user", "password", 200.0) + account.withdraw("user", "password", 200.0) - assertThat(output.toString().trim()).isEqualTo("Withdrew 200.0. New balance is 800.0") - System.setOut(System.out) + assertThat(output.toString().trim()).isEqualTo("Withdrew 200.0. New balance is 800.0") + System.setOut(System.out) } ``` @@ -871,16 +965,16 @@ fun testWithdraw_withSufficientBalance_updatesBalance() { Ensures that when a receipt is requested, it includes the correct balance details of the withdrawal. -``` +```kotlin @Test fun testWithdraw_withReceipt_generatesReceipt() { - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) - account.withdraw("user", "password", 200.0, needReceipt = true) + account.withdraw("user", "password", 200.0, needReceipt = true) - assertThat(output.toString().trim()).contains("Receipt: Withdrew 200.0. Current balance: 800.0") - System.setOut(System.out) + assertThat(output.toString().trim()).contains("Receipt: Withdrew 200.0. Current balance: 800.0") + System.setOut(System.out) } ``` @@ -888,21 +982,21 @@ fun testWithdraw_withReceipt_generatesReceipt() { Confirms balance statement with the passbook file being updated when a file is provided. -``` +```kotlin @Test fun testWithdraw_withPassbook_updatesPassbook() { - val passbookFile = File("passbook.pdf") - passbookFile.createNewFile() - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) + val passbookFile = File("passbook.pdf") + passbookFile.createNewFile() + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) - account.withdraw("user", "password", 200.0, file = passbookFile) + account.withdraw("user", "password", 200.0, file = passbookFile) - // Read the passbook file and check its contents - val fileContents = passbookFile.readText() - assertThat(fileContents).contains("Withdrew 200.0. New balance is 800.0") + // Read the passbook file and check its contents + val fileContents = passbookFile.readText() + assertThat(fileContents).contains("Withdrew 200.0. New balance is 800.0") - System.setOut(System.out) + System.setOut(System.out) } ``` @@ -910,16 +1004,16 @@ fun testWithdraw_withPassbook_updatesPassbook() { Validates that the correct message about the withdrawal is logged. -``` +```kotlin @Test fun testWithdraw_withSufficientBalance_logsCorrectMessage() { - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) - account.withdraw("user", "password", 200.0) + account.withdraw("user", "password", 200.0) - assertThat(output.toString().trim()).contains("New balance is 800.0") - System.setOut(System.out) + assertThat(output.toString().trim()).contains("New balance is 800.0") + System.setOut(System.out) } ``` @@ -927,11 +1021,11 @@ fun testWithdraw_withSufficientBalance_logsCorrectMessage() { Ensures that the balance is updated correctly after a withdrawal. -``` +```kotlin @Test fun testWithdraw_withSufficientBalance_updatesBalanceCorrectly() { - account.withdraw("user", "password", 200.0) - assertThat(account.balance).isEqualTo(800.0) + account.withdraw("user", "password", 200.0) + assertThat(account.balance).isEqualTo(800.0) } ``` @@ -945,10 +1039,10 @@ The following best practices are meant to assist in the organization and executi Helper functions are crucial for avoiding repetitive code, especially for pre-setup tasks that are used across multiple test cases. In this scenario, initializing the bank data or ensuring the proper state of the BankAccount object can be encapsulated in helper functions. -``` +```kotlin // Helper function to initialize a BankAccount with default data fun createDefaultBankAccount(): BankAccount { - return BankAccount(1000.0, "user", "password") + return BankAccount(1000.0, "user", "password") } ``` @@ -960,26 +1054,26 @@ Using `@Before` and `@After` annotations ensures that common setup and cleanup t **Example:** -``` +```kotlin class BankAccountTests { - private lateinit var account: BankAccount + private lateinit var account: BankAccount - @Before - fun setUp() { - // Initialize a BankAccount instance before each test - account = createDefaultBankAccount() - } + @Before + fun setUp() { + // Initialize a BankAccount instance before each test + account = createDefaultBankAccount() + } - @After - fun cleanup() { - // Clean up any resources or data after each test + @After + fun tearDown() { + // Clean up any resources or data after each test - // Restore the original system output stream after test - System.setOut(System.out) - } + // Restore the original system output stream after test + System.setOut(System.out) + } - // Test cases here + // Test cases here } ``` @@ -987,63 +1081,63 @@ class BankAccountTests { Naming test functions descriptively helps in identifying the purpose and scope of each test. Use names that reflect the specific behavior being tested. -``` +```kotlin @Test fun testWithdraw_withValidData_updatesBalance() { - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) - account.withdraw("user", "password", 200.0) + account.withdraw("user", "password", 200.0) - // Check that the withdrawal output is correct and no exceptions are thrown - assertThat(output.toString().trim()).contains("Withdrew 200.0. New balance is 800.0") - System.setOut(System.out) + // Check that the withdrawal output is correct and no exceptions are thrown + assertThat(output.toString().trim()).contains("Withdrew 200.0. New balance is 800.0") + System.setOut(System.out) } @Test fun testWithdraw_withNoFile_producesNoFileProcessingOutput() { - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) - account.withdraw("user", "password", 200.0) + account.withdraw("user", "password", 200.0) - // Ensure no file processing message is output when no file is provided - assertThat(output.toString().trim()).doesNotContain("Processing file") - System.setOut(System.out) + // Ensure no file processing message is output when no file is provided + assertThat(output.toString().trim()).doesNotContain("Processing file") + System.setOut(System.out) } @Test fun testWithdraw_withInvalidFileFormat_throwsException() { - val invalidFile = File("invalidFile.txt") + val invalidFile = File("invalidFile.txt") - // Verify that an invalid file format results in an appropriate exception - val exception = assertThrows { - account.withdraw("user", "password", 200.0, file = invalidFile) - } - assertThat(exception.message).contains("Invalid file format") + // Verify that an invalid file format results in an appropriate exception + val exception = assertThrows { + account.withdraw("user", "password", 200.0, file = invalidFile) + } + assertThat(exception.message).contains("Invalid file format") } @Test fun testWithdraw_withUnavailableFile_throwsException() { - val unavailableFile = File("nonExistentFile.pdf") + val unavailableFile = File("nonExistentFile.pdf") - // Verify that attempting to use a non-existent file results in an exception - val exception = assertThrows { - account.withdraw("user", "password", 200.0, file = unavailableFile) - } - assertThat(exception.message).contains("File not found") + // Verify that attempting to use a non-existent file results in an exception + val exception = assertThrows { + account.withdraw("user", "password", 200.0, file = unavailableFile) + } + assertThat(exception.message).contains("File not found") } @Test fun testWithdraw_withDefaultReceipt_noReceiptPrinted() { - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) - account.withdraw("user", "password", 200.0) + account.withdraw("user", "password", 200.0) - // Ensure that no receipt is printed when the default receipt flag is used - assertThat(output.toString().trim()).doesNotContain("Receipt:") - System.setOut(System.out) + // Ensure that no receipt is printed when the default receipt flag is used + assertThat(output.toString().trim()).doesNotContain("Receipt:") + System.setOut(System.out) } ``` @@ -1055,7 +1149,7 @@ fun testWithdraw_withDefaultReceipt_noReceiptPrinted() { - Documentation: Serves as self-documentation, providing insight into test purpose and scope. - Maintenance: Simplifies updates and modifications by clearly defining what each test covers. -# How to Map a Line of Code to Its Corresponding Behaviors? +# How to Map a Line of Code to Its Corresponding Behaviors Understanding how to map a line of code to its corresponding behaviors is essential for improving code coverage and writing effective tests. Here’s a structured approach to locate and cover lines of code: @@ -1065,19 +1159,19 @@ Consider that our test suite covers the code as follows: **Test:** -``` +```kotlin @Test fun testWithdraw_withValidCredentials_printsWithdrawMessage() { - val account = BankAccount(1000.0, "user", "password") - val file = File("file.pdf") + val account = BankAccount(1000.0, "user", "password") + val file = File("file.pdf") - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) - account.withdraw("user", "password", 200.0, file = file) + account.withdraw("user", "password", 200.0, file = file) - assertThat(output.toString().trim()).contains("Receipt: Withdrew 200.0. Current balance: 800.0") - System.setOut(System.out) + assertThat(output.toString().trim()).contains("Receipt: Withdrew 200.0. Current balance: 800.0") + System.setOut(System.out) } ``` @@ -1091,13 +1185,13 @@ You can utilize the Oppia Android code coverage tool to assess the coverage for Analyzing the report reveals that the line, -``` +```kotlin println("Receipt: Withdrew $amount. Current balance: $balance") ``` and its function call `printReceipt` are marked in red, indicating that this line was never executed by the test case. This suggests that the functionality is not covered by the current tests, potentially exposing it to issues or regressions if the code is modified in the future. The green highlights indicate the lines of code that are covered by test cases. -For more information on how to utilize the code coverage analysis tool, please refer to the [Oppia Android Code Coverage wiki page](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage) page. +For more information on how to utilize the code coverage analysis tool, please refer to the [Oppia Android Code Coverage](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage) page. ### 2. Map to the Line of Code @@ -1107,9 +1201,9 @@ Locate to the corresponding line number of the uncovered line in the source file BankAccount.kt -``` +```kotlin private fun printReceipt(amount: Double) { - println("Receipt: Withdrew $amount. Current balance: $balance") + println("Receipt: Withdrew $amount. Current balance: $balance") } ``` @@ -1118,31 +1212,34 @@ Flow Diagram ```mermaid graph TD F[printReceipt] --> G[print - Receipt: Withdrew $400] + + style F fill:#cfc,stroke:#333,stroke-width:1px + style G fill:#cfc,stroke:#333,stroke-width:1px ``` **2. Traceback to the calling point:** Next, trace the uncovered line back to where it is called in the code. This helps to understand why it wasn’t executed by the test case. -``` +```kotlin fun withdraw( - requestedUsername: String, - requestedPassword: String, - amount: Double, - needReceipt: Boolean = false, // Defaults to false - file: File? = null + requestedUsername: String, + requestedPassword: String, + amount: Double, + needReceipt: Boolean = false, // Defaults to false + file: File? = null ) { - // Other code here + // Other code here - if (needReceipt) { --------------------. - printReceipt(amount) : - } : + if (needReceipt) { --------------------. + printReceipt(amount) : + } : } : : ... : < -----------------` private fun printReceipt(amount: Double) { - println("Receipt: Withdrew $amount. Current balance: $balance") + println("Receipt: Withdrew $amount. Current balance: $balance") } ``` @@ -1150,9 +1247,9 @@ private fun printReceipt(amount: Double) { Identify the condition that controls whether the line of code is executed. Here it is the condition to have the value of **needReceipt** set to **true** to call the `printReceipt` method. -``` +```kotlin if (needReceipt) { - printReceipt(amount) + printReceipt(amount) } ``` @@ -1163,24 +1260,29 @@ graph TD D[Condition - if needReceipt is true] --> E[Method Call - printReceipt] E --> F[printReceipt] F --> G[print - Receipt: Withdrew $400] + + style D fill:#cfc,stroke:#333,stroke-width:1px + style E fill:#cfc,stroke:#333,stroke-width:1px + style F fill:#cfc,stroke:#333,stroke-width:1px + style G fill:#cfc,stroke:#333,stroke-width:1px ``` 2.2 Determine the Origin of the Conditional Value Next, trace where this conditional value is set in the method. This helps to identify the requirement of the condition to set a passing value and access the method call. -``` +```kotlin fun withdraw( - requestedUsername: String, - requestedPassword: String, - amount: Double, - needReceipt: Boolean = false, // Defaults to false ---------. - file: File? = null : + requestedUsername: String, + requestedPassword: String, + amount: Double, + needReceipt: Boolean = false, // Defaults to false ---------. + file: File? = null : ) { : : - if (needReceipt) { <-----------------------` - printReceipt(amount) - } + if (needReceipt) { <-----------------------` + printReceipt(amount) + } } ``` @@ -1192,18 +1294,24 @@ graph TD D[Condition - if needReceipt is true] --> E[Method Call - printReceipt] E --> F[printReceipt] F --> G[print - Receipt: Withdrew $400] + + style B fill:#FFB6C1,stroke:#333,stroke-width:1px, color:#00 + style D fill:#cfc,stroke:#333,stroke-width:1px + style E fill:#cfc,stroke:#333,stroke-width:1px + style F fill:#cfc,stroke:#333,stroke-width:1px + style G fill:#cfc,stroke:#333,stroke-width:1px ``` It can be seen that the **needReceipt** value is passed as a parameter while having a **default value** of **false**. Since the value was **never set to true** in our test case, -``` +```kotlin @Test fun testWithdraw_withValidCredentials_printsWithdrawMessage() { - ... + ... - account.withdraw("user", "password", 200.0, file = file) + account.withdraw("user", "password", 200.0, file = file) - ... + ... } ``` @@ -1213,13 +1321,13 @@ it defaulted to being false thereby never meeting the condition to perform the ` Identify the method or function that influences the needReceipt parameter and trace it to the public API where it is used. By understanding this connection, you can modify the needReceipt parameter’s default value in withdraw to affect the behavior of the code. -``` +```kotlin fun withdraw( - requestedUsername: String, - requestedPassword: String, - amount: Double, - needReceipt: Boolean = false, // Defaults to false - file: File? = null + requestedUsername: String, + requestedPassword: String, + amount: Double, + needReceipt: Boolean = false, // Defaults to false + file: File? = null ) { } ``` @@ -1233,6 +1341,14 @@ graph TD D --> E[Method Call - printReceipt] E --> F[printReceipt] F --> G[print - Receipt: Withdrew $400] + + style A fill:#f5f5f5,stroke:#333,stroke-width:1px, color:#000 + style B fill:#ffb6c1,stroke:#333,stroke-width:1px, color:#00 + style C fill:#cfc,stroke:#333,stroke-width:1px + style D fill:#cfc,stroke:#333,stroke-width:1px + style E fill:#cfc,stroke:#333,stroke-width:1px + style F fill:#cfc,stroke:#333,stroke-width:1px + style G fill:#cfc,stroke:#333,stroke-width:1px ``` **3. Add Test Case to Cover the Line:** @@ -1241,18 +1357,18 @@ To ensure that the `printReceipt` method is covered, we need to add a test case Test: -``` +```kotlin @Test fun testWithdraw_withReceipt_printsReceipt() { - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) - // Call withdraw with needReceipt set to true - account.withdraw("user", "password", 200.0, needReceipt = true) + // Call withdraw with needReceipt set to true + account.withdraw("user", "password", 200.0, needReceipt = true) - // Verify that receipt was printed - assertThat(output.toString().trim()).contains("Receipt: Withdrew 200.0. Current balance: 800.0") - System.setOut(System.out) + // Verify that receipt was printed + assertThat(output.toString().trim()).contains("Receipt: Withdrew 200.0. Current balance: 800.0") + System.setOut(System.out) } ``` From 1c1531d9d938dcee6c47078a1349d2ef2762de27 Mon Sep 17 00:00:00 2001 From: Rd Date: Sat, 17 Aug 2024 11:57:44 +0530 Subject: [PATCH 30/64] Addressing reviews part 2 - Structuring Test Functionalities --- ...ng-tests-with-good-behavioural-coverage.md | 245 +++++++++++++----- 1 file changed, 181 insertions(+), 64 deletions(-) diff --git a/wiki/Writing-tests-with-good-behavioural-coverage.md b/wiki/Writing-tests-with-good-behavioural-coverage.md index 0fb4795967b..3dc30a12763 100644 --- a/wiki/Writing-tests-with-good-behavioural-coverage.md +++ b/wiki/Writing-tests-with-good-behavioural-coverage.md @@ -9,6 +9,10 @@ - [Covering Different Scenarios](#4-covering-different-scenarios) - [Covering All Branches, Paths, and Conditions](#5-covering-all-branches-paths-and-conditions) - [Exception and Error Handling](#6-exception-and-error-handling) +- [Structuring Test Functionalities](#structuring-test-functionalities) + - [When and How to Divide Responsibilities](#1-when-and-how-to-divide-responsibilities) + - [When Not to Divide Responsibilities](#2-when-not-to-divide-responsibilities) + - [Importance of Descriptive Test Names](#3-importance-of-descriptive-test-names) - [Guidelines for Testing Public API](#guidelines-for-testing-public-api) - [How to Map a Line of Code to Its Corresponding Behaviors?](#how-to-map-a-line-of-code-to-its-corresponding-behaviors) @@ -1031,123 +1035,236 @@ fun testWithdraw_withSufficientBalance_updatesBalanceCorrectly() { These tests cover various aspects of the withdrawal functionality, ensuring that the balance updates correctly with receipts, passbooks and output messages. Although the core functionality of withdrawing funds and updating the balance is consistent, it can be observed in multiple ways. Each test focuses on a specific verification method while ultimately validating the same core functionality. -## Testing Practices for Effective Validation +# Structuring Test Functionalities -The following best practices are meant to assist in the organization and execution of tests. By following these guidelines, you can ensure that your testing process is efficient, thorough, and well-structured. +In testing, it's crucial to ensure that your tests verify implementation code while maintaining clarity and readability. Tests validate the correctness of the code, but it is humans who verify the correctness of the test code itself. Therefore, striking a balance between clarity and conciseness in test writing is essential. -### 1. Use of Helper Functions +```sh ++------------+ verify +--------------+ +| Tests | ---------> | Source Code | ++------------+  +--------------+ -Helper functions are crucial for avoiding repetitive code, especially for pre-setup tasks that are used across multiple test cases. In this scenario, initializing the bank data or ensuring the proper state of the BankAccount object can be encapsulated in helper functions. ++------------+ verify +--------------+ +| Human | ----------> | Tests | ++------------+ +--------------+ + +``` + +Tests should focus on verifying the behavior of the implementation code, while humans should be able to easily understand and verify the correctness of the test code itself. + +Let's use a Restaurant class as an example to explain how to structure test functionalities effectively. The Restaurant class manages orders, calculates bills, and applies discounts. ```kotlin -// Helper function to initialize a BankAccount with default data -fun createDefaultBankAccount(): BankAccount { - return BankAccount(1000.0, "user", "password") +class Restaurant(private val menu: Map) { + var items: List = listOf() + private var discount: Double = 0.0 + + fun placeOrder(orderItems: List) { + items = orderItems + println("Order placed: $items") + val totalBill = calculateBill() + println("Total bill after discount: ${totalBill - (totalBill * discount)}") + } + + fun applyDiscount(isMember: Boolean, code: String, quarter: YearQuarter) { + discount = when { + isMember && code == "SAVE10" -> 0.10 + code == "SAVE20" && quarter == YearQuarter.QUARTER1 -> 0.20 + code == "SUMMERSALE" && quarter == YearQuarter.QUARTER2 -> 0.15 + else -> 0.0 + } + println("Discount applied: ${discount * 100}%") + } + + private fun calculateBill(): Double { + return items.sumOf { menu[it] ?: 0.0 } + } +} + +enum class YearQuarter { + QUARTER1, QUARTER2, QUARTER3, QUARTER4 } ``` -This helps to reduce redundancy and maintain consistency across multiple test cases by centralizing repetitive setup tasks. +It's important to understand how to segment or split functionalities in your tests to maintain clarity and avoid confusion. Knowing when to use helper functions and `@Before` / `@After` annotations effectively, and when to keep logic within the test cases themselves, ensures that your tests are both clear and maintainable. Let’s explore these concepts with a Restaurant Ordering System example. + +## 1. When and How to Divide Responsibilities -### 2. Setup and Teardown using `@Before` and `@After` +### a. Helper Functions -Using `@Before` and `@After` annotations ensures that common setup and cleanup tasks are automatically executed before and after each test case, maintaining a clean and consistent test environment. +Helper Functions are valuable for reducing redundancy in tests. They encapsulate **non-behavioral tasks**, ensuring that the focus remains on testing the core logic. -**Example:** +**Helper Function:** ```kotlin -class BankAccountTests { +// Helper function to initialize a Restaurant with a menu +fun createRestaurantWithMenu(): Restaurant { + val menu = mapOf( + "Burger" to 5.0, + "Pizza" to 8.0, + "Salad" to 4.0 + ) + return Restaurant(menu) +} +``` - private lateinit var account: BankAccount +**Test using the Helper Function:** + +```kotlin +@Test +fun testPlaceOrder_withValidItems_displaysCorrectTotalBill() { + val restaurant = createRestaurantWithMenu() + restaurant.placeOrder(listOf("Burger", "Pizza")) + + val outputStream = ByteArrayOutputStream() + System.setOut(PrintStream(outputStream)) + + assertThat(outputStream.toString().trim()).contains("Total bill: 13.0") + + System.setOut(System.out) +} +``` + +### b. `@Before` and `@After` Annotations + +`@Before` and `@After` Annotations help in managing setup and teardown tasks, ensuring consistency and reducing redundancy in test cases. + +```kotlin +class RestaurantTests { + private lateinit var restaurant: Restaurant + private lateinit var outputStream: ByteArrayOutputStream @Before fun setUp() { - // Initialize a BankAccount instance before each test - account = createDefaultBankAccount() + // Setup necessary resources + outputStream = ByteArrayOutputStream() + System.setOut(PrintStream(outputStream)) + restaurant = createRestaurantWithMenu() } @After fun tearDown() { - // Clean up any resources or data after each test - - // Restore the original system output stream after test + // Clean up resources after tests System.setOut(System.out) } - // Test cases here + @Test + fun testPlaceOrder_withValidItems_displaysCorrectTotalBill() { + restaurant.placeOrder(listOf("Burger", "Pizza")) + assertThat(outputStream.toString().trim()).contains("Total bill: 13.0") + } } ``` -### 3. Descriptive Test Names +Use `@Before` and `@After` for tasks that need to be performed before and after every test case, such as setting up streams, initializing objects, or restoring states. These tasks should not contain logic that is part of the actual test behavior. -Naming test functions descriptively helps in identifying the purpose and scope of each test. Use names that reflect the specific behavior being tested. +## 2. When Not to Divide Responsibilities + +### Prioritizing Clarity Over Conciseness in Tests + +While it’s tempting to reduce code duplication in tests by using helper functions or annotations, **clarity should take precedence**. Tests are meant to verify that the code works correctly, but they also need to be clear enough for humans to understand and verify their correctness. + +**The Pitfalls of Complex Test Helper Implementations can be understood with the following case:** ```kotlin +fun createDiscount(): Triple { + val isMember = true + val code = "SAVE10" + val quarter = YearQuarter.QUARTER1 + return Triple(isMember, code, quarter) +} + @Test -fun testWithdraw_withValidData_updatesBalance() { - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) +fun testDiscountedBill_withHelper() { + val restaurant = createRestaurantWithMenu() + val discountDetails = createDiscount() - account.withdraw("user", "password", 200.0) + restaurant.applyDiscount(discountDetails.first, discountDetails.second, discountDetails.third) + restaurant.placeOrder(listOf("Burger")) - // Check that the withdrawal output is correct and no exceptions are thrown - assertThat(output.toString().trim()).contains("Withdrew 200.0. New balance is 800.0") - System.setOut(System.out) + assertThat((outputStream.toString().trim()).contains("Total bill after discount: 4.5") } +``` + +**The Drawbacks of This Approach** +- Hidden Logic: The helper function `createDiscount()` hides critical logic affecting the test outcome. This makes the test harder to understand and debug. +- Complexity: The helper function handles multiple scenarios, which should be tested separately. This introduces complexity and reduces clarity. +- Clarity: Hidden logic in a helper function makes the test less transparent and harder to interpret, compromising its readability. + +**Approach to write test with clarity:** +In test code, being explicit often trumps being concise. This means defining the necessary conditions and actions directly within the test case, so the test's intent is immediately clear to anyone reading it. + +```kotlin @Test -fun testWithdraw_withNoFile_producesNoFileProcessingOutput() { - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) +fun testDiscountedBill_withAppliedDicount_returnsDiscountedBill() { + val restaurant = createRestaurantWithMenu() + + // Explicitly defining discount details in the test + val isMember = true + val code = "SAVE10" + val quarter = YearQuarter.QUARTER1 - account.withdraw("user", "password", 200.0) + restaurant.applyDiscount(isMember, code, quarter) + restaurant.placeOrder(listOf("Burger")) - // Ensure no file processing message is output when no file is provided - assertThat(output.toString().trim()).doesNotContain("Processing file") - System.setOut(System.out) + assertThat((outputStream.toString().trim()).contains("Total bill after discount: 4.5") } +``` -@Test -fun testWithdraw_withInvalidFileFormat_throwsException() { - val invalidFile = File("invalidFile.txt") +Laying out the logic and conditions directly within the test makes it independent of external functions or files, which makes the test easier to understand, maintain, and debug. - // Verify that an invalid file format results in an appropriate exception - val exception = assertThrows { - account.withdraw("user", "password", 200.0, file = invalidFile) - } - assertThat(exception.message).contains("Invalid file format") +Unlike production code, where duplication is often avoided, in test code, it’s sometimes better to duplicate code if it leads to clearer, more understandable tests. This ensures that the behavior being tested is directly represented in the test case. + +## 3. Importance of Descriptive Test Names + +Naming test functions descriptively helps in identifying the purpose and scope of each test. Use names that reflect the specific behavior being tested. + +Oppia Android follows a naming convention where the test names should read like a sentence, and be consistent with other nearby test names to facilitate easily coming up with new tests. Consider using a format similar to the following for naming test functions: + +``` +testAction_withOneCondition_withSecondCondition_hasExpectedOutcome +``` + +```kotlin +@Test +fun testPlaceOrder_withValidItems_orderPlacedSuccessfully() { + // Test Logic: Order should be placed with given items } @Test -fun testWithdraw_withUnavailableFile_throwsException() { - val unavailableFile = File("nonExistentFile.pdf") +fun testPlaceOrder_withEmptyItems_orderNotPlaced() { + // Test Logic: Handle empty order gracefully +} - // Verify that attempting to use a non-existent file results in an exception - val exception = assertThrows { - account.withdraw("user", "password", 200.0, file = unavailableFile) - } - assertThat(exception.message).contains("File not found") +@Test +fun testCalculateBill_withValidItems_correctBillCalculated() { + // Test Logic: Calculate correct bill for ordered items } @Test -fun testWithdraw_withDefaultReceipt_noReceiptPrinted() { - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) +fun testApplyDiscount_withMemberAndValidCode_discountApplied() { + // Test Logic: Apply 10% discount for valid code and membership +} - account.withdraw("user", "password", 200.0) +@Test +fun testApplyDiscount_withNonMemberAndValidCode_noDiscountApplied() { + // Test Logic: No discount applied for non-member +} - // Ensure that no receipt is printed when the default receipt flag is used - assertThat(output.toString().trim()).doesNotContain("Receipt:") - System.setOut(System.out) +@Test +fun testApplyDiscount_withMemberAndQuarter2_discountApplied() { + // Test Logic: Apply discount for valid code in Quarter 2 } ``` -**Importance of Specific Naming and Conditions:** +### Benefits of Descriptive Naming Conventions -- Clarity: Specific names and conditions make tests easier to understand and manage. -- Focus: Helps pinpoint exact scenarios being tested, improving test coverage. -- Debugging: Clear names and conditions aid in quickly identifying the cause of failures. -- Documentation: Serves as self-documentation, providing insight into test purpose and scope. -- Maintenance: Simplifies updates and modifications by clearly defining what each test covers. +- **Clarity:** Specific names and conditions make tests easier to understand and manage. +- **Focus:** Helps pinpoint exact scenarios being tested, improving test coverage. +- **Debugging:** Clear names and conditions aid in quickly identifying the cause of failures. +- **Documentation:** Serves as self-documentation, providing insight into test purpose and scope. +- **Maintenance:** Simplifies updates and modifications by clearly defining what each test covers. # How to Map a Line of Code to Its Corresponding Behaviors @@ -1327,7 +1444,7 @@ fun withdraw( requestedPassword: String, amount: Double, needReceipt: Boolean = false, // Defaults to false - file: File? = null + file: File? = null ) { } ``` From f7552b300411213a49718121c24d84a7a322b56b Mon Sep 17 00:00:00 2001 From: Rd Date: Sat, 17 Aug 2024 12:46:06 +0530 Subject: [PATCH 31/64] Includes minor changes and removal of pull_request_target triggers to set things up to be ready for testing uploads of comments with Fork PRs --- .github/workflows/code_coverage.yml | 22 +++++-------------- ...ng-tests-with-good-behavioural-coverage.md | 6 ++--- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 8dab2b5b51d..cbb15b1581f 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -2,23 +2,14 @@ name: Code Coverage -on: - workflow_dispatch: - pull_request_target: - types: [ opened, synchronize, reopened ] - - # Controls when the action will run. Triggers the workflow on pull request # events or push events in the develop branch. -#on: -# pull_request_target: -# types: [opened, synchronize, reopened] -#on: -# pull_request: -# push: -# branches: -# # Push events on develop branch -# - develop +on: + pull_request: + push: + branches: + # Push events on develop branch + - develop permissions: pull-requests: write @@ -327,7 +318,6 @@ jobs: # 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-latest steps: - name: Download Generated Markdown Report diff --git a/wiki/Writing-tests-with-good-behavioural-coverage.md b/wiki/Writing-tests-with-good-behavioural-coverage.md index 3dc30a12763..02181db1328 100644 --- a/wiki/Writing-tests-with-good-behavioural-coverage.md +++ b/wiki/Writing-tests-with-good-behavioural-coverage.md @@ -168,7 +168,7 @@ Consider a function that manages a food order process. This function does the fo 1. Lists the food items. 2. Calculates the total price. -3. Display the order. +3. Displays the order. 4. Checks if the payment has been made and provides the corresponding message. ```kotlin @@ -211,7 +211,7 @@ fun testProcessOrder_allSteps_returnsCorrectMessage() { **Difficulties in Testing All Aspects Together:** - Complex Failure Diagnosis: If this test fails, you need to diagnose whether the issue lies in listing items, calculating the total, displaying the order, or payment status. -- Less Focused: It does not target individual aspects of the function, making it harder to identify which specific action failed. +- Less Focused: It does not target individual aspects of the function, making it harder to identify which specific action failed for cases when the test overall is failing. ### Testing Specific Aspects @@ -1175,7 +1175,7 @@ fun createDiscount(): Triple { } @Test -fun testDiscountedBill_withHelper() { +fun testDiscountedBill_withCreateDiscountHelper_returnsDiscountedBill() { val restaurant = createRestaurantWithMenu() val discountDetails = createDiscount() From b1a5bd76878cfe21ce66ca01df984d0d904c4337 Mon Sep 17 00:00:00 2001 From: Rd Date: Sun, 18 Aug 2024 02:41:42 +0530 Subject: [PATCH 32/64] Addressed 2nd Pass of review - API Testing, Quality --- ...ng-tests-with-good-behavioural-coverage.md | 493 ++++++++++-------- 1 file changed, 262 insertions(+), 231 deletions(-) diff --git a/wiki/Writing-tests-with-good-behavioural-coverage.md b/wiki/Writing-tests-with-good-behavioural-coverage.md index 02181db1328..b6637e2deb1 100644 --- a/wiki/Writing-tests-with-good-behavioural-coverage.md +++ b/wiki/Writing-tests-with-good-behavioural-coverage.md @@ -9,11 +9,12 @@ - [Covering Different Scenarios](#4-covering-different-scenarios) - [Covering All Branches, Paths, and Conditions](#5-covering-all-branches-paths-and-conditions) - [Exception and Error Handling](#6-exception-and-error-handling) + - [Absence of Unwanted Output](#7-absence-of-unwanted-output) +- [Testing Public API](#testing-public-api) - [Structuring Test Functionalities](#structuring-test-functionalities) - [When and How to Divide Responsibilities](#1-when-and-how-to-divide-responsibilities) - [When Not to Divide Responsibilities](#2-when-not-to-divide-responsibilities) - [Importance of Descriptive Test Names](#3-importance-of-descriptive-test-names) -- [Guidelines for Testing Public API](#guidelines-for-testing-public-api) - [How to Map a Line of Code to Its Corresponding Behaviors?](#how-to-map-a-line-of-code-to-its-corresponding-behaviors) # What is Behavioral Coverage? @@ -86,6 +87,45 @@ To ensure behavioral coverage, the test needs to verify various conditions: While line coverage might reach 100% with a single test, it doesn’t ensure that all scenarios are tested. Behavioral coverage ensures quality by validating all possible scenarios and edge cases, which is more important for reliable software. +**Evaluation and Enhancement Flow:** + +```sh + + +-------------------+ + | Coverage Analysis | + +-------------------+ + ↓ ++---------------------------+ +--------------------------------+ +| 100% Line Coverage | | Less Than 100% Coverage | +| Does not guarantee full | | Guarantees at least one | +| behavioural coverage | | important behaviour is missing | ++---------------------------+ +--------------------------------+ + + ↓ ↓ ++------------------------------+ +---------------------------------+ +| Add tests for lines that | | Find uncovered lines and add | +| misses behavioural coverages | <--- | tests that cover all behaviours | +| even if stated as covered | | and not just line coverages | ++------------------------------+ +---------------------------------+ + +``` + +**Coverage and Behaviour:** + +- 100% Line Coverage: Does not guarantee that all critical behaviors and edge cases are tested. It merely shows that each line of code has been executed. +- Less Than 100% Coverage: Guarantees that some part of the code is not covered by tests, which likely means that at least one important behavior or edge case is missing from the tests. + +**Line Coverage as a Starting Point:** + +- Use line coverage metrics as a starting point to identify which parts of the code have not been tested. +- Analyze these uncovered lines to determine which behaviors are associated with them. While line coverage helps locate untested code, it is crucial to ensure that all significant behaviors are tested, especially for volatile or complex scenarios. + +**Improving Coverage:** + +- Testing a specific line of code is useful, but it’s equally important to check for other missing behaviors or edge cases even if a line is stated as covered with coverage analysis. +- If a line of code is covered but does not account for all potential scenarios, add tests to cover those additional important behaviors. +- Ensure that tests are comprehensive by addressing various scenarios, including edge cases and error conditions, which might not be immediately obvious from line coverage alone. + For more details on testing methodologies specific to Oppia Android, please refer to the [Oppia Android Testing](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Testing). # Writing Effective Tests @@ -387,65 +427,109 @@ Testing all branches and conditions ensures that your function can handle all po ## 6. Exception and Error Handling -**Exception Handling:** +### Exceptions + +Exceptions are unexpected events or errors that occur during the execution of a program, disrupting the normal flow of instructions. These are typically conditions that a program cannot anticipate or recover from easily, such as a division by zero, accessing an invalid index in an array, or trying to open a file that doesn’t exist. When an exception occurs, the program may terminate abruptly unless the exception is properly handled. + +```kotlin +fun divideNumbers(numerator: Int, denominator: Int): Int { + if (denominator == 0) throw IllegalArgumentException("Denominator cannot be zero") + return numerator / denominator +} +``` -Exceptions are unexpected conditions or events that disrupt the normal flow of a program. They handle situations not part of the regular program flow, like invalid user input or file operation issues. +In this example, if the denominator is zero, an `IllegalArgumentException` is thrown. This is a standard way to handle situations where continuing the program as usual doesn’t make sense due to an error in the input. -Examples: -- `NullPointerException` (accessing a null reference) -- `IllegalArgumentException` (invalid function argument) -- `IOException` (I/O operation failure) +**Testing Exceptions:** -This allows to catch and handle issues, allowing the program to continue running or fail in a controlled manner. +The primary focus when testing exceptions is ensuring that the correct exception is thrown in the appropriate circumstances and that the program can handle it gracefully. -For instance, to handle division by zero error. +**Test:** ```kotlin -fun divide(a: Int, b: Int): Int { - if (b == 0) throw ArithmeticException("Division by zero") - return a / b +@Test +fun testDivideNumbers_forZeroDenominator_throwsIllegalArgumentException() { + val exception = assertThrows { + divideNumbers(10, 0) + } + assertThat(exception).contains("Denominator cannot be zero") } ``` -**Error Handling:** +This test verifies that the divideNumbers function throws an IllegalArgumentException when the denominator is zero. It also checks that the exception message contains the expected text. Testing exceptions involves confirming that the application correctly identifies and handles these unexpected situations. -Errors are more severe issues indicating fundamental problems with the program's environment or state, often beyond the program's control. In Java/Kotlin, these are represented by classes extending Error, like `OutOfMemoryError` or `StackOverflowError`. +### Domain Errors -This allows to manage situations where the application may not continue running properly, ensuring the program does not enter an unstable state. +Domain errors are errors related to the business logic or rules of the application, rather than technical issues like those covered by exceptions. These errors occur when the data or conditions within the domain do not meet the specific rules or constraints defined by the business. Domain errors are expected conditions and are handled within the normal flow of the application, rather than through exceptions. -For instance, +Let's understand this with a sample snippet from the source **[FractionParser.kt](https://github.com/oppia/oppia-android/blob/develop/utility/src/main/java/org/oppia/android/util/math/FractionParser.kt)** ```kotlin -fun simulateError() { - val arr = IntArray(Int.MAX_VALUE) // May cause OutOfMemoryError +fun getSubmitTimeError(text: String): FractionParsingError { + if (text.isNullOrBlank()) { + return FractionParsingError.EMPTY_INPUT + } + if (invalidCharsLengthRegex.find(text) != null) { + return FractionParsingError.NUMBER_TOO_LONG + } + if (text.endsWith("/")) { + return FractionParsingError.INVALID_FORMAT + } + val fraction = parseFraction(text) + return when { + fraction == null -> FractionParsingError.INVALID_FORMAT + fraction.denominator == 0 -> FractionParsingError.DIVISION_BY_ZERO + else -> FractionParsingError.VALID + } } ``` -### Testing Exceptions +This function checks various conditions on the input string text to determine whether it meets the criteria for a valid fraction. Each condition that fails returns a specific domain error from the FractionParsingError enum. Unlike exceptions, these errors are expected as part of the application's logic and represent specific, recoverable conditions that the application can handle. -Ensure that the code handles invalid or unexpected inputs gracefully and verify that appropriate errors or exceptions are triggered. +**Testing Domain Errors:** -**Functionality:** +The goal when testing domain errors is to ensure that the application correctly identifies and responds to these errors as part of its normal operation. + +**Test samples from [FractionParserTest.kt](https://github.com/oppia/oppia-android/blob/develop/utility/src/test/java/org/oppia/android/util/math/FractionParserTest.kt):** ```kotlin -fun divideNumbers(numerator: Int, denominator: Int): Int { - if (denominator == 0) throw IllegalArgumentException("Denominator cannot be zero") - return numerator / denominator +@Test +fun testSubmitTimeError_tenDigitNumber_returnsNumberTooLong() { + val error = getSubmitTimeError("0123456789") + assertThat(error).isEqualTo(FractionParsingError.NUMBER_TOO_LONG) } -``` -**Test Case:** +@Test +fun testSubmitTimeError_nonDigits_returnsInvalidFormat() { + val error = getSubmitTimeError("jdhfc") + assertThat(error).isEqualTo(FractionParsingError.INVALID_FORMAT) +} -```kotlin @Test -fun testDivideNumbers_forZeroDenominator_throwsIllegalArgumentException() { - val exception = assertThrows { - divideNumbers(10, 0) - } - assertThat(exception).contains("Denominator cannot be zero") +fun testSubmitTimeError_divisionByZero_returnsDivisionByZero() { + val error = getSubmitTimeError("123/0") + assertThat(error).isEqualTo(FractionParsingError.DIVISION_BY_ZERO) +} + +@Test +fun testSubmitTimeError_ambiguousSpacing_returnsInvalidFormat() { + val error = getSubmitTimeError("1 2 3/4") + assertThat(error).isEqualTo(FractionParsingError.INVALID_FORMAT) } ``` +These tests check various inputs to ensure the getSubmitTimeError function correctly identifies and returns the appropriate FractionParsingError. Each test case focuses on a specific condition that is expected and handled within the domain logic, ensuring the application behaves as intended. + +### Handling Exceptions and Domain Errors + +- **Nature:** Exceptions are typically unforeseen disruptions in program execution, whereas domain errors are expected results of business logic conditions. + +- **Focus:** When testing exceptions, the focus is on ensuring proper handling and recovery from unexpected situations. In contrast, testing domain errors involves verifying that the application correctly identifies and manages these expected conditions. + +- **Handling and Recovery:** Exceptions often require special recovery mechanisms, such as try-catch blocks, while domain errors are managed through normal application logic and flow control. + +## Key Considerations for Testing Exception / Error Scenarios + Testing exceptions and error handling is vital to ensure that applications behave correctly under error conditions, provide meaningful feedback, and maintain reliability. Without these tests, applications are prone to unpredictable failures, poor user experiences, and potential security issues. ### 1. Verifying Correct Exception Types @@ -616,7 +700,78 @@ fun testCalculateDiscount_forNegativeDiscount_throwsIllegalArgumentException() { } ``` -# Guidelines for Testing Public API +## 7. Absence of Unwanted Output + +In addition to validating correct handling of valid and invalid files, it's also important to ensure that unwanted output or behavior does not occur. + +Let's use a simple `Pizza` class with an `orderPizza` function that has optional parameters like **addCheese** and **takeaway**. The idea is to test that when these options are set to **false**, no corresponding messages are printed. + +**Functionality:** + +```kotlin +class Pizza { + fun orderPizza(addCheese: Boolean = false, takeaway: Boolean = false): String { + var orderDetails = "Ordered a pizza" + + if (addCheese) { + orderDetails += " with extra cheese" + } + + if (takeaway) { + orderDetails += " for takeaway" + } + + println(orderDetails) + return orderDetails + } +} +``` + +**Test:** + +Ensure No Cheese Message When **addCheese** is **false** + +```kotlin +@Test +fun testOrderPizza_withNoCheese_doesNotPrintCheeseMessage() { + val pizza = Pizza() + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) + + // Order pizza without cheese, default value is false + pizza.orderPizza(addCheese = false) + + // Verify that "with extra cheese" is not printed + assertThat(output.toString().trim()).doesNotContain("with extra cheese") + + System.setOut(System.out) +} +``` + +**Test:** + +Ensure No Takeaway Message When **takeaway** is **false** + +```kotlin +@Test +fun testOrderPizza_withNoTakeaway_doesNotPrintTakeawayMessage() { + val pizza = Pizza() + val output = ByteArrayOutputStream() + System.setOut(PrintStream(output)) + + // Order pizza without takeaway, default value is false + pizza.orderPizza(takeaway = false) + + // Verify that "with extra cheese" is not printed + assertThat(output.toString().trim()).doesNotContain("for takeaway") + + System.setOut(System.out) +} +``` + +These tests confirm that the class behaves as expected without producing unnecessary outputs. + +# Testing Public API A public API (Application Programming Interface) refers to the set of methods, properties, and functionalities exposed by a class or module for use by external code. It defines how other parts of a system or external systems can interact with the functionality provided by that class or module. @@ -678,273 +833,149 @@ class BankAccount( The **`withdraw`** method serves as the single public entry point for withdrawing money from the account. It handles user validation, amount checking, optional file upload and printing of the receipt. By keeping the internal methods private, the class ensures that the operations are performed in a controlled manner while hiding the complexity of these operations from the user. -## Testing Public API - -Testing a public API involves verifying that its methods and functionalities work as expected under various conditions. To ensure comprehensive coverage, it's important to focus on the behavior of the API rather than just individual lines of code. - -### 1. Checking Pre-Conditions - -Ensure the necessary setup, such as initializing bank data, is completed before running tests. - -**Test:** - -```kotlin -@Test -fun testWithdraw_noBankData_initializationError() { - val account = BankAccount(null, null, null) // Initialize with null values - - val exception = assertThrows { - account.withdraw("user", "password", 200.0) - } - assertThat(exception).contains("Bank data not initialized") -} -``` - -This test ensures that if the bank data is not set up properly, the system throws an appropriate exception. This is important to verify that the API behaves correctly when initialization is incomplete. - -### 2. Testing Valid Data - -Verify that the API correctly processes valid input data. - -```kotlin -@Test -fun testWithdraw_validData_outputsCorrectBalance() { - val account = BankAccount(1000.0, "user", "password") - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) +## How to Write Tests for Public API - account.withdraw("user", "password", 200.0) +```sh ++---------+ +---------------+ +-----------+ +--------+ +| Analyze | -> | Map Behaviors | -> | Add Tests | -> | Refine | ++---------+ +---------------+ +-----------+ +--------+ - val outputLines = output.toString().trim() - assertThat(outputLines).isEqualTo("Withdrew 200.0. New balance is 800.0") - System.setOut(System.out) -} ``` -### 3. Testing Invalid Data +### 1. Analyze the Public API -Ensure the API handles incorrect or invalid data properly. +Goal: Identify and map all possible behaviors and edge cases of the API method. -```kotlin -@Test -fun testWithdraw_invalidUserName_throwsException() { - val account = BankAccount(1000.0, "user", "password") +**1. Identify Core Functionalities:** Break down the public method into its core functionalities. For the withdraw method, these include: - val exception = assertThrows { - account.withdraw("invalidUser", "password", 200.0) - } - assertThat(exception).contains("Invalid credentials") -} +- User authentication +- Amount validation +- Withdrawal execution +- Receipt printing +- File processing -@Test -fun testWithdraw_invalidPassword_throwsException() { - val account = BankAccount(1000.0, "user", "password") +**2. Determine Expected Behaviors:** List the expected behaviors and outcomes for each core functionality. Consider both normal and edge cases. - val exception = assertThrows { - account.withdraw("user", "invalidPassword", 200.0) - } - assertThat(exception).contains("Invalid credentials") -} -``` +- Valid and invalid user credentials +- Valid and invalid withdrawal amounts +- Presence and absence of receipt +- File presence and absence -### 4. Testing Edge Cases +### 2. Map Expected Behaviors to Test Cases -Verify the API's behavior with edge cases and boundary conditions. +- Goal: Create a comprehensive list of test cases based on the identified behaviors. +- Format: Use clear and descriptive test names to represent each behavior. -```kotlin -@Test -fun testWithdraw_emptyUsername_throwsException() { - val account = BankAccount(1000.0, "user", "password") +Example Mappings: - val exception = assertThrows { - account.withdraw("", "password", 200.0) - } - assertThat(exception).contains("Username cannot be empty") -} +**1. User Authentication:** +- testWithdraw_validCredentials_outputsCorrectBalance +- testWithdraw_invalidUsername_throwsException +- testWithdraw_invalidPassword_throwsException +- testWithdraw_noBankData_initializationError -@Test -fun testWithdraw_emptyBalance_throwsException() { - val account = BankAccount(0.0, "user", "password") +**2. Amount Validation:** +- testWithdraw_validAmount_updatesBalance +- testWithdraw_negativeAmount_throwsException +- testWithdraw_amountGreaterThanBalance_throwsException - val exception = assertThrows { - account.withdraw("user", "password", 200.0) - } - assertThat(exception).contains("Insufficient balance") -} +**3. Receipt Printing:** +- testWithdraw_withNeedReceipt_receiptPrinted +- testWithdraw_withoutNeedReceipt_noReceiptPrinted +- testWithdraw_withDefaultReceipt_noReceiptPrinted -@Test -fun testWithdraw_emptyAmount_throwsException() { - val account = BankAccount(1000.0, "user", "password") +**4. File Processing:** +- testWithdraw_withValidFile_processesFile +- testWithdraw_withInvalidFileFormat_throwsException +- testWithdraw_withAvailableFile_processesFile +- testWithdraw_withUnavailableFile_throwsException - val exception = assertThrows { - account.withdraw("user", "password", 0.0) - } - assertThat(exception).contains("Invalid withdrawal amount") -} +**5. Edge Cases:** +- testWithdraw_emptyUsername_throwsException +- testWithdraw_emptyPassword_throwsException +- testWithdraw_emptyAmount_throwsException +- testWithdraw_noBankData_initializationError -@Test -fun testWithdraw_amountGreaterThanBalance_throwsException() { - val account = BankAccount(1000.0, "user", "password") +### 3. Write the Tests Based on Mapped Behaviors - val exception = assertThrows { - account.withdraw("user", "password", 1500.0) - } - assertThat(exception).contains("Invalid withdrawal amount") -} -``` +Goal: Implement the test cases using your mapping as a guide. -### 5. Testing Default Values - -Verify the API's behaviour with different receipt parameter values. +**Test Samples:** ```kotlin @Test -fun testWithdraw_withDefaultReceipt_noReceiptPrinted() { +fun testWithdraw_validCredentials_outputsCorrectBalance() { val account = BankAccount(1000.0, "user", "password") + val output = ByteArrayOutputStream() System.setOut(PrintStream(output)) account.withdraw("user", "password", 200.0) - val outputLines = output.toString().trim() - assertThat(outputLines).doesNotContain("Receipt: Withdrew 200.0. Current balance: 800.0") + assertThat(output.toString().trim()).contains("Withdrew 200.0. New balance is 800.0") System.setOut(System.out) } @Test -fun testWithdraw_withNeedReceipt_receiptPrinted() { +fun testWithdraw_invalidUsername_throwsInvalidCredentialsException() { val account = BankAccount(1000.0, "user", "password") - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) - - account.withdraw("user", "password", 200.0, needReceipt = true) - val outputLines = output.toString().trim() - assertThat(outputLines).contains("Receipt: Withdrew 200.0. Current balance: 800.0") - System.setOut(System.out) + val exception = assertThrows { + account.withdraw("invalidUser", "password", 200.0) + } + assertThat(exception).contains("Invalid credentials") } @Test -fun testWithdraw_withoutNeedReceipt_noReceiptPrinted() { +fun testWithdraw_withNeedReceipt_receiptPrinted() { val account = BankAccount(1000.0, "user", "password") + val output = ByteArrayOutputStream() System.setOut(PrintStream(output)) - account.withdraw("user", "password", 200.0, needReceipt = false) + account.withdraw("user", "password", 200.0, needReceipt = true) - val outputLines = output.toString().trim() - assertThat(outputLines).doesNotContain("Receipt: Withdrew 200.0. Current balance: 800.0") + assertThat(output.toString().trim()).contains("Receipt: Withdrew 200.0. Current balance: 800.0") System.setOut(System.out) } -``` - -These tests check if the receipt value is correctly processed. They ensure that receipts are printed when requested and not printed otherwise. - -### 6. Testing Validity of File - -Testing how an API handles file inputs is crucial because: - -1. File Format Validation: Ensures that only acceptable file formats are processed, avoiding potential errors or security issues. -2. File Existence: Confirms that the API properly handles cases where files are missing, preventing unexpected failures. -3. File Processing: Validates that the API processes files correctly when they are valid, ensuring proper functionality. -**Test:** - -a. Testing with Invalid File Format - -```kotlin @Test -fun testWithdraw_withInvalidFileFormat_throwsException() { +fun testWithdraw_withInvalidFileFormat_throwsInvalidFileFormatException() { val account = BankAccount(1000.0, "user", "password") - val invalidFile = File("invalidFile.txt") // File with an invalid format + val invalidFile = File("invalid.txt") - val exception = assertThrows { - account.withdraw("user", "password", 200.0, file = invalidFile) + val exception = assertThrows { + account.withdraw("user", "password", invalidFile, 200.0) } assertThat(exception).contains("Invalid file format") } -``` - -b. Testing with Unavailable File - -```kotlin -@Test -fun testWithdraw_withUnavailableFile_throwsException() { - val account = BankAccount(1000.0, "user", "password") - val unavailableFile = File("nonExistentFile.pdf") // File that does not exist - - val exception = assertThrows { - account.withdraw("user", "password", 200.0, file = unavailableFile) - } - assertThat(exception).contains("File not found") -} -``` - -c. Testing with Valid File -```kotlin @Test -fun testWithdraw_withValidFile_processesFile() { +fun testWithdraw_withDefaultReceipt_noReceiptPrinted() { val account = BankAccount(1000.0, "user", "password") - val validFile = File("passbook.pdf") - validFile.createNewFile() // Ensure the file exists for the test val output = ByteArrayOutputStream() System.setOut(PrintStream(output)) - account.withdraw("user", "password", 200.0, file = validFile) + account.withdraw("user", "password", 200.0) - assertThat(output.toString().trim()).isEqualTo("Processing file: passbook.pdf") + assertThat(output.toString().trim()).doesNotContain("Receipt:") System.setOut(System.out) } -``` -These tests collectively cover critical aspects of file handling, ensuring robust and reliable API functionality. - -### 7. Testing Absence of Unwanted Output - -In addition to validating correct handling of valid and invalid files, it's also important to ensure that unwanted output or behavior does not occur. - -**Test:** - -a. Ensure No Processing Message for Missing File - -```kotlin -@Test -fun testWithdraw_withNoFile_producesNoFileProcessingOutput() { - val account = BankAccount(1000.0, "user", "password") - - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) - - account.withdraw("user", "password", 200.0) // No file provided - - assertThat(output.toString().trim()).doesNotContain("Processing file") - System.setOut(System.out) -} ``` -This test ensures that no unwanted methods, such as file processing, are called when the file parameter is not provided, thereby verifying that the API behaves as expected without unnecessary actions. - -b. Ensure No Receipt Message for Default Value - -```kotlin -@Test -fun testWithdraw_withDefaultReceipt_noReceiptPrinted() { - val account = BankAccount(1000.0, "user", "password") - val output = ByteArrayOutputStream() - System.setOut(PrintStream(output)) +### 4. Review and Refine - account.withdraw("user", "password", 200.0) // Using default receipt flag (false) +Goal: Ensure all scenarios are covered and tests are effective. - assertThat(output.toString().trim()).doesNotContain("Receipt:") - System.setOut(System.out) -} -``` +1. Cross-Reference: Verify that all behaviors from the mapping are tested. +2. Add Additional Cases: Incorporate any edge cases or additional scenarios discovered during review. +3. Refactor Tests: Ensure tests are clear, maintainable, and provide meaningful results. -This test verifies that no receipt message is output when the default receipt flag is in use, ensuring that the API respects the default behavior and does not produce unintended output. +By systematically analyzing the public API, mapping expected behaviors to test cases, and writing tests based on these mappings, you can ensure comprehensive and effective testing. This structured approach helps in covering all scenarios and provides clarity on how to test the API thoroughly. -### 8. Testing a Single Outcome in Multiple Ways +## 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. From fba4b8657152a92001a16e7b86f78e1dab49a96f Mon Sep 17 00:00:00 2001 From: Rd Date: Sun, 18 Aug 2024 03:24:31 +0530 Subject: [PATCH 33/64] Split code coverage workflow to 2 operations to handle analysis, evaluation and generation to one workflow and isolating the comment functionality to a different workflow due to permission issues that comes with using pull_request The new workflow will now trigger since it does not have any target to refer to and will only be triggered once it gets merged into the develop --- .github/workflows/code_coverage.yml | 47 ++++------------- .github/workflows/coverage_report.yml | 76 +++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 36 deletions(-) create mode 100644 .github/workflows/coverage_report.yml diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index cbb15b1581f..8baa75f34b1 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -11,27 +11,24 @@ on: # Push events on develop branch - develop -permissions: - pull-requests: write - concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true jobs: -# check_unit_tests_completed: -# name: Check unit test completed -# runs-on: ubuntu-latest -# steps: -# - name: Wait for unit tests to checks -# uses: ArcticLampyrid/action-wait-for-workflow@v1.2.0 -# with: -# workflow: unit_tests.yml -# sha: auto + check_unit_tests_completed: + name: Check unit test completed + runs-on: ubuntu-latest + steps: + - name: Wait for unit tests to checks + uses: ArcticLampyrid/action-wait-for-workflow@v1.2.0 + with: + workflow: unit_tests.yml + sha: auto compute_changed_files: name: Compute changed files -# needs: check_unit_tests_completed + needs: check_unit_tests_completed runs-on: ubuntu-20.04 outputs: matrix: ${{ steps.compute-file-matrix.outputs.matrix }} @@ -309,32 +306,10 @@ jobs: name: final-coverage-report path: coverage_reports/CoverageReport.md - publish_coverage_report: - name: Publish Code Coverage Report - needs: evaluate-code-coverage-reports - if: ${{ needs.compute_changed_files.outputs.can_skip_files != 'true' && !cancelled()}} - permissions: - pull-requests: write - - # 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. - runs-on: ubuntu-latest - steps: - - name: Download Generated Markdown Report - uses: actions/download-artifact@v4 - with: - name: final-coverage-report - - - name: Upload Coverage Report as PR Comment - uses: peter-evans/create-or-update-comment@v1 - with: - issue-number: ${{ github.event.pull_request.number }} - body-path: 'CoverageReport.md' - # Reference: https://github.community/t/127354/7. check_coverage_results: name: Check Code Coverage Results - needs: [ compute_changed_files, code_coverage_run, publish_coverage_report, evaluate-code-coverage-reports ] + needs: [ 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() }} diff --git a/.github/workflows/coverage_report.yml b/.github/workflows/coverage_report.yml new file mode 100644 index 00000000000..9d034afc175 --- /dev/null +++ b/.github/workflows/coverage_report.yml @@ -0,0 +1,76 @@ +# Contains jobs corresponding to publishing coverage reports generated by code_coverage.yml. + +name: Coverage Report + +# Controls when the action will run. Triggers the workflow on pull request events +# (assigned, opened, synchronize, reopened) + +on: + pull_request_target: + types: [opened, synchronize, reopened] + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + check_code_coverage_completed: + name: Check code coverage completed + runs-on: ubuntu-latest + steps: + - name: Wait for code coverage to complete + id: wait-for-coverage + uses: ArcticLampyrid/action-wait-for-workflow@v1.2.0 + with: + workflow: code_coverage.yml + sha: auto + allowed-conclusions: | + success + failure + neutral + + comment_coverage_report: + name: Comment Code Coverage Report + needs: check_code_coverage_completed + permissions: + pull-requests: write + runs-on: ubuntu-latest + steps: + - name: Find CI workflow run for PR + id: find-workflow-run + uses: actions/github-script@v7 + continue-on-error: true + with: + script: | + const { owner, repo } = context.repo; + const runsResponse = await github.rest.actions.listWorkflowRuns({ + owner, + repo, + workflow_id: 'code_coverage.yml', + event: 'pull_request', + head_sha: '${{ github.event.pull_request.head.sha }}', + }); + + const runs = runsResponse.data.workflow_runs; + runs.sort((a, b) => new Date(b.created_at).getTime() - new Date(a.created_at).getTime()); + + const run = runs[0]; + if(!run) { + core.setFailed('Could not find a succesful workflow run for the PR'); + return; + } + + core.setOutput('run-id', run.id); + + - name: Download Generated Markdown Report + uses: actions/download-artifact@v4 + with: + name: final-coverage-report + github-token: ${{ secrets.GITHUB_TOKEN }} + run-id: ${{ steps.find-workflow-run.outputs.run-id }} + + - name: Upload Coverage Report as PR Comment + uses: peter-evans/create-or-update-comment@v4 + with: + issue-number: ${{ github.event.pull_request.number }} + body-path: 'CoverageReport.md' From 1770f0abfc606b0f603cd81f87f6a9752ea77e26 Mon Sep 17 00:00:00 2001 From: Rd Date: Sun, 18 Aug 2024 03:28:48 +0530 Subject: [PATCH 34/64] Reverting the new line change in CoverageReporter - introduced for testing, reverting since no test can be performed until pull_request_target is in base branch --- .../oppia/android/scripts/coverage/reporter/CoverageReporter.kt | 1 - 1 file changed, 1 deletion(-) 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 2d668e6bf05..b27ec9283ce 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 @@ -690,4 +690,3 @@ private fun loadTestFileExemptionsProto( }.build() } } - From 41cda2a18467e3d072ab045eae7c106217c031fe Mon Sep 17 00:00:00 2001 From: Rd Date: Sun, 18 Aug 2024 12:26:19 +0530 Subject: [PATCH 35/64] Added a reference footnote to the Oppia Android Code Coverage Wiki page --- .../coverage/reporter/CoverageReporter.kt | 12 ++++++++- .../scripts/coverage/RunCoverageTest.kt | 26 +++++++++++++++++++ .../coverage/reporter/CoverageReporterTest.kt | 18 +++++++++++++ 3 files changed, 55 insertions(+), 1 deletion(-) 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 b27ec9283ce..0fef8cb9f54 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 @@ -496,6 +496,15 @@ class CoverageReporter( } } + val wikiPageLinkNote = buildString { + val wikiPageReferenceNote = ">To learn more, visit the [Oppia Android Code Coverage]" + + "(https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage) wiki page" + append("\n") + append("#") + append("\n") + append(wikiPageReferenceNote) + } + val finalReportText = "## Coverage Report\n\n" + "### Results\n" + "Number of files assessed: ${coverageReportContainer.coverageReportList.size}\n" + @@ -505,7 +514,8 @@ class CoverageReporter( failureMarkdownTable + failureMarkdownEntries + successMarkdownEntries + - testFileExemptedSection + testFileExemptedSection + + wikiPageLinkNote val finalReportOutputPath = mdReportOutputPath ?.let { it } 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 864ff9beb51..a724341b2a6 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt @@ -90,6 +90,7 @@ class RunCoverageTest { append("| File | Failure Reason | Status |\n") append("|------|----------------|--------|\n") append("| ${getFilenameAsDetailsSummary(sampleFile)} | $failureMessage | :x: |") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) @@ -229,6 +230,7 @@ class RunCoverageTest { append("\n\n") append(exemptionsReferenceNote) append("") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedResult) @@ -278,6 +280,7 @@ class RunCoverageTest { append("\n\n") append(exemptionsReferenceNote) append("") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedResult) @@ -385,6 +388,7 @@ class RunCoverageTest { append("| File | Failure Reason | Status |\n") append("|------|----------------|--------|\n") append("| //coverage/example:AddNumsTest | $failureMessage | :x: |") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) @@ -469,6 +473,7 @@ class RunCoverageTest { append("| File | Failure Reason | Status |\n") append("|------|----------------|--------|\n") append("| //coverage/test/java/com/example:SubNumsTest | $failureMessage | :x: |") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) @@ -655,6 +660,7 @@ class RunCoverageTest { ":white_check_mark: | $MIN_THRESHOLD% |\n\n" ) append("") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedResult) @@ -843,6 +849,7 @@ class RunCoverageTest { "| ${getFilenameAsDetailsSummary(filePathList.get(0))} | 0.00% | 0 / 4 | " + ":x: | $MIN_THRESHOLD% |\n" ) + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedResult) @@ -934,6 +941,7 @@ class RunCoverageTest { ":x: | 101% _*_ |" ) append("\n\n>**_*_** represents tests with custom overridden pass/fail coverage thresholds") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedResult) @@ -1021,6 +1029,7 @@ class RunCoverageTest { ) append("\n\n>**_*_** represents tests with custom overridden pass/fail coverage thresholds\n") append("") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedResult) @@ -1090,6 +1099,7 @@ class RunCoverageTest { ":white_check_mark: | $MIN_THRESHOLD% |\n\n" ) append("") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedResult) @@ -1162,6 +1172,7 @@ class RunCoverageTest { append("\n\n") append(exemptionsReferenceNote) append("") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedResult) @@ -1238,6 +1249,7 @@ class RunCoverageTest { append("\n\n") append(exemptionsReferenceNote) append("") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedResult) @@ -1333,6 +1345,7 @@ class RunCoverageTest { append("\n\n") append(exemptionsReferenceNote) append("") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedResult) @@ -1438,6 +1451,7 @@ class RunCoverageTest { append("\n\n") append(exemptionsReferenceNote) append("") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedResult) @@ -1642,6 +1656,7 @@ class RunCoverageTest { ":white_check_mark: | $MIN_THRESHOLD% |\n\n" ) append("") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedResult) @@ -1722,6 +1737,7 @@ class RunCoverageTest { ":white_check_mark: | $MIN_THRESHOLD% |\n\n" ) append("") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedResult) @@ -2282,6 +2298,7 @@ class RunCoverageTest { "3 / 4 | :white_check_mark: | $MIN_THRESHOLD% |\n\n" ) append("") + append(oppiaCoverageWikiPageLinkNote) } return markdownText @@ -2584,6 +2601,15 @@ class RunCoverageTest { return "
$fileName$additionalDataPart$filePath
" } + private val oppiaCoverageWikiPageLinkNote = buildString { + val wikiPageReferenceNote = ">To learn more, visit the [Oppia Android Code Coverage]" + + "(https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage) wiki page" + append("\n") + append("#") + append("\n") + append(wikiPageReferenceNote) + } + private fun loadCoverageReportProto( coverageReportProto: String ): CoverageReport { 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 cfbfb5d45b2..264dbc11972 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 @@ -75,6 +75,7 @@ class CoverageReporterTest { "| 100.00% | 10 / 10 | :white_check_mark: | $MIN_THRESHOLD% |\n\n" ) append("") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) @@ -116,6 +117,7 @@ class CoverageReporterTest { "| ${getFilenameAsDetailsSummary(filename)} | " + "0.00% | 0 / 10 | :x: | $MIN_THRESHOLD% |\n" ) + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) @@ -152,6 +154,7 @@ class CoverageReporterTest { append("| File | Failure Reason | Status |\n") append("|------|----------------|--------|\n") append("| ://bazelTestTarget | Failure Message | :x: |") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) @@ -209,6 +212,7 @@ class CoverageReporterTest { append("\n\n") append(exemptionsReferenceNote) append("") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) @@ -267,6 +271,7 @@ class CoverageReporterTest { append("\n\n") append(exemptionsReferenceNote) append("") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) @@ -319,6 +324,7 @@ class CoverageReporterTest { "20.00% | 2 / 10 | :x: | 101% _*_ |\n" ) append("\n>**_*_** represents tests with custom overridden pass/fail coverage thresholds") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) @@ -374,6 +380,7 @@ class CoverageReporterTest { ) append("\n>**_*_** represents tests with custom overridden pass/fail coverage thresholds\n") append("") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) @@ -483,6 +490,7 @@ class CoverageReporterTest { append("\n\n") append(exemptionsReferenceNote) append("") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) @@ -979,6 +987,7 @@ class CoverageReporterTest { "| 100.00% | 10 / 10 | :white_check_mark: | $MIN_THRESHOLD% |\n\n" ) append("") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) @@ -1001,6 +1010,15 @@ class CoverageReporterTest { return "
$fileName$additionalDataPart$filePath
" } + private val oppiaCoverageWikiPageLinkNote = buildString { + val wikiPageReferenceNote = ">To learn more, visit the [Oppia Android Code Coverage]" + + "(https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage) wiki page" + append("\n") + append("#") + append("\n") + append(wikiPageReferenceNote) + } + private fun createTestFileExemptionsProtoFile(testFileExemptions: TestFileExemptions): String { return tempFolder.newFile("test_file_exemptions.pb").also { it.outputStream().use(testFileExemptions::writeTo) From de406920b909ba0cba658cf2e1298fc0f945ddc5 Mon Sep 17 00:00:00 2001 From: Rd Date: Sun, 18 Aug 2024 23:42:09 +0530 Subject: [PATCH 36/64] Evaluation job moved to the coverage report workflow to aid in skip jobs and pr number missing issues --- .github/workflows/code_coverage.yml | 88 ++--------------- .github/workflows/coverage_report.yml | 97 +++++++++++++++++-- .../coverage/reporter/CoverageReporter.kt | 36 ++++--- .../scripts/coverage/RunCoverageTest.kt | 8 +- .../coverage/reporter/CoverageReporterTest.kt | 40 +++++++- 5 files changed, 163 insertions(+), 106 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 8baa75f34b1..6d79ba9a4a7 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -16,19 +16,19 @@ concurrency: cancel-in-progress: true jobs: - check_unit_tests_completed: - name: Check unit test completed - runs-on: ubuntu-latest - steps: - - name: Wait for unit tests to checks - uses: ArcticLampyrid/action-wait-for-workflow@v1.2.0 - with: - workflow: unit_tests.yml - sha: auto + # check_unit_tests_completed: + # name: Check unit test completed + # runs-on: ubuntu-latest + # steps: + # - name: Wait for unit tests to checks + # uses: ArcticLampyrid/action-wait-for-workflow@v1.2.0 + # with: + # workflow: unit_tests.yml + # sha: auto compute_changed_files: name: Compute changed files - needs: check_unit_tests_completed + # needs: check_unit_tests_completed runs-on: ubuntu-20.04 outputs: matrix: ${{ steps.compute-file-matrix.outputs.matrix }} @@ -254,71 +254,3 @@ jobs: with: name: coverage-report-${{ env.SHARD_NAME }} # Saving with unique names to avoid conflict path: coverage_reports - - evaluate-code-coverage-reports: - name: Evaluate Code Coverage Reports - runs-on: ubuntu-20.04 - needs: code_coverage_run - if: ${{ needs.compute_changed_files.outputs.can_skip_files != 'true' }} - env: - CACHE_DIRECTORY: ~/.bazel_cache - steps: - - uses: actions/checkout@v2 - - - name: Download Coverage Report Artifacts - uses: actions/download-artifact@v4 - with: - path: coverage-report-artifact - pattern: coverage-report-* - merge-multiple: true - - - name: Filter Coverage Reports - run: | - # Find all coverage_report.pb files in the current directory and subdirectories - PB_FILES_LIST=($(find . -name "coverage_report.pb" -type f -print0 | xargs -0 -n 1 echo)) - echo "${PB_FILES_LIST[@]}" > pb_files.txt - - - name: Set up Bazel - uses: abhinavsingh/setup-bazel@v3 - with: - version: 6.5.0 - - - uses: actions/cache@v2 - id: scripts_cache - with: - path: ${{ env.CACHE_DIRECTORY }} - key: ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel-scripts-${{ github.sha }} - restore-keys: | - ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel-scripts- - ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel- - - - name: Set up build environment - uses: ./.github/actions/set-up-android-bazel-build-environment - - - name: Generate Markdown Coverage Report - run: | - bazel run //scripts:coverage_reporter -- $(pwd) pb_files.txt - - - name: Upload Generated Markdown Report - uses: actions/upload-artifact@v4 - if: ${{ !cancelled() }} # IMPORTANT: Upload reports regardless of success or failure status - with: - name: final-coverage-report - path: coverage_reports/CoverageReport.md - - # 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 ] - # 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 coverages passed - if: ${{ needs.compute_changed_files.outputs.can_skip_files != 'true' && needs.code_coverage_run.result != 'success' }} - run: exit 1 - - - name: Check that coverage status is passed - if: ${{ needs.compute_changed_files.outputs.can_skip_files != 'true' && needs.evaluate-code-coverage-reports.result != 'success' }} - run: exit 1 diff --git a/.github/workflows/coverage_report.yml b/.github/workflows/coverage_report.yml index 9d034afc175..d49430ff069 100644 --- a/.github/workflows/coverage_report.yml +++ b/.github/workflows/coverage_report.yml @@ -7,7 +7,7 @@ name: Coverage Report on: pull_request_target: - types: [opened, synchronize, reopened] + types: [assigned, opened, synchronize, reopened] concurrency: group: ${{ github.workflow }}-${{ github.ref }} @@ -27,21 +27,25 @@ jobs: allowed-conclusions: | success failure - neutral - comment_coverage_report: - name: Comment Code Coverage Report + evaluate-code-coverage-reports: + name: Evaluate Code Coverage Reports + runs-on: ubuntu-20.04 + outputs: + pb_file_empty: ${{ steps.filter-coverage-reports.outputs.pb_file_empty }} needs: check_code_coverage_completed - permissions: - pull-requests: write - runs-on: ubuntu-latest + env: + CACHE_DIRECTORY: ~/.bazel_cache steps: - - name: Find CI workflow run for PR + - uses: actions/checkout@v2 + + - name: Find CI workflow ru for PR id: find-workflow-run uses: actions/github-script@v7 continue-on-error: true with: script: | + // Find the last successful workflow run for the current PR's head const { owner, repo } = context.repo; const runsResponse = await github.rest.actions.listWorkflowRuns({ owner, @@ -62,15 +66,88 @@ jobs: core.setOutput('run-id', run.id); - - name: Download Generated Markdown Report + + - name: Download Coverage Report Artifacts uses: actions/download-artifact@v4 with: - name: final-coverage-report + path: coverage-report-artifact + pattern: coverage-report-* + merge-multiple: true github-token: ${{ secrets.GITHUB_TOKEN }} run-id: ${{ steps.find-workflow-run.outputs.run-id }} + - name: Filter Coverage Reports + id: filter-coverage-reports + run: | + # Find all coverage_report.pb files in the current directory and subdirectories + PB_FILES_LIST=($(find . -name "coverage_report.pb" -type f -print0 | xargs -0 -n 1 echo)) + echo "${PB_FILES_LIST[@]}" > pb_files.txt + if [ ! -s pb_files.txt ]; then + echo "::set-output name=pb_file_empty::false" + else + echo "::set-output name=pb_file_empty::true" + echo "No coverage report generated. If this is wrong, you can add '[RunAllTests]' to the PR title to force a run." + fi + + - name: Set up Bazel + uses: abhinavsingh/setup-bazel@v3 + with: + version: 6.5.0 + + - uses: actions/cache@v2 + id: scripts_cache + with: + path: ${{ env.CACHE_DIRECTORY }} + key: ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel-scripts-${{ github.sha }} + restore-keys: | + ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel-scripts- + ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel- + + - name: Set up build environment + uses: ./.github/actions/set-up-android-bazel-build-environment + + - name: Generate Markdown Coverage Report + run: | + bazel run //scripts:coverage_reporter -- $(pwd) pb_files.txt + + - name: Upload Generated Markdown Report + uses: actions/upload-artifact@v4 + if: ${{ !cancelled() }} # IMPORTANT: Upload reports regardless of success or failure status + with: + name: final-coverage-report + path: coverage_reports/CoverageReport.md + + comment_coverage_report: + name: Comment Code Coverage Report + needs: evaluate-code-coverage-reports + permissions: + pull-requests: write + + # 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-latest + steps: + - name: Download Generated Markdown Report + uses: actions/download-artifact@v4 + with: + name: final-coverage-report + - name: Upload Coverage Report as PR Comment uses: peter-evans/create-or-update-comment@v4 with: issue-number: ${{ github.event.pull_request.number }} body-path: 'CoverageReport.md' + + # Reference: https://github.community/t/127354/7. + check_coverage_results: + name: Check Code Coverage Results + needs: [ evaluate-code-coverage-reports, comment_coverage_report ] + # 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: ${{ needs.compute_changed_files.outputs.can_skip_files != 'true' && !cancelled()}} + runs-on: ubuntu-20.04 + steps: + - name: Check that coverage status is passed + if: ${{ needs.evaluate-code-coverage-reports.outputs.pb_file_empty != 'true' && needs.evaluate-code-coverage-reports.result != 'success' }} + run: exit 1 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 0fef8cb9f54..c192a9d110d 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 @@ -496,26 +496,38 @@ class CoverageReporter( } } + val skipCoverageReportText = buildString { + append("## Coverage Report\n") + append("### Results\n") + append("Coverage Analysis: **SKIP** :next_track_button:\n\n") + append("_This PR did not introduce any changes to Kotlin source or test files._\n\n") + append("#\n") + append("> To learn more, visit the [Oppia Android Code Coverage](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage) wiki page") + } + val wikiPageLinkNote = buildString { val wikiPageReferenceNote = ">To learn more, visit the [Oppia Android Code Coverage]" + "(https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage) wiki page" - append("\n") + append("\n\n") append("#") append("\n") append(wikiPageReferenceNote) } - val finalReportText = "## Coverage Report\n\n" + - "### Results\n" + - "Number of files assessed: ${coverageReportContainer.coverageReportList.size}\n" + - "Overall Coverage: **${"%.2f".format(calculateOverallCoveragePercentage())}%**\n" + - "Coverage Analysis: $status\n" + - "##" + - failureMarkdownTable + - failureMarkdownEntries + - successMarkdownEntries + - testFileExemptedSection + - wikiPageLinkNote + val finalReportText = coverageReportContainer.coverageReportList.takeIf { it.isNotEmpty() } + ?.let { + "## Coverage Report\n\n" + + "### Results\n" + + "Number of files assessed: ${coverageReportContainer.coverageReportList.size}\n" + + "Overall Coverage: **${"%.2f".format(calculateOverallCoveragePercentage())}%**\n" + + "Coverage Analysis: $status\n" + + "##" + + failureMarkdownTable + + failureMarkdownEntries + + successMarkdownEntries + + testFileExemptedSection + + wikiPageLinkNote + } ?: skipCoverageReportText val finalReportOutputPath = mdReportOutputPath ?.let { it } 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 a724341b2a6..7ef0183f00d 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/coverage/RunCoverageTest.kt @@ -2592,19 +2592,17 @@ class RunCoverageTest { } private fun getFilenameAsDetailsSummary( - filePath: String, - additionalData: String? = null + filePath: String ): String { val fileName = filePath.substringAfterLast("/") - val additionalDataPart = additionalData?.let { " - $it" } ?: "" - return "
$fileName$additionalDataPart$filePath
" + return "
$fileName$filePath
" } private val oppiaCoverageWikiPageLinkNote = buildString { val wikiPageReferenceNote = ">To learn more, visit the [Oppia Android Code Coverage]" + "(https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage) wiki page" - append("\n") + append("\n\n") append("#") append("\n") append(wikiPageReferenceNote) 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 264dbc11972..2014f5f32a3 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 @@ -916,6 +916,22 @@ class CoverageReporterTest { .contains("Coverage Analysis$BOLD$RED FAILED$RESET") } + @Test + fun testCoverageReporter_emptyCoverageProtoFile_passesCoverageStatus() { + System.setOut(PrintStream(outContent)) + + val protoListTextPath = "protoList.txt" + tempFolder.newFile(protoListTextPath) + + main( + "${tempFolder.root}", + protoListTextPath + ) + + assertThat(outContent.toString().trim()) + .contains("Coverage Analysis$BOLD$GREEN PASSED$RESET") + } + @Test fun testCoverageReporter_listOfCoverageProtoPath_generatesMarkdownReport() { val successProtoPath = "successCoverageReport.pb" @@ -993,6 +1009,28 @@ class CoverageReporterTest { assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) } + @Test + fun testCoverageReporter_emptyCoverageProtoFile_generatesSkipCoverageReport() { + val protoListTextPath = "protoList.txt" + tempFolder.newFile(protoListTextPath) + + main( + "${tempFolder.root}", + protoListTextPath + ) + + val expectedMarkdown = buildString { + append("## Coverage Report\n") + append("### Results\n") + append("Coverage Analysis: **SKIP** :next_track_button:\n\n") + append("_This PR did not introduce any changes to Kotlin source or test files._\n\n") + append("#\n") + append("> To learn more, visit the [Oppia Android Code Coverage](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage) wiki page") + } + + assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) + } + private fun readFinalMdReport(): String { return File( "${tempFolder.root}" + @@ -1013,7 +1051,7 @@ class CoverageReporterTest { private val oppiaCoverageWikiPageLinkNote = buildString { val wikiPageReferenceNote = ">To learn more, visit the [Oppia Android Code Coverage]" + "(https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage) wiki page" - append("\n") + append("\n\n") append("#") append("\n") append(wikiPageReferenceNote) From 6989c6c3bd92db740a517418671e55d9b3824b64 Mon Sep 17 00:00:00 2001 From: Rd Date: Mon, 19 Aug 2024 00:57:58 +0530 Subject: [PATCH 37/64] Code Coverage Status Check on pb file presence, Lint fixes --- .github/workflows/coverage_report.yml | 2 +- .../coverage/reporter/CoverageReporter.kt | 17 ++++++++--------- .../coverage/reporter/CoverageReporterTest.kt | 5 ++--- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/.github/workflows/coverage_report.yml b/.github/workflows/coverage_report.yml index d49430ff069..19593b184e6 100644 --- a/.github/workflows/coverage_report.yml +++ b/.github/workflows/coverage_report.yml @@ -145,7 +145,7 @@ jobs: needs: [ evaluate-code-coverage-reports, comment_coverage_report ] # 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: ${{ needs.compute_changed_files.outputs.can_skip_files != 'true' && !cancelled()}} + if: ${{ needs.evaluate-code-coverage-reports.outputs.pb_file_empty != 'true' && !cancelled()}} runs-on: ubuntu-20.04 steps: - name: Check that coverage status is passed 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 c192a9d110d..31763392945 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 @@ -496,15 +496,6 @@ class CoverageReporter( } } - val skipCoverageReportText = buildString { - append("## Coverage Report\n") - append("### Results\n") - append("Coverage Analysis: **SKIP** :next_track_button:\n\n") - append("_This PR did not introduce any changes to Kotlin source or test files._\n\n") - append("#\n") - append("> To learn more, visit the [Oppia Android Code Coverage](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage) wiki page") - } - val wikiPageLinkNote = buildString { val wikiPageReferenceNote = ">To learn more, visit the [Oppia Android Code Coverage]" + "(https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage) wiki page" @@ -514,6 +505,14 @@ class CoverageReporter( append(wikiPageReferenceNote) } + val skipCoverageReportText = buildString { + append("## Coverage Report\n") + append("### Results\n") + append("Coverage Analysis: **SKIP** :next_track_button:\n\n") + append("_This PR did not introduce any changes to Kotlin source or test files._") + append(wikiPageLinkNote) + } + val finalReportText = coverageReportContainer.coverageReportList.takeIf { it.isNotEmpty() } ?.let { "## Coverage Report\n\n" + 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 2014f5f32a3..b0af56a1115 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 @@ -1023,9 +1023,8 @@ class CoverageReporterTest { append("## Coverage Report\n") append("### Results\n") append("Coverage Analysis: **SKIP** :next_track_button:\n\n") - append("_This PR did not introduce any changes to Kotlin source or test files._\n\n") - append("#\n") - append("> To learn more, visit the [Oppia Android Code Coverage](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage) wiki page") + append("_This PR did not introduce any changes to Kotlin source or test files._") + append(oppiaCoverageWikiPageLinkNote) } assertThat(readFinalMdReport()).isEqualTo(expectedMarkdown) From a2e2c7aa7ce5883af5c8ea486ccb7a2b19e904f6 Mon Sep 17 00:00:00 2001 From: Rd Date: Mon, 19 Aug 2024 09:59:08 +0530 Subject: [PATCH 38/64] Removed empty pb file output variable --- .github/workflows/code_coverage.yml | 20 ++++++++++---------- .github/workflows/coverage_report.yml | 12 ++---------- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 6d79ba9a4a7..d5f1792cf60 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -16,19 +16,19 @@ concurrency: cancel-in-progress: true jobs: - # check_unit_tests_completed: - # name: Check unit test completed - # runs-on: ubuntu-latest - # steps: - # - name: Wait for unit tests to checks - # uses: ArcticLampyrid/action-wait-for-workflow@v1.2.0 - # with: - # workflow: unit_tests.yml - # sha: auto + check_unit_tests_completed: + name: Check unit test completed + runs-on: ubuntu-latest + steps: + - name: Wait for unit tests to checks + uses: ArcticLampyrid/action-wait-for-workflow@v1.2.0 + with: + workflow: unit_tests.yml + sha: auto compute_changed_files: name: Compute changed files - # needs: check_unit_tests_completed + needs: check_unit_tests_completed runs-on: ubuntu-20.04 outputs: matrix: ${{ steps.compute-file-matrix.outputs.matrix }} diff --git a/.github/workflows/coverage_report.yml b/.github/workflows/coverage_report.yml index 19593b184e6..4a73c2d44fa 100644 --- a/.github/workflows/coverage_report.yml +++ b/.github/workflows/coverage_report.yml @@ -31,8 +31,6 @@ jobs: evaluate-code-coverage-reports: name: Evaluate Code Coverage Reports runs-on: ubuntu-20.04 - outputs: - pb_file_empty: ${{ steps.filter-coverage-reports.outputs.pb_file_empty }} needs: check_code_coverage_completed env: CACHE_DIRECTORY: ~/.bazel_cache @@ -82,12 +80,6 @@ jobs: # Find all coverage_report.pb files in the current directory and subdirectories PB_FILES_LIST=($(find . -name "coverage_report.pb" -type f -print0 | xargs -0 -n 1 echo)) echo "${PB_FILES_LIST[@]}" > pb_files.txt - if [ ! -s pb_files.txt ]; then - echo "::set-output name=pb_file_empty::false" - else - echo "::set-output name=pb_file_empty::true" - echo "No coverage report generated. If this is wrong, you can add '[RunAllTests]' to the PR title to force a run." - fi - name: Set up Bazel uses: abhinavsingh/setup-bazel@v3 @@ -145,9 +137,9 @@ jobs: needs: [ evaluate-code-coverage-reports, comment_coverage_report ] # 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: ${{ needs.evaluate-code-coverage-reports.outputs.pb_file_empty != 'true' && !cancelled()}} + if: ${{ !cancelled()}} runs-on: ubuntu-20.04 steps: - name: Check that coverage status is passed - if: ${{ needs.evaluate-code-coverage-reports.outputs.pb_file_empty != 'true' && needs.evaluate-code-coverage-reports.result != 'success' }} + if: ${{ needs.evaluate-code-coverage-reports.result != 'success' }} run: exit 1 From 1ae73a2a110177f2de9b6228bd444dc26de2a001 Mon Sep 17 00:00:00 2001 From: Rd Date: Mon, 19 Aug 2024 12:23:40 +0530 Subject: [PATCH 39/64] Removed concurrency as it fails every concurrent run of the workflow regardless of the origin PR since it utilizes pull_request_target --- .github/workflows/code_coverage.yml | 20 ++++++++++---------- .github/workflows/coverage_report.yml | 4 ---- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index d5f1792cf60..e49331bdc6a 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -16,19 +16,19 @@ concurrency: cancel-in-progress: true jobs: - check_unit_tests_completed: - name: Check unit test completed - runs-on: ubuntu-latest - steps: - - name: Wait for unit tests to checks - uses: ArcticLampyrid/action-wait-for-workflow@v1.2.0 - with: - workflow: unit_tests.yml - sha: auto + check_unit_tests_completed: + name: Check unit test completed + runs-on: ubuntu-latest + steps: + - name: Wait for unit tests to checks + uses: ArcticLampyrid/action-wait-for-workflow@v1.2.0 + with: + workflow: unit_tests.yml + sha: auto compute_changed_files: name: Compute changed files - needs: check_unit_tests_completed + needs: check_unit_tests_completed runs-on: ubuntu-20.04 outputs: matrix: ${{ steps.compute-file-matrix.outputs.matrix }} diff --git a/.github/workflows/coverage_report.yml b/.github/workflows/coverage_report.yml index 4a73c2d44fa..a3d32187ea6 100644 --- a/.github/workflows/coverage_report.yml +++ b/.github/workflows/coverage_report.yml @@ -9,10 +9,6 @@ on: pull_request_target: types: [assigned, opened, synchronize, reopened] -concurrency: - group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true - jobs: check_code_coverage_completed: name: Check code coverage completed From e04cd2b7da02dc3a836b713781b4af03b863131c Mon Sep 17 00:00:00 2001 From: Rd Date: Mon, 19 Aug 2024 12:26:57 +0530 Subject: [PATCH 40/64] Indentation fix --- .github/workflows/code_coverage.yml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index e49331bdc6a..7bdc9e19041 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -17,14 +17,14 @@ concurrency: jobs: check_unit_tests_completed: - name: Check unit test completed - runs-on: ubuntu-latest - steps: - - name: Wait for unit tests to checks - uses: ArcticLampyrid/action-wait-for-workflow@v1.2.0 - with: - workflow: unit_tests.yml - sha: auto + name: Check unit test completed + runs-on: ubuntu-latest + steps: + - name: Wait for unit tests to checks + uses: ArcticLampyrid/action-wait-for-workflow@v1.2.0 + with: + workflow: unit_tests.yml + sha: auto compute_changed_files: name: Compute changed files From ef924f18f8c98ed54dc07eb5afdda25ad2268568 Mon Sep 17 00:00:00 2001 From: Rd Date: Mon, 19 Aug 2024 14:53:26 +0530 Subject: [PATCH 41/64] Fixed concurrency issues with uploading comments --- .github/workflows/coverage_report.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/coverage_report.yml b/.github/workflows/coverage_report.yml index a3d32187ea6..ff01185a7a2 100644 --- a/.github/workflows/coverage_report.yml +++ b/.github/workflows/coverage_report.yml @@ -9,6 +9,10 @@ on: pull_request_target: types: [assigned, opened, synchronize, reopened] +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number }}-${{ github.ref }} + cancel-in-progress: true + jobs: check_code_coverage_completed: name: Check code coverage completed From a4c48d6dec31934b35fa4defafc113ad0c4f3661 Mon Sep 17 00:00:00 2001 From: Rd Date: Tue, 20 Aug 2024 08:26:06 +0530 Subject: [PATCH 42/64] Added missed hasMessageThat() assertion chain --- ...iting-tests-with-good-behavioural-coverage.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/wiki/Writing-tests-with-good-behavioural-coverage.md b/wiki/Writing-tests-with-good-behavioural-coverage.md index b6637e2deb1..85726db3536 100644 --- a/wiki/Writing-tests-with-good-behavioural-coverage.md +++ b/wiki/Writing-tests-with-good-behavioural-coverage.md @@ -452,7 +452,7 @@ fun testDivideNumbers_forZeroDenominator_throwsIllegalArgumentException() { val exception = assertThrows { divideNumbers(10, 0) } - assertThat(exception).contains("Denominator cannot be zero") + assertThat(exception).hasMessageThat().contains("Denominator cannot be zero") } ``` @@ -577,7 +577,7 @@ fun testBookTickets_withUnavailableSeats_throwsException() { val exception = assertThrows { booking.reserveSeat(0) } - assertThat(exception).contains("No seats are available. Please check other bookings for available seats.") + assertThat(exception).hasMessageThat().contains("No seats are available. Please check other bookings for available seats.") } ``` @@ -633,7 +633,7 @@ fun testBookTickets_withUnavailableSeats_throwsException() { val exception = assertThrows { booking.reserveSeat(0) } - assertThat(exception).contains("No seats are available. Please check other bookings for available seats.") + assertThat(exception).hasMessageThat().contains("No seats are available. Please check other bookings for available seats.") } ``` @@ -648,7 +648,7 @@ fun testConfirmPayment_withUnsuccessfulPayment_throwsException() { val exception = assertThrows { booking.confirmPayment(false) } - assertThat(exception).contains("Payment not successful. Please try again.") + assertThat(exception).hasMessageThat().contains("Payment not successful. Please try again.") } ``` @@ -688,7 +688,7 @@ fun testCalculateDiscount_forNegativePrice_throwsIllegalArgumentException() { val exception = assertThrows { calculateDiscount(-100.0, 10.0) } - assertThat(exception).contains("Price and discount cannot be negative") + assertThat(exception).hasMessageThat().contains("Price and discount cannot be negative") } @Test @@ -696,7 +696,7 @@ fun testCalculateDiscount_forNegativeDiscount_throwsIllegalArgumentException() { val exception = assertThrows { calculateDiscount(100.0, -10.0) } - assertThat(exception).contains("Price and discount cannot be negative") + assertThat(exception).hasMessageThat().contains("Price and discount cannot be negative") } ``` @@ -923,7 +923,7 @@ fun testWithdraw_invalidUsername_throwsInvalidCredentialsException() { val exception = assertThrows { account.withdraw("invalidUser", "password", 200.0) } - assertThat(exception).contains("Invalid credentials") + assertThat(exception).hasMessageThat().contains("Invalid credentials") } @Test @@ -947,7 +947,7 @@ fun testWithdraw_withInvalidFileFormat_throwsInvalidFileFormatException() { val exception = assertThrows { account.withdraw("user", "password", invalidFile, 200.0) } - assertThat(exception).contains("Invalid file format") + assertThat(exception).hasMessageThat().contains("Invalid file format") } @Test From f8da0f5280a37fc4e60277397137ee3c32d62700 Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 00:13:32 +0530 Subject: [PATCH 43/64] Test changes --- .../java/org/oppia/android/util/math/MathExpressionParser.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/utility/src/main/java/org/oppia/android/util/math/MathExpressionParser.kt b/utility/src/main/java/org/oppia/android/util/math/MathExpressionParser.kt index 7119665bfc4..9e8329d7370 100644 --- a/utility/src/main/java/org/oppia/android/util/math/MathExpressionParser.kt +++ b/utility/src/main/java/org/oppia/android/util/math/MathExpressionParser.kt @@ -725,8 +725,10 @@ class MathExpressionParser private constructor(private val parseContext: ParseCo inline fun consumeTokenOfType( missingError: () -> MathParsingError = { GenericError } ): MathParsingResult { + println("In consumer Token of type") val maybeToken = tokens.expectNextMatches { it is T } as? T return maybeToken?.let { token -> + println("Hit in may be Token") previousToken = token MathParsingResult.Success(token) } ?: missingError().toFailure() From 251cfd2a335865b5ceeadc48faedc98b46e94be0 Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 00:16:17 +0530 Subject: [PATCH 44/64] Dropping unnecessary changes committed with wrong branch refernce --- .../java/org/oppia/android/util/math/MathExpressionParser.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/utility/src/main/java/org/oppia/android/util/math/MathExpressionParser.kt b/utility/src/main/java/org/oppia/android/util/math/MathExpressionParser.kt index 9e8329d7370..7119665bfc4 100644 --- a/utility/src/main/java/org/oppia/android/util/math/MathExpressionParser.kt +++ b/utility/src/main/java/org/oppia/android/util/math/MathExpressionParser.kt @@ -725,10 +725,8 @@ class MathExpressionParser private constructor(private val parseContext: ParseCo inline fun consumeTokenOfType( missingError: () -> MathParsingError = { GenericError } ): MathParsingResult { - println("In consumer Token of type") val maybeToken = tokens.expectNextMatches { it is T } as? T return maybeToken?.let { token -> - println("Hit in may be Token") previousToken = token MathParsingResult.Success(token) } ?: missingError().toFailure() From c6ed248c89cf47ac84c3cdd04263d6abdb533a64 Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 00:23:58 +0530 Subject: [PATCH 45/64] Structuring test bodies, isEqualTo() --- ...ng-tests-with-good-behavioural-coverage.md | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/wiki/Writing-tests-with-good-behavioural-coverage.md b/wiki/Writing-tests-with-good-behavioural-coverage.md index 85726db3536..9b718e0ce06 100644 --- a/wiki/Writing-tests-with-good-behavioural-coverage.md +++ b/wiki/Writing-tests-with-good-behavioural-coverage.md @@ -11,7 +11,7 @@ - [Exception and Error Handling](#6-exception-and-error-handling) - [Absence of Unwanted Output](#7-absence-of-unwanted-output) - [Testing Public API](#testing-public-api) -- [Structuring Test Functionalities](#structuring-test-functionalities) +- [Structuring Test Bodies](#structuring-test-bodies) - [When and How to Divide Responsibilities](#1-when-and-how-to-divide-responsibilities) - [When Not to Divide Responsibilities](#2-when-not-to-divide-responsibilities) - [Importance of Descriptive Test Names](#3-importance-of-descriptive-test-names) @@ -44,7 +44,7 @@ A basic test case for line coverage might look like this: ```kotlin @Test fun testValidName() { // Tests line coverage by hitting the line where name is accessed - assertThat(getName("Alice"), equalTo("Alice")) + assertThat(getName("Alice")).isEqualTo("Alice") } ``` @@ -57,7 +57,7 @@ To ensure behavioral coverage, the test needs to verify various conditions: ```kotlin @Test fun testGetName_withDefaultValue_result(getName()) { // Default value when no name is provided - assertThat(getName(), equalTo(" ")) + assertThat(getName()).isEqualTo(" ") } @Test fun testGetName_withNullName_throwsException() { @@ -72,12 +72,12 @@ To ensure behavioral coverage, the test needs to verify various conditions: @Test fun testGetName_withEmptyName_result(getName("")) { // Empty string should use default value - assertThat(getName(""), equalTo(" ")) + assertThat(getName("")).isEqualTo(" ") } @Test fun testGetName_withWhitespaceName_result(getName(" ")) { // Whitespace name - assertThat(getName(" "), equalTo(" ")) + assertThat(getName(" ")).isEqualTo(" ") } ``` @@ -328,7 +328,7 @@ Positive Number: Verifies that the function correctly identifies positive number ```kotlin @Test fun testCheckNumber_forPositiveInput_returnsPositive() { - assertThat(checkNumber(5), equalTo("Positive")) + assertThat(checkNumber(5)).isEqualTo("Positive") } ``` @@ -337,7 +337,7 @@ Negative Number: Ensures that negative numbers are correctly classified. ```kotlin @Test fun testCheckNumber_forNegativeInput_returnsNegative() { - assertThat(checkNumber(-3), equalTo("Negative")) + assertThat(checkNumber(-3)).isEqualTo("Negative") } ``` @@ -346,7 +346,7 @@ Zero: Checks that zero is handled correctly. ```kotlin @Test fun testCheckNumber_forZeroInput_returnsZero() { - assertThat(checkNumber(0), equalTo("Zero")) + assertThat(checkNumber(0)).isEqualTo("Zero") } ``` @@ -355,7 +355,7 @@ Maximum Value: Tests the function with Int.MAX_VALUE to ensure it handles the up ```kotlin @Test fun testCheckNumber_forMaxValue_returnsPositive() { - assertThat(checkNumber(Int.MAX_VALUE), equalTo("Positive")) + assertThat(checkNumber(Int.MAX_VALUE)).isEqualTo("Positive") } ``` @@ -364,7 +364,7 @@ Minimum Value: Tests the function with Int.MIN_VALUE to ensure it handles the lo ```kotlin @Test fun testCheckNumber_forMinValue_returnsNegative() { - assertThat(checkNumber(Int.MIN_VALUE), equalTo("Negative")) + assertThat(checkNumber(Int.MIN_VALUE)).isEqualTo("Negative") } ``` @@ -404,22 +404,22 @@ Testing needs to be performed to cover all branches, paths and conditions. ```kotlin @Test fun testEvaluateAccess_forAdultMember_grantsAccess() { - assertThat(evaluateAccess(25, true), equalTo("Access granted")) + assertThat(evaluateAccess(25, true)).isEqualTo("Access granted") } @Test fun testEvaluateAccess_forAdultNonMember_requiresMembership() { - assertThat(evaluateAccess(30, false), equalTo("Membership required")) + assertThat(evaluateAccess(30, false)).isEqualTo("Membership required") } @Test fun testEvaluateAccess_forMinorMember_deniesAccess() { - assertThat(evaluateAccess(16, true), equalTo("Access denied")) + assertThat(evaluateAccess(16, true)).isEqualTo("Access denied") } @Test fun testEvaluateAccess_forminorNonMember_deniesAccess() { - assertThat(evaluateAccess(15, false), equalTo("Access denied")) + assertThat(evaluateAccess(15, false)).isEqualTo("Access denied") } ``` @@ -1066,7 +1066,7 @@ fun testWithdraw_withSufficientBalance_updatesBalanceCorrectly() { These tests cover various aspects of the withdrawal functionality, ensuring that the balance updates correctly with receipts, passbooks and output messages. Although the core functionality of withdrawing funds and updating the balance is consistent, it can be observed in multiple ways. Each test focuses on a specific verification method while ultimately validating the same core functionality. -# Structuring Test Functionalities +# Structuring Test Bodies In testing, it's crucial to ensure that your tests verify implementation code while maintaining clarity and readability. Tests validate the correctness of the code, but it is humans who verify the correctness of the test code itself. Therefore, striking a balance between clarity and conciseness in test writing is essential. From 8dd0eb2ff3a231c2bab0a29659aaf7dc2159ec14 Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 00:41:15 +0530 Subject: [PATCH 46/64] Removed sh highlighting on illustrations and replaced strings with contains --- wiki/Writing-tests-with-good-behavioural-coverage.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/wiki/Writing-tests-with-good-behavioural-coverage.md b/wiki/Writing-tests-with-good-behavioural-coverage.md index 9b718e0ce06..67737cbe470 100644 --- a/wiki/Writing-tests-with-good-behavioural-coverage.md +++ b/wiki/Writing-tests-with-good-behavioural-coverage.md @@ -89,7 +89,7 @@ While line coverage might reach 100% with a single test, it doesn’t ensure tha **Evaluation and Enhancement Flow:** -```sh +``` +-------------------+ | Coverage Analysis | @@ -262,7 +262,7 @@ fun testProcessOrder_allSteps_returnsCorrectMessage() { fun testProcessOrder_providedWithList_displaysListOfItems() { processOrder(listOf("Pizza", "Burger"), paymentMade = true) val output = outContent.toString().trim() - assertThat(output, containsString("Order: Pizza, Burger")) + assertThat(output).contains("Order: Pizza, Burger") } ``` @@ -273,7 +273,7 @@ fun testProcessOrder_providedWithList_displaysListOfItems() { fun testProcessOrder_forListItems_calculatesCorrectTotalPrice() { processOrder(listOf("Pizza", "Burger"), paymentMade = true) val output = outContent.toString().trim() - assertThat(output, containsString("Total: 20.0")) + assertThat(output).contains("Total: 20.0") } ``` @@ -284,7 +284,7 @@ fun testProcessOrder_forListItems_calculatesCorrectTotalPrice() { fun testProcessOrder_whenPaymentMade_displaysPaymentSuccess() { processOrder(listOf("Pizza", "Burger"), paymentMade = true) val output = outContent.toString().trim() - assertThat(output, containsString("Payment successful")) + assertThat(output).contains("Payment successful") } ``` @@ -295,7 +295,7 @@ fun testProcessOrder_whenPaymentMade_displaysPaymentSuccess() { fun testProcessOrder_whenNotPaymentMade_displaysPaymentPending() { processOrder(listOf("Pizza", "Burger"), paymentMade = false) val output = outContent.toString().trim() - assertThat(output, containsString("Payment pending")) + assertThat(output).contains("Payment pending") } ``` From c102d7f8880e4c0fbebe0e3ae2522a44b5006a96 Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 00:51:43 +0530 Subject: [PATCH 47/64] A public api, permalink, a bank account --- wiki/Writing-tests-with-good-behavioural-coverage.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/wiki/Writing-tests-with-good-behavioural-coverage.md b/wiki/Writing-tests-with-good-behavioural-coverage.md index 67737cbe470..e8b0bca5f81 100644 --- a/wiki/Writing-tests-with-good-behavioural-coverage.md +++ b/wiki/Writing-tests-with-good-behavioural-coverage.md @@ -490,7 +490,7 @@ This function checks various conditions on the input string text to determine wh The goal when testing domain errors is to ensure that the application correctly identifies and responds to these errors as part of its normal operation. -**Test samples from [FractionParserTest.kt](https://github.com/oppia/oppia-android/blob/develop/utility/src/test/java/org/oppia/android/util/math/FractionParserTest.kt):** +**Test samples from [FractionParserTest.kt](https://github.com/oppia/oppia-android/blob/f9106d91297abd09b24da25d5485deb8473b0125/utility/src/test/java/org/oppia/android/util/math/FractionParserTest.kt#L19):** ```kotlin @Test @@ -777,7 +777,7 @@ A public API (Application Programming Interface) refers to the set of methods, p Public APIs are essential because they provide a way to interact with the functionality of a class or module without exposing its internal workings. They define how external code can use the functionality offered by the class or module, ensuring that interactions are safe and predictable while keeping the internal implementation hidden and secure. -Let's consider the following example for a public API to withdraw money from the BankAccount. +Let's consider the following example for a public API to withdraw money from a BankAccount. ```kotlin class BankAccount( @@ -833,7 +833,7 @@ class BankAccount( The **`withdraw`** method serves as the single public entry point for withdrawing money from the account. It handles user validation, amount checking, optional file upload and printing of the receipt. By keeping the internal methods private, the class ensures that the operations are performed in a controlled manner while hiding the complexity of these operations from the user. -## How to Write Tests for Public API +## How to Write Tests for a Public API ```sh +---------+ +---------------+ +-----------+ +--------+ From e575034f264dce9ec250c2b26d85ec36ea45a5d2 Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 02:20:55 +0530 Subject: [PATCH 48/64] Replaced textual illustration with mermaid blocks --- ...ng-tests-with-good-behavioural-coverage.md | 46 +++++++++++-------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/wiki/Writing-tests-with-good-behavioural-coverage.md b/wiki/Writing-tests-with-good-behavioural-coverage.md index e8b0bca5f81..a09c46a7695 100644 --- a/wiki/Writing-tests-with-good-behavioural-coverage.md +++ b/wiki/Writing-tests-with-good-behavioural-coverage.md @@ -89,24 +89,32 @@ While line coverage might reach 100% with a single test, it doesn’t ensure tha **Evaluation and Enhancement Flow:** -``` - - +-------------------+ - | Coverage Analysis | - +-------------------+ - ↓ -+---------------------------+ +--------------------------------+ -| 100% Line Coverage | | Less Than 100% Coverage | -| Does not guarantee full | | Guarantees at least one | -| behavioural coverage | | important behaviour is missing | -+---------------------------+ +--------------------------------+ - - ↓ ↓ -+------------------------------+ +---------------------------------+ -| Add tests for lines that | | Find uncovered lines and add | -| misses behavioural coverages | <--- | tests that cover all behaviours | -| even if stated as covered | | and not just line coverages | -+------------------------------+ +---------------------------------+ +```mermaid +block-beta +columns 2 + A[["Coverage Analysis"]]:2 + block:Coverage1 + B["100% Line Coverage
Does not guarantee full
behavioural coverage"] + end + block:Coverage2 + C["Less Than 100% Coverage
Guarantees at least one
important behaviour is missing"] + end + block:Action1 + D["Add tests for lines that
miss behavioural coverages
even if stated as covered"] + end + block:Action2 + E["Find uncovered lines and add
tests that cover all behaviours
and not just line coverages"] + end + space + Coverage1 --> D + Coverage2 --> E + E --> D + + style A fill:#f3f3f3,stroke:#333,stroke-width:1px + style B fill:#e8f7c8,stroke:#333,stroke-width:1px + style C fill:#ffdad4,stroke:#333,stroke-width:1px + style D fill:#dcf5a4,stroke:#333,stroke-width:1px + style E fill:#e4f7ba,stroke:#333,stroke-width:1px ``` @@ -835,7 +843,7 @@ The **`withdraw`** method serves as the single public entry point for withdrawin ## How to Write Tests for a Public API -```sh +``` +---------+ +---------------+ +-----------+ +--------+ | Analyze | -> | Map Behaviors | -> | Add Tests | -> | Refine | +---------+ +---------------+ +-----------+ +--------+ From 89f9d9b2a855d89dbc1582188e368f275e2e763d Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 02:28:48 +0530 Subject: [PATCH 49/64] Replaced textual representation of procedure to test public api process with mermaid --- ...Writing-tests-with-good-behavioural-coverage.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/wiki/Writing-tests-with-good-behavioural-coverage.md b/wiki/Writing-tests-with-good-behavioural-coverage.md index a09c46a7695..cae078aa6fe 100644 --- a/wiki/Writing-tests-with-good-behavioural-coverage.md +++ b/wiki/Writing-tests-with-good-behavioural-coverage.md @@ -843,10 +843,16 @@ The **`withdraw`** method serves as the single public entry point for withdrawin ## How to Write Tests for a Public API -``` -+---------+ +---------------+ +-----------+ +--------+ -| Analyze | -> | Map Behaviors | -> | Add Tests | -> | Refine | -+---------+ +---------------+ +-----------+ +--------+ +```mermaid +graph LR + A[Analyze] --> B[Map Behaviors] + B --> C[Add Tests] + C --> D[Refine] + + style A fill:#ffffcf,stroke:#333,stroke-width:1px + style B fill:#fffea6,stroke:#333,stroke-width:1px + style C fill:#f0ffbf,stroke:#333,stroke-width:1px + style D fill:#e5ff91,stroke:#333,stroke-width:1px ``` From 489a08bb70e124ec58f912b2ffd3b9589f29a0ae Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 02:42:58 +0530 Subject: [PATCH 50/64] Represent Structuring and mapping with mermaid graphs --- ...iting-tests-with-good-behavioural-coverage.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/wiki/Writing-tests-with-good-behavioural-coverage.md b/wiki/Writing-tests-with-good-behavioural-coverage.md index cae078aa6fe..7d63562a6f2 100644 --- a/wiki/Writing-tests-with-good-behavioural-coverage.md +++ b/wiki/Writing-tests-with-good-behavioural-coverage.md @@ -1084,15 +1084,15 @@ These tests cover various aspects of the withdrawal functionality, ensuring that In testing, it's crucial to ensure that your tests verify implementation code while maintaining clarity and readability. Tests validate the correctness of the code, but it is humans who verify the correctness of the test code itself. Therefore, striking a balance between clarity and conciseness in test writing is essential. -```sh -+------------+ verify +--------------+ -| Tests | ---------> | Source Code | -+------------+  +--------------+ - -+------------+ verify +--------------+ -| Human | ----------> | Tests | -+------------+ +--------------+ +```mermaid +graph LR + A[  Tests ] -..->|verify| B[Source Code] + C[Humans] -..->|verify| D[  Tests  ] + style A fill:#e8e8e8,stroke:#333,stroke-width:1px + style B fill:#dcf5a4,stroke:#333,stroke-width:1px + style C fill:#e8e8e8,stroke:#333,stroke-width:1px + style D fill:#dcf5a4,stroke:#333,stroke-width:1px ``` Tests should focus on verifying the behavior of the implementation code, while humans should be able to easily understand and verify the correctness of the test code itself. From 509e3c38270a9787434b3e306ad86d124f144e4d Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 03:04:18 +0530 Subject: [PATCH 51/64] Common color codes for all graph illustrations --- ...ng-tests-with-good-behavioural-coverage.md | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/wiki/Writing-tests-with-good-behavioural-coverage.md b/wiki/Writing-tests-with-good-behavioural-coverage.md index 7d63562a6f2..33c3b69f8a6 100644 --- a/wiki/Writing-tests-with-good-behavioural-coverage.md +++ b/wiki/Writing-tests-with-good-behavioural-coverage.md @@ -111,10 +111,10 @@ columns 2 E --> D style A fill:#f3f3f3,stroke:#333,stroke-width:1px - style B fill:#e8f7c8,stroke:#333,stroke-width:1px + style B fill:#e8fcde,stroke:#333,stroke-width:1px style C fill:#ffdad4,stroke:#333,stroke-width:1px - style D fill:#dcf5a4,stroke:#333,stroke-width:1px - style E fill:#e4f7ba,stroke:#333,stroke-width:1px + style D fill:#d8fac5,stroke:#333,stroke-width:1px + style E fill:#e2fcd4,stroke:#333,stroke-width:1px ``` @@ -849,10 +849,10 @@ graph LR B --> C[Add Tests] C --> D[Refine] - style A fill:#ffffcf,stroke:#333,stroke-width:1px - style B fill:#fffea6,stroke:#333,stroke-width:1px - style C fill:#f0ffbf,stroke:#333,stroke-width:1px - style D fill:#e5ff91,stroke:#333,stroke-width:1px + style A fill:#f5f5f5,stroke:#333,stroke-width:1px + style B fill:#f2fced,stroke:#333,stroke-width:1px + style C fill:#e8fcde,stroke:#333,stroke-width:1px + style D fill:#dcf5d0,stroke:#333,stroke-width:1px ``` @@ -1090,9 +1090,9 @@ graph LR C[Humans] -..->|verify| D[  Tests  ] style A fill:#e8e8e8,stroke:#333,stroke-width:1px - style B fill:#dcf5a4,stroke:#333,stroke-width:1px + style B fill:#e0f7d7,stroke:#333,stroke-width:1px style C fill:#e8e8e8,stroke:#333,stroke-width:1px - style D fill:#dcf5a4,stroke:#333,stroke-width:1px + style D fill:#e0f7d7,stroke:#333,stroke-width:1px ``` Tests should focus on verifying the behavior of the implementation code, while humans should be able to easily understand and verify the correctness of the test code itself. @@ -1375,8 +1375,8 @@ Flow Diagram graph TD F[printReceipt] --> G[print - Receipt: Withdrew $400] - style F fill:#cfc,stroke:#333,stroke-width:1px - style G fill:#cfc,stroke:#333,stroke-width:1px + style F fill:#e0f7d7,stroke:#333,stroke-width:1px + style G fill:#e0f7d7,stroke:#333,stroke-width:1px ``` **2. Traceback to the calling point:** @@ -1423,10 +1423,10 @@ graph TD E --> F[printReceipt] F --> G[print - Receipt: Withdrew $400] - style D fill:#cfc,stroke:#333,stroke-width:1px - style E fill:#cfc,stroke:#333,stroke-width:1px - style F fill:#cfc,stroke:#333,stroke-width:1px - style G fill:#cfc,stroke:#333,stroke-width:1px + style D fill:#e0f7d7,stroke:#333,stroke-width:1px + style E fill:#e0f7d7,stroke:#333,stroke-width:1px + style F fill:#e0f7d7,stroke:#333,stroke-width:1px + style G fill:#e0f7d7,stroke:#333,stroke-width:1px ``` 2.2 Determine the Origin of the Conditional Value @@ -1458,10 +1458,10 @@ graph TD F --> G[print - Receipt: Withdrew $400] style B fill:#FFB6C1,stroke:#333,stroke-width:1px, color:#00 - style D fill:#cfc,stroke:#333,stroke-width:1px - style E fill:#cfc,stroke:#333,stroke-width:1px - style F fill:#cfc,stroke:#333,stroke-width:1px - style G fill:#cfc,stroke:#333,stroke-width:1px + style D fill:#e0f7d7,stroke:#333,stroke-width:1px + style E fill:#e0f7d7,stroke:#333,stroke-width:1px + style F fill:#e0f7d7,stroke:#333,stroke-width:1px + style G fill:#e0f7d7,stroke:#333,stroke-width:1px ``` It can be seen that the **needReceipt** value is passed as a parameter while having a **default value** of **false**. Since the value was **never set to true** in our test case, @@ -1506,11 +1506,11 @@ graph TD style A fill:#f5f5f5,stroke:#333,stroke-width:1px, color:#000 style B fill:#ffb6c1,stroke:#333,stroke-width:1px, color:#00 - style C fill:#cfc,stroke:#333,stroke-width:1px - style D fill:#cfc,stroke:#333,stroke-width:1px - style E fill:#cfc,stroke:#333,stroke-width:1px - style F fill:#cfc,stroke:#333,stroke-width:1px - style G fill:#cfc,stroke:#333,stroke-width:1px + style C fill:#e0f7d7,stroke:#333,stroke-width:1px + style D fill:#e0f7d7,stroke:#333,stroke-width:1px + style E fill:#e0f7d7,stroke:#333,stroke-width:1px + style F fill:#e0f7d7,stroke:#333,stroke-width:1px + style G fill:#e0f7d7,stroke:#333,stroke-width:1px ``` **3. Add Test Case to Cover the Line:** From d67e4d0e36719727b80af5024875a4f4ff55f13b Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 04:07:41 +0530 Subject: [PATCH 52/64] Added code coverage analysis to the review process --- ...ng-tests-with-good-behavioural-coverage.md | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/wiki/Writing-tests-with-good-behavioural-coverage.md b/wiki/Writing-tests-with-good-behavioural-coverage.md index 33c3b69f8a6..4c39c0e02d4 100644 --- a/wiki/Writing-tests-with-good-behavioural-coverage.md +++ b/wiki/Writing-tests-with-good-behavioural-coverage.md @@ -983,12 +983,32 @@ fun testWithdraw_withDefaultReceipt_noReceiptPrinted() { Goal: Ensure all scenarios are covered and tests are effective. -1. Cross-Reference: Verify that all behaviors from the mapping are tested. -2. Add Additional Cases: Incorporate any edge cases or additional scenarios discovered during review. -3. Refactor Tests: Ensure tests are clear, maintainable, and provide meaningful results. +Once you have confirmed that all mappings are tested, proceed with the following steps. + +```mermaid +graph LR + A[Code Coverage] --> B[Map Lines] + B --> C[Add Tests] + C --> A + + style A fill:#e6e6e6,stroke:#333,stroke-width:1px + style B fill:#ecfae3,stroke:#333,stroke-width:1px + style C fill:#d8ffbf,stroke:#333,stroke-width:1px +``` + +1. **Perform Code Coverage Analysis:** Evaluate code coverage to identify any uncovered lines. + +2. **Trace and map uncovered lines:** Investigate to identify lines not covered by existing tests. + +3. **Add Additional Cases:** Add behavioral tests for uncovered lines and include other missing tests even for covered lines. + + +Ensure tests are clear, maintainable, and provide meaningful results. Continue this process iteratively until all lines and behaviors are fully covered By systematically analyzing the public API, mapping expected behaviors to test cases, and writing tests based on these mappings, you can ensure comprehensive and effective testing. This structured approach helps in covering all scenarios and provides clarity on how to test the API thoroughly. +Note: For more information on how to utilize the code coverage analysis tool, please refer to the [Oppia Android Code Coverage](https://github.com/oppia/oppia-android/wiki/Oppia-Android-Code-Coverage) page. + ## 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. From 8f254846f6601e857ee50f9285e6adee5155ae9b Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 04:21:15 +0530 Subject: [PATCH 53/64] Standard usage of behavior --- wiki/Oppia-Android-Code-Coverage.md | 2 +- ...iting-tests-with-good-behavioral-coverage.md} | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) rename wiki/{Writing-tests-with-good-behavioural-coverage.md => Writing-tests-with-good-behavioral-coverage.md} (99%) diff --git a/wiki/Oppia-Android-Code-Coverage.md b/wiki/Oppia-Android-Code-Coverage.md index b230df5e784..cb92d5e5d36 100644 --- a/wiki/Oppia-Android-Code-Coverage.md +++ b/wiki/Oppia-Android-Code-Coverage.md @@ -374,7 +374,7 @@ These generated html reports are be used to identify areas of your code that may To improve code coverage, start by identifying which lines are covered and which are not. To effectively increase coverage, trace the uncovered lines back to their calling functions, and ensure these are executed and tested in your test suites. The corresponding test files for the source code are usually located in the same package within the test directories. Add tests that cover all relevant behavioral scenarios for the uncovered lines to achieve comprehensive testing. -For more guidance on best practices, refer to the [Writing Tests with Good Behavioural Coverage](https://github.com/oppia/oppia-android/wiki/Writing-Tests-With-Good-Behavioural-Coverage) wiki page. +For more guidance on best practices, refer to the [Writing Tests with Good Behavioral Coverage](https://github.com/oppia/oppia-android/wiki/Writing-Tests-With-Good-Behavioral-Coverage) wiki page. Note: Some files in the app module may have multiple test files, located in the sharedTest and test packages, all testing a single source file. For example: [StateFragment.kt](https://github.com/oppia/oppia-android/blob/develop/app/src/main/java/org/oppia/android/app/player/state/StateFragment.kt) has 2 test files [StateFragmentTest.kt](https://github.com/oppia/oppia-android/blob/develop/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt) under ``app/src/sharedTest`` and [StateFragmentLocalTest.kt](https://github.com/oppia/oppia-android/blob/develop/app/src/test/java/org/oppia/android/app/player/state/StateFragmentLocalTest.kt) under ``app/src/test``. diff --git a/wiki/Writing-tests-with-good-behavioural-coverage.md b/wiki/Writing-tests-with-good-behavioral-coverage.md similarity index 99% rename from wiki/Writing-tests-with-good-behavioural-coverage.md rename to wiki/Writing-tests-with-good-behavioral-coverage.md index 4c39c0e02d4..a1fc1ba6b2f 100644 --- a/wiki/Writing-tests-with-good-behavioural-coverage.md +++ b/wiki/Writing-tests-with-good-behavioral-coverage.md @@ -50,7 +50,7 @@ A basic test case for line coverage might look like this: **Line Coverage Result:** This test covers the line of code but doesn’t test all scenarios. -### Behavioural Coverage Testing +### Behavioral Coverage Testing To ensure behavioral coverage, the test needs to verify various conditions: @@ -81,7 +81,7 @@ To ensure behavioral coverage, the test needs to verify various conditions: } ``` -**Behavioural Coverage Result:** These tests ensure that all potential behaviors, including edge cases and exceptions, are covered, providing a more thorough validation. +**Behavioral Coverage Result:** These tests ensure that all potential behaviors, including edge cases and exceptions, are covered, providing a more thorough validation. ### Quality > Percentage @@ -94,16 +94,16 @@ block-beta columns 2 A[["Coverage Analysis"]]:2 block:Coverage1 - B["100% Line Coverage
Does not guarantee full
behavioural coverage"] + B["100% Line Coverage
Does not guarantee full
behavioral coverage"] end block:Coverage2 - C["Less Than 100% Coverage
Guarantees at least one
important behaviour is missing"] + C["Less Than 100% Coverage
Guarantees at least one
important behavior is missing"] end block:Action1 - D["Add tests for lines that
miss behavioural coverages
even if stated as covered"] + D["Add tests for lines that
miss behavioral coverages
even if stated as covered"] end block:Action2 - E["Find uncovered lines and add
tests that cover all behaviours
and not just line coverages"] + E["Find uncovered lines and add
tests that cover all behaviors
and not just line coverages"] end space Coverage1 --> D @@ -118,7 +118,7 @@ columns 2 ``` -**Coverage and Behaviour:** +**Coverage and Behavior:** - 100% Line Coverage: Does not guarantee that all critical behaviors and edge cases are tested. It merely shows that each line of code has been executed. - Less Than 100% Coverage: Guarantees that some part of the code is not covered by tests, which likely means that at least one important behavior or edge case is missing from the tests. @@ -1357,7 +1357,7 @@ fun testWithdraw_withValidCredentials_printsWithdrawMessage() { } ``` -This validates the code behaviour to function properly with valid inputs. +This validates the code behavior to function properly with valid inputs. ### 1. Identify the Line of Code From dab4faadf531f05d41de7186a4c6e70506aa61a7 Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 05:28:04 +0530 Subject: [PATCH 54/64] Updted test name with multiple conditions, specific use of words --- wiki/Writing-tests-with-good-behavioral-coverage.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/wiki/Writing-tests-with-good-behavioral-coverage.md b/wiki/Writing-tests-with-good-behavioral-coverage.md index a1fc1ba6b2f..ffe409133fd 100644 --- a/wiki/Writing-tests-with-good-behavioral-coverage.md +++ b/wiki/Writing-tests-with-good-behavioral-coverage.md @@ -468,7 +468,7 @@ This test verifies that the divideNumbers function throws an IllegalArgumentExce ### Domain Errors -Domain errors are errors related to the business logic or rules of the application, rather than technical issues like those covered by exceptions. These errors occur when the data or conditions within the domain do not meet the specific rules or constraints defined by the business. Domain errors are expected conditions and are handled within the normal flow of the application, rather than through exceptions. +Domain errors are errors related to the logic or rules of the application, rather than technical issues like those covered by exceptions. These errors occur when the data or conditions within the domain do not meet the specific rules or constraints defined during application design. Domain errors are expected conditions and are handled within the normal flow of the application, rather than through exceptions. Let's understand this with a sample snippet from the source **[FractionParser.kt](https://github.com/oppia/oppia-android/blob/develop/utility/src/main/java/org/oppia/android/util/math/FractionParser.kt)** @@ -1285,7 +1285,7 @@ Unlike production code, where duplication is often avoided, in test code, it’s Naming test functions descriptively helps in identifying the purpose and scope of each test. Use names that reflect the specific behavior being tested. -Oppia Android follows a naming convention where the test names should read like a sentence, and be consistent with other nearby test names to facilitate easily coming up with new tests. Consider using a format similar to the following for naming test functions: +Oppia Android follows a naming convention where the test names should read like a sentence, and be consistent with other nearby test names to facilitate easily coming up with new tests. It's preferred that the following format be used for naming test functions: ``` testAction_withOneCondition_withSecondCondition_hasExpectedOutcome @@ -1318,7 +1318,7 @@ fun testApplyDiscount_withNonMemberAndValidCode_noDiscountApplied() { } @Test -fun testApplyDiscount_withMemberAndQuarter2_discountApplied() { +fun testApplyDiscount_withMemberAndValidCode_inQuarter2_discountApplied() { // Test Logic: Apply discount for valid code in Quarter 2 } ``` From 38c0a5c509a9349e7abe461f6ba6e43ff2cc7ab5 Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 05:35:03 +0530 Subject: [PATCH 55/64] Moved @Test annotation and function to its own line --- wiki/Oppia-Android-Code-Coverage.md | 26 +++++++++---------- ...ing-tests-with-good-behavioral-coverage.md | 18 ++++++++----- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/wiki/Oppia-Android-Code-Coverage.md b/wiki/Oppia-Android-Code-Coverage.md index cb92d5e5d36..e5ed0479619 100644 --- a/wiki/Oppia-Android-Code-Coverage.md +++ b/wiki/Oppia-Android-Code-Coverage.md @@ -40,10 +40,10 @@ import org.junit.Test import com.google.common.truth.Truth.assertThat class checkSignTest { - @Test - fun testCheckSign_withPositiveInteger_returnsPositive() { - assertThat(checkSign(4)).isEqualTo("Positive") - } + @Test + fun testCheckSign_withPositiveInteger_returnsPositive() { + assertThat(checkSign(4)).isEqualTo("Positive") + } } ``` @@ -57,15 +57,15 @@ import org.junit.Test import com.google.common.truth.Truth.assertThat class checkSignTest { - @Test - fun testCheckSign_withPositiveInteger_returnsPositive() { - assertThat(checkSign(4)).isEqualTo("Positive") - } - -+ @Test -+ fun testCheckSign_withNegativeInteger_returnsNegative() { -+ assertThat(checkSign(-7)).isEqualTo("Negative") -+ } + @Test + fun testCheckSign_withPositiveInteger_returnsPositive() { + assertThat(checkSign(4)).isEqualTo("Positive") + } + ++ @Test ++ fun testCheckSign_withNegativeInteger_returnsNegative() { ++ assertThat(checkSign(-7)).isEqualTo("Negative") ++ } } ``` diff --git a/wiki/Writing-tests-with-good-behavioral-coverage.md b/wiki/Writing-tests-with-good-behavioral-coverage.md index ffe409133fd..940563be586 100644 --- a/wiki/Writing-tests-with-good-behavioral-coverage.md +++ b/wiki/Writing-tests-with-good-behavioral-coverage.md @@ -42,7 +42,8 @@ fun getName(name: String? = " ") = A basic test case for line coverage might look like this: ```kotlin -@Test fun testValidName() { +@Test +fun testValidName() { // Tests line coverage by hitting the line where name is accessed assertThat(getName("Alice")).isEqualTo("Alice") } @@ -55,27 +56,32 @@ A basic test case for line coverage might look like this: To ensure behavioral coverage, the test needs to verify various conditions: ```kotlin -@Test fun testGetName_withDefaultValue_result(getName()) { +@Test +fun testGetName_withDefaultValue_result(getName()) { // Default value when no name is provided assertThat(getName()).isEqualTo(" ") } -@Test fun testGetName_withNullName_throwsException() { +@Test +fun testGetName_withNullName_throwsException() { // Exception for null value assertThrows { getName(null) } } -@Test fun testGetName_withSpecialCharacters_throwsException() { +@Test +fun testGetName_withSpecialCharacters_throwsException() { // Exception for special characters assertThrows { getName("!@#$%^&*()") } } -@Test fun testGetName_withEmptyName_result(getName("")) { +@Test +fun testGetName_withEmptyName_result(getName("")) { // Empty string should use default value assertThat(getName("")).isEqualTo(" ") } -@Test fun testGetName_withWhitespaceName_result(getName(" ")) { +@Test +fun testGetName_withWhitespaceName_result(getName(" ")) { // Whitespace name assertThat(getName(" ")).isEqualTo(" ") } From bcfc30fc1e16986be1bc54c0dca70351e81086dc Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 06:56:16 +0530 Subject: [PATCH 56/64] Updating the helper function example to use log file utility case --- ...ing-tests-with-good-behavioral-coverage.md | 55 ++++++++++++------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/wiki/Writing-tests-with-good-behavioral-coverage.md b/wiki/Writing-tests-with-good-behavioral-coverage.md index 940563be586..de2ff2fa252 100644 --- a/wiki/Writing-tests-with-good-behavioral-coverage.md +++ b/wiki/Writing-tests-with-good-behavioral-coverage.md @@ -1165,17 +1165,21 @@ It's important to understand how to segment or split functionalities in your tes Helper Functions are valuable for reducing redundancy in tests. They encapsulate **non-behavioral tasks**, ensuring that the focus remains on testing the core logic. +With the above Restaurant class example, it can be seen that each order is logged to a provided file. While testing these functionalities it is crucial to ensure that each test case operates with its own unique log file. This is necessary to avoid cross-test interference and to verify that each test correctly logs its own data. To streamline this process, instead of creating a new log file manually within each test case, we use a utility function to handle the file creation. + +This approach keeps the test code clean and focused on testing the core logic, as the utility function purely deals with file management. By keeping this utility function separate from the core logic, it ensures that it doesn't affect the actual functionality being tested, making it an ideal candidate for a helper function. + **Helper Function:** ```kotlin -// Helper function to initialize a Restaurant with a menu -fun createRestaurantWithMenu(): Restaurant { - val menu = mapOf( - "Burger" to 5.0, - "Pizza" to 8.0, - "Salad" to 4.0 - ) - return Restaurant(menu) +// Helper function to create a new log file and return its path +fun createLogFile(filePath: String): String { + val file = File(filePath) + if (file.exists()) { + file.delete() + } + file.createNewFile() + return filePath } ``` @@ -1183,26 +1187,29 @@ fun createRestaurantWithMenu(): Restaurant { ```kotlin @Test -fun testPlaceOrder_withValidItems_displaysCorrectTotalBill() { - val restaurant = createRestaurantWithMenu() - restaurant.placeOrder(listOf("Burger", "Pizza")) +fun testPlaceOrder_withValidItems_logsOrderDetails() { + val menu = mapOf("Burger" to 5.0, "Pizza" to 8.0, "Salad" to 4.0) + // Use the utility function to create a log file + val logFilePath = createLogFile("testLogHistory.txt") + val restaurant = Restaurant(menu, logOrders = true, logHistoryPath = logFilePath) - val outputStream = ByteArrayOutputStream() - System.setOut(PrintStream(outputStream)) + restaurant.placeOrder(listOf("Burger", "Pizza")) - assertThat(outputStream.toString().trim()).contains("Total bill: 13.0") + val logContent = File(logFilePath).readText() - System.setOut(System.out) + assertThat(logContent).contains("Items: [Burger, Pizza]") + assertThat(logContent).contains("Total bill: 13.0") } ``` +Using the `createLogFile` utility function helps maintain clean and focused test cases. It manages the file creation process without interfering with the actual logic being tested. + ### b. `@Before` and `@After` Annotations `@Before` and `@After` Annotations help in managing setup and teardown tasks, ensuring consistency and reducing redundancy in test cases. ```kotlin class RestaurantTests { - private lateinit var restaurant: Restaurant private lateinit var outputStream: ByteArrayOutputStream @Before @@ -1210,7 +1217,6 @@ class RestaurantTests { // Setup necessary resources outputStream = ByteArrayOutputStream() System.setOut(PrintStream(outputStream)) - restaurant = createRestaurantWithMenu() } @After @@ -1221,6 +1227,10 @@ class RestaurantTests { @Test fun testPlaceOrder_withValidItems_displaysCorrectTotalBill() { + val menu = mapOf("Burger" to 5.0, "Pizza" to 8.0, "Salad" to 4.0) + val logFilePath = createLogFile("LogHistory.txt") + val restaurant = Restaurant(menu, logOrders = true, logHistoryPath = logFilePath) + restaurant.placeOrder(listOf("Burger", "Pizza")) assertThat(outputStream.toString().trim()).contains("Total bill: 13.0") } @@ -1247,7 +1257,10 @@ fun createDiscount(): Triple { @Test fun testDiscountedBill_withCreateDiscountHelper_returnsDiscountedBill() { - val restaurant = createRestaurantWithMenu() + val menu = mapOf("Burger" to 5.0, "Pizza" to 8.0, "Salad" to 4.0) + val logFilePath = createLogFile("LogHistory.txt") + val restaurant = Restaurant(menu, logOrders = true, logHistoryPath = logFilePath) + val discountDetails = createDiscount() restaurant.applyDiscount(discountDetails.first, discountDetails.second, discountDetails.third) @@ -1269,8 +1282,10 @@ In test code, being explicit often trumps being concise. This means defining the ```kotlin @Test fun testDiscountedBill_withAppliedDicount_returnsDiscountedBill() { - val restaurant = createRestaurantWithMenu() - + val menu = mapOf("Burger" to 5.0, "Pizza" to 8.0, "Salad" to 4.0) + val logFilePath = createLogFile("LogHistory.txt") + val restaurant = Restaurant(menu, logOrders = true, logHistoryPath = logFilePath) + // Explicitly defining discount details in the test val isMember = true val code = "SAVE10" From 3e8a57c2f72584700cc1122c98ff420f2ee9652d Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 07:16:23 +0530 Subject: [PATCH 57/64] Added source file changes to helper function changes --- ...ing-tests-with-good-behavioral-coverage.md | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/wiki/Writing-tests-with-good-behavioral-coverage.md b/wiki/Writing-tests-with-good-behavioral-coverage.md index de2ff2fa252..395bb36ead5 100644 --- a/wiki/Writing-tests-with-good-behavioral-coverage.md +++ b/wiki/Writing-tests-with-good-behavioral-coverage.md @@ -1126,7 +1126,10 @@ Tests should focus on verifying the behavior of the implementation code, while h Let's use a Restaurant class as an example to explain how to structure test functionalities effectively. The Restaurant class manages orders, calculates bills, and applies discounts. ```kotlin -class Restaurant(private val menu: Map) { +class Restaurant( + private val menu: Map, + private val logHistoryPath: String = "LogHistory.txt" +) { var items: List = listOf() private var discount: Double = 0.0 @@ -1135,6 +1138,10 @@ class Restaurant(private val menu: Map) { println("Order placed: $items") val totalBill = calculateBill() println("Total bill after discount: ${totalBill - (totalBill * discount)}") + + val logDetails = "Items: $items, Total bill: ${totalBill - (totalBill * discount)}" + val file = loadLogHistory() + file.appendText(logDetails + "\n") } fun applyDiscount(isMember: Boolean, code: String, quarter: YearQuarter) { @@ -1150,6 +1157,14 @@ class Restaurant(private val menu: Map) { private fun calculateBill(): Double { return items.sumOf { menu[it] ?: 0.0 } } + + private fun loadLogHistory(): File { + val file = File(logHistoryPath) + if (!file.exists()) { + file.createNewFile() + } + return file + } } enum class YearQuarter { @@ -1191,7 +1206,7 @@ fun testPlaceOrder_withValidItems_logsOrderDetails() { val menu = mapOf("Burger" to 5.0, "Pizza" to 8.0, "Salad" to 4.0) // Use the utility function to create a log file val logFilePath = createLogFile("testLogHistory.txt") - val restaurant = Restaurant(menu, logOrders = true, logHistoryPath = logFilePath) + val restaurant = Restaurant(menu, logHistoryPath = logFilePath) restaurant.placeOrder(listOf("Burger", "Pizza")) @@ -1229,7 +1244,7 @@ class RestaurantTests { fun testPlaceOrder_withValidItems_displaysCorrectTotalBill() { val menu = mapOf("Burger" to 5.0, "Pizza" to 8.0, "Salad" to 4.0) val logFilePath = createLogFile("LogHistory.txt") - val restaurant = Restaurant(menu, logOrders = true, logHistoryPath = logFilePath) + val restaurant = Restaurant(menu, logHistoryPath = logFilePath) restaurant.placeOrder(listOf("Burger", "Pizza")) assertThat(outputStream.toString().trim()).contains("Total bill: 13.0") @@ -1259,7 +1274,7 @@ fun createDiscount(): Triple { fun testDiscountedBill_withCreateDiscountHelper_returnsDiscountedBill() { val menu = mapOf("Burger" to 5.0, "Pizza" to 8.0, "Salad" to 4.0) val logFilePath = createLogFile("LogHistory.txt") - val restaurant = Restaurant(menu, logOrders = true, logHistoryPath = logFilePath) + val restaurant = Restaurant(menu logHistoryPath = logFilePath) val discountDetails = createDiscount() @@ -1284,7 +1299,7 @@ In test code, being explicit often trumps being concise. This means defining the fun testDiscountedBill_withAppliedDicount_returnsDiscountedBill() { val menu = mapOf("Burger" to 5.0, "Pizza" to 8.0, "Salad" to 4.0) val logFilePath = createLogFile("LogHistory.txt") - val restaurant = Restaurant(menu, logOrders = true, logHistoryPath = logFilePath) + val restaurant = Restaurant(menu, logHistoryPath = logFilePath) // Explicitly defining discount details in the test val isMember = true From e4eb862666b4767278665e9a175a6fc7dca2a585 Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 08:03:16 +0530 Subject: [PATCH 58/64] Setting the text color for the graphs to black as they weren't visible with dark mode --- ...ing-tests-with-good-behavioral-coverage.md | 68 +++++++++---------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/wiki/Writing-tests-with-good-behavioral-coverage.md b/wiki/Writing-tests-with-good-behavioral-coverage.md index 395bb36ead5..b8c11d3e208 100644 --- a/wiki/Writing-tests-with-good-behavioral-coverage.md +++ b/wiki/Writing-tests-with-good-behavioral-coverage.md @@ -116,11 +116,11 @@ columns 2 Coverage2 --> E E --> D - style A fill:#f3f3f3,stroke:#333,stroke-width:1px - style B fill:#e8fcde,stroke:#333,stroke-width:1px - style C fill:#ffdad4,stroke:#333,stroke-width:1px - style D fill:#d8fac5,stroke:#333,stroke-width:1px - style E fill:#e2fcd4,stroke:#333,stroke-width:1px + style A fill:#f3f3f3,stroke:#333,stroke-width:1px,color:#000000 + style B fill:#e8fcde,stroke:#333,stroke-width:1px,color:#000000 + style C fill:#ffdad4,stroke:#333,stroke-width:1px,color:#000000 + style D fill:#d8fac5,stroke:#333,stroke-width:1px,color:#000000 + style E fill:#e2fcd4,stroke:#333,stroke-width:1px,color:#000000 ``` @@ -855,10 +855,10 @@ graph LR B --> C[Add Tests] C --> D[Refine] - style A fill:#f5f5f5,stroke:#333,stroke-width:1px - style B fill:#f2fced,stroke:#333,stroke-width:1px - style C fill:#e8fcde,stroke:#333,stroke-width:1px - style D fill:#dcf5d0,stroke:#333,stroke-width:1px + style A fill:#f5f5f5,stroke:#333,stroke-width:1px,color:#000000 + style B fill:#f2fced,stroke:#333,stroke-width:1px,color:#000000 + style C fill:#e8fcde,stroke:#333,stroke-width:1px,color:#000000 + style D fill:#dcf5d0,stroke:#333,stroke-width:1px,color:#000000 ``` @@ -997,9 +997,9 @@ graph LR B --> C[Add Tests] C --> A - style A fill:#e6e6e6,stroke:#333,stroke-width:1px - style B fill:#ecfae3,stroke:#333,stroke-width:1px - style C fill:#d8ffbf,stroke:#333,stroke-width:1px + style A fill:#e6e6e6,stroke:#333,stroke-width:1px,color:#000000 + style B fill:#ecfae3,stroke:#333,stroke-width:1px,color:#000000 + style C fill:#d8ffbf,stroke:#333,stroke-width:1px,color:#000000 ``` 1. **Perform Code Coverage Analysis:** Evaluate code coverage to identify any uncovered lines. @@ -1115,10 +1115,10 @@ graph LR A[  Tests ] -..->|verify| B[Source Code] C[Humans] -..->|verify| D[  Tests  ] - style A fill:#e8e8e8,stroke:#333,stroke-width:1px - style B fill:#e0f7d7,stroke:#333,stroke-width:1px - style C fill:#e8e8e8,stroke:#333,stroke-width:1px - style D fill:#e0f7d7,stroke:#333,stroke-width:1px + style A fill:#e8e8e8,stroke:#333,stroke-width:1px,color:#000000 + style B fill:#e0f7d7,stroke:#333,stroke-width:1px,color:#000000 + style C fill:#e8e8e8,stroke:#333,stroke-width:1px,color:#000000 + style D fill:#e0f7d7,stroke:#333,stroke-width:1px,color:#000000 ``` Tests should focus on verifying the behavior of the implementation code, while humans should be able to easily understand and verify the correctness of the test code itself. @@ -1431,8 +1431,8 @@ Flow Diagram graph TD F[printReceipt] --> G[print - Receipt: Withdrew $400] - style F fill:#e0f7d7,stroke:#333,stroke-width:1px - style G fill:#e0f7d7,stroke:#333,stroke-width:1px + style F fill:#e0f7d7,stroke:#333,stroke-width:1px,color:#000000 + style G fill:#e0f7d7,stroke:#333,stroke-width:1px,color:#000000 ``` **2. Traceback to the calling point:** @@ -1479,10 +1479,10 @@ graph TD E --> F[printReceipt] F --> G[print - Receipt: Withdrew $400] - style D fill:#e0f7d7,stroke:#333,stroke-width:1px - style E fill:#e0f7d7,stroke:#333,stroke-width:1px - style F fill:#e0f7d7,stroke:#333,stroke-width:1px - style G fill:#e0f7d7,stroke:#333,stroke-width:1px + style D fill:#e0f7d7,stroke:#333,stroke-width:1px,color:#000000 + style E fill:#e0f7d7,stroke:#333,stroke-width:1px,color:#000000 + style F fill:#e0f7d7,stroke:#333,stroke-width:1px,color:#000000 + style G fill:#e0f7d7,stroke:#333,stroke-width:1px,color:#000000 ``` 2.2 Determine the Origin of the Conditional Value @@ -1513,11 +1513,11 @@ graph TD E --> F[printReceipt] F --> G[print - Receipt: Withdrew $400] - style B fill:#FFB6C1,stroke:#333,stroke-width:1px, color:#00 - style D fill:#e0f7d7,stroke:#333,stroke-width:1px - style E fill:#e0f7d7,stroke:#333,stroke-width:1px - style F fill:#e0f7d7,stroke:#333,stroke-width:1px - style G fill:#e0f7d7,stroke:#333,stroke-width:1px + style B fill:#FFB6C1,stroke:#333,stroke-width:1px,color:#000000 + style D fill:#e0f7d7,stroke:#333,stroke-width:1px,color:#000000 + style E fill:#e0f7d7,stroke:#333,stroke-width:1px,color:#000000 + style F fill:#e0f7d7,stroke:#333,stroke-width:1px,color:#000000 + style G fill:#e0f7d7,stroke:#333,stroke-width:1px,color:#000000 ``` It can be seen that the **needReceipt** value is passed as a parameter while having a **default value** of **false**. Since the value was **never set to true** in our test case, @@ -1560,13 +1560,13 @@ graph TD E --> F[printReceipt] F --> G[print - Receipt: Withdrew $400] - style A fill:#f5f5f5,stroke:#333,stroke-width:1px, color:#000 - style B fill:#ffb6c1,stroke:#333,stroke-width:1px, color:#00 - style C fill:#e0f7d7,stroke:#333,stroke-width:1px - style D fill:#e0f7d7,stroke:#333,stroke-width:1px - style E fill:#e0f7d7,stroke:#333,stroke-width:1px - style F fill:#e0f7d7,stroke:#333,stroke-width:1px - style G fill:#e0f7d7,stroke:#333,stroke-width:1px + style A fill:#f5f5f5,stroke:#333,stroke-width:1px,color:#000000 + style B fill:#ffb6c1,stroke:#333,stroke-width:1px,color:#000000 + style C fill:#e0f7d7,stroke:#333,stroke-width:1px,color:#000000 + style D fill:#e0f7d7,stroke:#333,stroke-width:1px,color:#000000 + style E fill:#e0f7d7,stroke:#333,stroke-width:1px,color:#000000 + style F fill:#e0f7d7,stroke:#333,stroke-width:1px,color:#000000 + style G fill:#e0f7d7,stroke:#333,stroke-width:1px,color:#000000 ``` **3. Add Test Case to Cover the Line:** From c9a6df7958b697b28bd5d8aa062576768275a026 Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 08:04:46 +0530 Subject: [PATCH 59/64] Updated the side bar info too to align with contents --- wiki/_Sidebar.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wiki/_Sidebar.md b/wiki/_Sidebar.md index ac9e870df63..ea5a254f2c6 100644 --- a/wiki/_Sidebar.md +++ b/wiki/_Sidebar.md @@ -26,7 +26,7 @@ * [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) - * [Writing tests with good behavioural coverage](https://github.com/oppia/oppia-android/wiki/Writing-Tests-With-Good-Behavioural-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) * [RTL Guidelines](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines) From 13d566ed962f9d300991eb4c8b71916a7507a722 Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 09:38:45 +0530 Subject: [PATCH 60/64] Added missed comma on parameter declaration --- wiki/Writing-tests-with-good-behavioral-coverage.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wiki/Writing-tests-with-good-behavioral-coverage.md b/wiki/Writing-tests-with-good-behavioral-coverage.md index b8c11d3e208..09e6bc3ffc9 100644 --- a/wiki/Writing-tests-with-good-behavioral-coverage.md +++ b/wiki/Writing-tests-with-good-behavioral-coverage.md @@ -1274,7 +1274,7 @@ fun createDiscount(): Triple { fun testDiscountedBill_withCreateDiscountHelper_returnsDiscountedBill() { val menu = mapOf("Burger" to 5.0, "Pizza" to 8.0, "Salad" to 4.0) val logFilePath = createLogFile("LogHistory.txt") - val restaurant = Restaurant(menu logHistoryPath = logFilePath) + val restaurant = Restaurant(menu, logHistoryPath = logFilePath) val discountDetails = createDiscount() From a56c40a8c04e9b0cceab19c70899959132ffe6b4 Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 09:41:38 +0530 Subject: [PATCH 61/64] Added missed permalink for FractionParser --- wiki/Writing-tests-with-good-behavioral-coverage.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wiki/Writing-tests-with-good-behavioral-coverage.md b/wiki/Writing-tests-with-good-behavioral-coverage.md index 09e6bc3ffc9..de75fd0df55 100644 --- a/wiki/Writing-tests-with-good-behavioral-coverage.md +++ b/wiki/Writing-tests-with-good-behavioral-coverage.md @@ -476,7 +476,7 @@ This test verifies that the divideNumbers function throws an IllegalArgumentExce Domain errors are errors related to the logic or rules of the application, rather than technical issues like those covered by exceptions. These errors occur when the data or conditions within the domain do not meet the specific rules or constraints defined during application design. Domain errors are expected conditions and are handled within the normal flow of the application, rather than through exceptions. -Let's understand this with a sample snippet from the source **[FractionParser.kt](https://github.com/oppia/oppia-android/blob/develop/utility/src/main/java/org/oppia/android/util/math/FractionParser.kt)** +Let's understand this with a sample snippet from the source **[FractionParser.kt](https://github.com/oppia/oppia-android/blob/f9106d91297abd09b24da25d5485deb8473b0125/utility/src/main/java/org/oppia/android/util/math/FractionParser.kt#L7)** ```kotlin fun getSubmitTimeError(text: String): FractionParsingError { From 6c29802e9a696c6ea185814369f4cd9ed5012253 Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 21 Aug 2024 19:23:55 +0530 Subject: [PATCH 62/64] Copy pasted the exact code for code coverage and coverage report from the clone develop --- .github/workflows/coverage_report.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/coverage_report.yml b/.github/workflows/coverage_report.yml index ff01185a7a2..3fce10ff97e 100644 --- a/.github/workflows/coverage_report.yml +++ b/.github/workflows/coverage_report.yml @@ -37,7 +37,7 @@ jobs: steps: - uses: actions/checkout@v2 - - name: Find CI workflow ru for PR + - name: Find CI workflow run for PR id: find-workflow-run uses: actions/github-script@v7 continue-on-error: true From 9bfd4c26bead40022bca78ca1272f77b7f4adb1b Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 22 Aug 2024 04:51:57 +0530 Subject: [PATCH 63/64] Fix remaining nits with punctuations, plural forms and empty check --- ...riting-tests-with-good-behavioral-coverage.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/wiki/Writing-tests-with-good-behavioral-coverage.md b/wiki/Writing-tests-with-good-behavioral-coverage.md index de75fd0df55..7a0aa5caebc 100644 --- a/wiki/Writing-tests-with-good-behavioral-coverage.md +++ b/wiki/Writing-tests-with-good-behavioral-coverage.md @@ -10,7 +10,7 @@ - [Covering All Branches, Paths, and Conditions](#5-covering-all-branches-paths-and-conditions) - [Exception and Error Handling](#6-exception-and-error-handling) - [Absence of Unwanted Output](#7-absence-of-unwanted-output) -- [Testing Public API](#testing-public-api) +- [Testing Public APIs](#testing-public-apis) - [Structuring Test Bodies](#structuring-test-bodies) - [When and How to Divide Responsibilities](#1-when-and-how-to-divide-responsibilities) - [When Not to Divide Responsibilities](#2-when-not-to-divide-responsibilities) @@ -57,9 +57,9 @@ To ensure behavioral coverage, the test needs to verify various conditions: ```kotlin @Test -fun testGetName_withDefaultValue_result(getName()) { +fun testGetName_withDefaultValue_returnsEmptyValue() { // Default value when no name is provided - assertThat(getName()).isEqualTo(" ") + assertThat(getName()).isEmpty() } @Test @@ -75,13 +75,13 @@ fun testGetName_withSpecialCharacters_throwsException() { } @Test -fun testGetName_withEmptyName_result(getName("")) { +fun testGetName_withEmptyName_returnsEmptyValue() { // Empty string should use default value - assertThat(getName("")).isEqualTo(" ") + assertThat(getName("")).isEmpty() } @Test -fun testGetName_withWhitespaceName_result(getName(" ")) { +fun testGetName_withWhitespaceName_returnsWhitespaceValue() { // Whitespace name assertThat(getName(" ")).isEqualTo(" ") } @@ -785,7 +785,7 @@ fun testOrderPizza_withNoTakeaway_doesNotPrintTakeawayMessage() { These tests confirm that the class behaves as expected without producing unnecessary outputs. -# Testing Public API +# Testing Public APIs A public API (Application Programming Interface) refers to the set of methods, properties, and functionalities exposed by a class or module for use by external code. It defines how other parts of a system or external systems can interact with the functionality provided by that class or module. @@ -1009,7 +1009,7 @@ graph LR 3. **Add Additional Cases:** Add behavioral tests for uncovered lines and include other missing tests even for covered lines. -Ensure tests are clear, maintainable, and provide meaningful results. Continue this process iteratively until all lines and behaviors are fully covered +Ensure tests are clear, maintainable, and provide meaningful results. Continue this process iteratively until all lines and behaviors are fully covered. By systematically analyzing the public API, mapping expected behaviors to test cases, and writing tests based on these mappings, you can ensure comprehensive and effective testing. This structured approach helps in covering all scenarios and provides clarity on how to test the API thoroughly. From 98587f304d0029daa7f7da5b6e598070956d8d4d Mon Sep 17 00:00:00 2001 From: Rd Date: Fri, 23 Aug 2024 22:07:46 +0530 Subject: [PATCH 64/64] Moved the evaluation and check status back to their original workflows and fixing concurrency cancellations with merge develop --- .github/workflows/build_tests.yml | 2 +- .github/workflows/code_coverage.yml | 72 ++++++++++++++- ...report.yml => comment_coverage_report.yml} | 92 +++---------------- .github/workflows/main.yml | 2 +- .github/workflows/static_checks.yml | 2 +- .github/workflows/unit_tests.yml | 2 +- 6 files changed, 89 insertions(+), 83 deletions(-) rename .github/workflows/{coverage_report.yml => comment_coverage_report.yml} (54%) diff --git a/.github/workflows/build_tests.yml b/.github/workflows/build_tests.yml index 1f293b1282b..7dde621a72f 100644 --- a/.github/workflows/build_tests.yml +++ b/.github/workflows/build_tests.yml @@ -11,7 +11,7 @@ on: - develop concurrency: - group: ${{ github.workflow }}-${{ github.ref }} + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true jobs: diff --git a/.github/workflows/code_coverage.yml b/.github/workflows/code_coverage.yml index 7bdc9e19041..992a7e86cb1 100644 --- a/.github/workflows/code_coverage.yml +++ b/.github/workflows/code_coverage.yml @@ -12,7 +12,7 @@ on: - develop concurrency: - group: ${{ github.workflow }}-${{ github.ref }} + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true jobs: @@ -254,3 +254,73 @@ jobs: with: name: coverage-report-${{ env.SHARD_NAME }} # Saving with unique names to avoid conflict path: coverage_reports + + evaluate_code_coverage_reports: + name: Evaluate Code Coverage Reports + runs-on: ubuntu-20.04 + needs: 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() }} + env: + CACHE_DIRECTORY: ~/.bazel_cache + steps: + - uses: actions/checkout@v2 + + - name: Download Coverage Report Artifacts + uses: actions/download-artifact@v4 + with: + path: coverage-report-artifact + pattern: coverage-report-* + merge-multiple: true + + - name: Filter Coverage Reports + run: | + # Find all coverage_report.pb files in the current directory and subdirectories + PB_FILES_LIST=($(find . -name "coverage_report.pb" -type f -print0 | xargs -0 -n 1 echo)) + echo "${PB_FILES_LIST[@]}" > pb_files.txt + + - name: Set up Bazel + uses: abhinavsingh/setup-bazel@v3 + with: + version: 6.5.0 + + - uses: actions/cache@v2 + id: scripts_cache + with: + path: ${{ env.CACHE_DIRECTORY }} + key: ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel-scripts-${{ github.sha }} + restore-keys: | + ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel-scripts- + ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel- + + - name: Set up build environment + uses: ./.github/actions/set-up-android-bazel-build-environment + + - name: Generate Markdown Coverage Report + run: | + bazel run //scripts:coverage_reporter -- $(pwd) pb_files.txt + + - name: Upload Generated Markdown Report + uses: actions/upload-artifact@v4 + if: ${{ !cancelled() }} # IMPORTANT: Upload reports regardless of success or failure status + with: + name: final-coverage-report + path: coverage_reports/CoverageReport.md + + # 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 ] + # 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 coverages passed + if: ${{ needs.compute_changed_files.outputs.can_skip_files != 'true' && needs.code_coverage_run.result != 'success' }} + run: exit 1 + + - name: Check that coverage status is passed + if: ${{ needs.compute_changed_files.outputs.can_skip_files != 'true' && needs.evaluate_code_coverage_reports.result != 'success' }} + run: exit 1 diff --git a/.github/workflows/coverage_report.yml b/.github/workflows/comment_coverage_report.yml similarity index 54% rename from .github/workflows/coverage_report.yml rename to .github/workflows/comment_coverage_report.yml index 3fce10ff97e..9ecb4e2e95c 100644 --- a/.github/workflows/coverage_report.yml +++ b/.github/workflows/comment_coverage_report.yml @@ -1,6 +1,6 @@ # Contains jobs corresponding to publishing coverage reports generated by code_coverage.yml. -name: Coverage Report +name: Comment Coverage Report # Controls when the action will run. Triggers the workflow on pull request events # (assigned, opened, synchronize, reopened) @@ -10,7 +10,7 @@ on: types: [assigned, opened, synchronize, reopened] concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number }}-${{ github.ref }} + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true jobs: @@ -28,15 +28,17 @@ jobs: success failure - evaluate-code-coverage-reports: - name: Evaluate Code Coverage Reports - runs-on: ubuntu-20.04 + comment_coverage_report: + name: Comment Code Coverage Report needs: check_code_coverage_completed - env: - CACHE_DIRECTORY: ~/.bazel_cache - steps: - - uses: actions/checkout@v2 + permissions: + pull-requests: write + # 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-latest + steps: - name: Find CI workflow run for PR id: find-workflow-run uses: actions/github-script@v7 @@ -64,82 +66,16 @@ jobs: core.setOutput('run-id', run.id); - - - name: Download Coverage Report Artifacts - uses: actions/download-artifact@v4 - with: - path: coverage-report-artifact - pattern: coverage-report-* - merge-multiple: true - github-token: ${{ secrets.GITHUB_TOKEN }} - run-id: ${{ steps.find-workflow-run.outputs.run-id }} - - - name: Filter Coverage Reports - id: filter-coverage-reports - run: | - # Find all coverage_report.pb files in the current directory and subdirectories - PB_FILES_LIST=($(find . -name "coverage_report.pb" -type f -print0 | xargs -0 -n 1 echo)) - echo "${PB_FILES_LIST[@]}" > pb_files.txt - - - name: Set up Bazel - uses: abhinavsingh/setup-bazel@v3 - with: - version: 6.5.0 - - - uses: actions/cache@v2 - id: scripts_cache - with: - path: ${{ env.CACHE_DIRECTORY }} - key: ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel-scripts-${{ github.sha }} - restore-keys: | - ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel-scripts- - ${{ runner.os }}-${{ env.CACHE_DIRECTORY }}-bazel- - - - name: Set up build environment - uses: ./.github/actions/set-up-android-bazel-build-environment - - - name: Generate Markdown Coverage Report - run: | - bazel run //scripts:coverage_reporter -- $(pwd) pb_files.txt - - - name: Upload Generated Markdown Report - uses: actions/upload-artifact@v4 - if: ${{ !cancelled() }} # IMPORTANT: Upload reports regardless of success or failure status - with: - name: final-coverage-report - path: coverage_reports/CoverageReport.md - - comment_coverage_report: - name: Comment Code Coverage Report - needs: evaluate-code-coverage-reports - permissions: - pull-requests: write - - # 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-latest - steps: - name: Download Generated Markdown Report uses: actions/download-artifact@v4 + if: ${{ !cancelled() }} # IMPORTANT: Upload reports regardless of success or failure status with: name: final-coverage-report + github-token: ${{ secrets.GITHUB_TOKEN }} + run-id: ${{ steps.find-workflow-run.outputs.run-id }} - name: Upload Coverage Report as PR Comment uses: peter-evans/create-or-update-comment@v4 with: issue-number: ${{ github.event.pull_request.number }} body-path: 'CoverageReport.md' - - # Reference: https://github.community/t/127354/7. - check_coverage_results: - name: Check Code Coverage Results - needs: [ evaluate-code-coverage-reports, comment_coverage_report ] - # 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 that coverage status is passed - if: ${{ needs.evaluate-code-coverage-reports.result != 'success' }} - run: exit 1 diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ed6cd752264..84cc12006a8 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -13,7 +13,7 @@ on: - develop concurrency: - group: ${{ github.workflow }}-${{ github.ref }} + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true # This workflow has the following jobs: diff --git a/.github/workflows/static_checks.yml b/.github/workflows/static_checks.yml index 1f9500c1dfc..15dd3e28e76 100644 --- a/.github/workflows/static_checks.yml +++ b/.github/workflows/static_checks.yml @@ -10,7 +10,7 @@ on: - develop concurrency: - group: ${{ github.workflow }}-${{ github.ref }} + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true jobs: diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index f59a5b9bee7..82833edfede 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -13,7 +13,7 @@ on: - develop concurrency: - group: ${{ github.workflow }}-${{ github.ref }} + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true jobs: