From 7955f7f320065ce741c23d481ba516ced7d4af09 Mon Sep 17 00:00:00 2001 From: MOHIT GUPTA <76530270+MohitGupta121@users.noreply.github.com> Date: Tue, 19 Nov 2024 20:32:05 +0530 Subject: [PATCH 1/2] Fix #5578 : Developer Videos Link are Broken (#5579) ## Explanation Fix #5578 : Developer Videos Link are Broken Update links to developer videos to point to the Oppia Channel. ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [ ] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). --- wiki/Accessibility-A11y-Guide.md | 8 ++++---- wiki/Interpreting-CI-Results.md | 2 +- wiki/Static-Analysis-Checks.md | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/wiki/Accessibility-A11y-Guide.md b/wiki/Accessibility-A11y-Guide.md index a9e01c6c59e..d80d73f6efd 100644 --- a/wiki/Accessibility-A11y-Guide.md +++ b/wiki/Accessibility-A11y-Guide.md @@ -32,7 +32,7 @@ There are various manual and automated tests to check if app is accessible by al **[Accessibility Scanner](https://support.google.com/accessibility/android/answer/6376570?hl=en)** : Using Accessibility Scanner we can take screenshots of each and every screen in the Oppia-app manually and the Accessibility Scanner app will give the output for the individual screenshot mentioning all the errors. -[Here](https://youtu.be/LF5AgGI5H3A) is a video tutorial on how to set up and use the Accessibility Scanner. +[Here](https://youtu.be/zCIlMlbFf7I?si=Xn1L5iCJ-TJ4DRVq) is a video tutorial on how to set up and use the Accessibility Scanner. **Screen Reader**: Screen readers like **Talkback** can be used to test the app manually. Talkback app is used by blind people to navigate to different items in the screen and get audio based output. This app will not give any error like Accessibility Scanner. @@ -80,7 +80,7 @@ TalkBack is the Google **screen reader** included on Android devices. TalkBack g 5. Read all the instructions written on the screen as using Talkback requires specific steps. 6. Turn on **Use Service** -> **Allow** -[Here](https://youtu.be/xpIM9xlowjs) is a video tutorial on "How to use Talkback and what does its output mean?". +[Here](https://youtu.be/JBRV1dauxyI?si=VzrxFJSpU9r3pdqq) is a video tutorial on "How to use Talkback and what does its output mean?". ### Useful Resources * [Android A11Y Overview](https://support.google.com/accessibility/android/answer/6006564) @@ -89,9 +89,9 @@ TalkBack is the Google **screen reader** included on Android devices. TalkBack g * [Display speech output as Text: Talkback](https://developer.android.com/guide/topics/ui/accessibility/testing#optional_talkback_developer_settings) ### Developer Videos -* [How to use Accessibility Scanner? - Tutorial](https://youtu.be/LF5AgGI5H3A) +* [How to use Accessibility Scanner? - Tutorial](https://youtu.be/zCIlMlbFf7I?si=Xn1L5iCJ-TJ4DRVq) * [Presentation Slides](https://docs.google.com/presentation/d/1PM_gs3TV2LVKFv6WuF9CUQHWbK7koepAxypzxeZTFzE/edit?usp=sharing) -* [How to use Talkback and what does its output mean? - Tutorial](https://youtu.be/xpIM9xlowjs) +* [How to use Talkback and what does its output mean? - Tutorial](https://youtu.be/JBRV1dauxyI?si=VzrxFJSpU9r3pdqq) * [Presentation Slides](https://docs.google.com/presentation/d/17SeKJLKT-rUNa_Yupe97bMFSsjTNzp83jX-lZPKEtnQ/edit?usp=sharing) ## Using AccessibilityTestRule in Espresso Tests diff --git a/wiki/Interpreting-CI-Results.md b/wiki/Interpreting-CI-Results.md index 00b3c5667c7..14293e02f55 100644 --- a/wiki/Interpreting-CI-Results.md +++ b/wiki/Interpreting-CI-Results.md @@ -21,4 +21,4 @@ Example in the below check the second job has some error or failure Navigate to logs or search the keyword ‘error’ to find the error message to understand what might have caused the failure in the checks. ### Developer Video - Understanding CI check failures -Learn how to interpret and troubleshoot oppia-android CI check failures in this insightful [developer video](https://youtu.be/I2bRf6fvgJ0?si=35sAagbUFSk6bOBA). \ No newline at end of file +Learn how to interpret and troubleshoot oppia-android CI check failures in this insightful [developer video](https://youtu.be/HLzHQULZbJE?si=RLY9_8Yzv5cjYM7q). \ No newline at end of file diff --git a/wiki/Static-Analysis-Checks.md b/wiki/Static-Analysis-Checks.md index 372a9b27bc1..6cfd8b2e022 100644 --- a/wiki/Static-Analysis-Checks.md +++ b/wiki/Static-Analysis-Checks.md @@ -267,4 +267,4 @@ To fix failing tests from GitHub CI individually, follow the steps below. Note: Before running the script command in your local terminal, make sure you have Bazel installed. To learn how to set up Bazel for Oppia Android, follow these [instructions](https://github.com/oppia/oppia-android/wiki/Oppia-Bazel-Setup-Instructions). Also make sure you have oppia-android-tools installed since static checks rely on these tools to be able to perform some of the checks. To install oppia-android-tools, run `bash scripts/setup.sh` in the oppia-android directory. ### Developer Video - Understanding CI check failures -Learn how to interpret and troubleshoot oppia-android CI check failures in this insightful [developer video](https://youtu.be/I2bRf6fvgJ0?si=35sAagbUFSk6bOBA). \ No newline at end of file +Learn how to interpret and troubleshoot oppia-android CI check failures in this insightful [developer video](https://youtu.be/HLzHQULZbJE?si=RLY9_8Yzv5cjYM7q). \ No newline at end of file From 2f68639f90e84fde029b92da48a9bc925cea7cc8 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Tue, 19 Nov 2024 23:30:04 +0530 Subject: [PATCH 2/2] Fix #5431 : Todo Checks Should Check Exclusively Against Issues (#5564) ## Explanation Fix #5431 This pull request fixes issue by implementing the following changes: Filters out pull requests from the "open issues" list. Ensures all "To-Do" checks will pass only for open issues (not for prs). ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only If your PR includes UI-related changes, then: - Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes - For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see [RTL guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines)) - Add a video showing the full UX flow with a screen reader enabled (see [accessibility guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide)) - For PRs introducing new UI elements or color changes, both light and dark mode screenshots must be included - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing --------- Co-authored-by: Sneha Datta Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com> Co-authored-by: Mr. 17 --- scripts/assets/todo_open_exemptions.textproto | 5 ++ .../android/scripts/common/GitHubClient.kt | 3 +- .../scripts/common/model/GitHubIssue.kt | 16 ++++- .../android/scripts/todo/TodoOpenCheckTest.kt | 62 ++++++++++++++++++- 4 files changed, 81 insertions(+), 5 deletions(-) diff --git a/scripts/assets/todo_open_exemptions.textproto b/scripts/assets/todo_open_exemptions.textproto index 8dd53a51ce9..035e618e357 100644 --- a/scripts/assets/todo_open_exemptions.textproto +++ b/scripts/assets/todo_open_exemptions.textproto @@ -333,6 +333,11 @@ todo_open_exemption { line_number: 689 line_number: 737 line_number: 741 + line_number: 790 + line_number: 791 + line_number: 792 + line_number: 798 + line_number: 800 } todo_open_exemption { exempted_file_path: "scripts/static_checks.sh" diff --git a/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt b/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt index fd7cc105037..23348c05136 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt @@ -68,13 +68,14 @@ class GitHubClient( val call = gitHubService.fetchOpenIssues(repoOwner, repoName, authorizationBearer, pageNumber) // Deferred blocking I/O operation to the dedicated I/O dispatcher. val response = withContext(Dispatchers.IO) { call.execute() } + check(response.isSuccessful()) { "Failed to fetch issues at page $pageNumber: ${response.code()}\n${call.request()}" + "\n${response.errorBody()}." } return@async checkNotNull(response.body()) { "No issues response from GitHub for page: $pageNumber." - } + }.filter { it.pullRequest == null } } } diff --git a/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt b/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt index 7a824fc3120..67b814440fa 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/model/GitHubIssue.kt @@ -10,4 +10,18 @@ import com.squareup.moshi.JsonClass * 'issues/' in an issue's GitHub URL) */ @JsonClass(generateAdapter = true) -data class GitHubIssue(@Json(name = "number") val number: Int) +data class GitHubIssue( + @Json(name = "number") val number: Int, + @Json(name = "pull_request") val pullRequest: PullRequestInfo? = null +) + +/** + * Data class representing information about a pull request associated with a GitHub issue. + * + * @property url the URL of the pull request, if it exists. This provides the link to the specific + * pull request associated with the issue on GitHub. + */ +@JsonClass(generateAdapter = true) +data class PullRequestInfo( + @Json(name = "url") val url: String? = null +) diff --git a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt index b614bc361d7..b5a3f783b70 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt @@ -777,11 +777,67 @@ class TodoOpenCheckTest { assertThat(outContent.toString().trim()).isEqualTo(failureMessage) } - private fun setUpGitHubService(issueNumbers: List) { - val issueJsons = issueNumbers.joinToString(separator = ",") { "{\"number\":$it}" } + @Test + fun testTodoCheck_PrPresent_checkShouldFail() { + setUpGitHubService( + issueNumbers = listOf(11004, 11003, 11002, 11001), + pullRequestNumbers = listOf(11005) + ) + val tempFile1 = tempFolder.newFile("testfiles/TempFile1.kt") + val tempFile2 = tempFolder.newFile("testfiles/TempFile2.kt") + val testContent1 = + """ + // TODO(#11002): test summary 1. + # TODO(#11004): test summary 2. + # TODO(#11001): test summary 3. + test Todo + test TODO + """.trimIndent() + val testContent2 = + """ + // TODO(#11005): test summary 3. + todo + + + """.trimIndent() + tempFile1.writeText(testContent1) + tempFile2.writeText(testContent2) + + val exception = assertThrows() { runScript() } + + assertThat(exception).hasMessageThat().contains(TODO_SYNTAX_CHECK_FAILED_OUTPUT_INDICATOR) + val failureMessage = + """ + TODOs not corresponding to open issues on GitHub: + - TempFile2.kt:1 + + $wikiReferenceNote + + $regenerateNote + """.trimIndent() + assertThat(outContent.toString().trim()).isEqualTo(failureMessage) + } + + private fun setUpGitHubService( + issueNumbers: List, + pullRequestNumbers: List = emptyList() + ) { + // Create JSON for issues with "pull_request" set to null + val issueJsons = issueNumbers + .joinToString(separator = ",") { "{\"number\":$it,\"pull_request\":null}" } + + // Create JSON for pull requests with "pull_request" as an empty object + val pullRequestJsons = pullRequestNumbers + .joinToString(separator = ",") { "{\"number\":$it,\"pull_request\":{}}" } + + // Combine issues and pull requests into one JSON array + val combinedJsons = + "[$issueJsons${if (pullRequestNumbers.isNotEmpty()) ", $pullRequestJsons" else ""}]" + val mockWebServer = MockWebServer() mockWebServer.enqueue(MockResponse().setBody("[$issueJsons]")) - mockWebServer.enqueue(MockResponse().setBody("[]")) // No more issues. + mockWebServer.enqueue(MockResponse().setBody(combinedJsons)) + mockWebServer.enqueue(MockResponse().setBody("[]")) GitHubClient.remoteApiUrl = mockWebServer.url("/").toString() }