-
Notifications
You must be signed in to change notification settings - Fork 521
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix #3318: Add check to ensure all todos are addressed corresponding …
…to the closed issue (#3629) * Fix #3291: Add check for XML syntax correctness * nit fixes * Introduce XML syntax check in static checks workflow * Fix #3292: Add check for test files presence for prod files * Make nit suggestions * Make nit suggestions * Make nit suggestions * Fix #3300: Add check for accessibility labels for activities * Add CI check for label presence for activities * Add ScriptConstants file to exemptions test list * Test if: always() * Revert "Test if: always()" This reverts commit b72e419 * Add if: always() * Add if: always() * Add if: always() * Apply review suggestions on PR * Make review suggestions from regex pattern checks * Implement review suggestions based on #3340 * Refactor PR as per feedback recieved * Implement feedback suggestions * Implement review suggestions * Implement review suggestions * Implement review suggestions * Implement review suggestions * nits * Nit fixes * Nit fixes * Nit fixes * Implement feedback suggestions as per #3340 * nit fix * nit fix * nit fix * nit fix * Review suggestions part 1 * Implement review suggestions part 2 * nit fixes * update static_checks * add test to repository file * nit fix * nit fix * nit fix * Do review suggestions * Implement review suggestions * Implement review suggestions * nit fix * Implement review suggestions * Add Ben as a codeowner for the ScriptExemptions file * bazel files nit fix * Introduce a library for the regex assets * Make directory structure same as that of #3374 * Make directory structure similar to #3374 * add new files to test file script exemptions and nit fixes * diable ktlint max-line-length * disable ktlint-max-line * disable ktlint max-length * remove testonly attribute from tests * add new files to script exemptions * nit fix * nit fix * nit fix * Apply review suggestions * ktlint fix * nit fix * Apply nit fixes * Apply nit fixes * Remove script constants * nit fix * add todo * add todo * nit fixes * add test * add testOnly * nit fixes * nit fix * nit fixes * nit fixes * nit fixes * nit fixes * fix textproto file * add exemptions to textproto * sort failures lexicographically * add new files to exemption list * nit fixes * nit fixes * add kdocs * nit fix * nit fix * Trigger CI failure exclusively * Revert CI check failure * Fix #3340: Create a script to ensure KDoc presence * Add initial tests * Add more tests and add documentation * nit fix * nit fix * nit fix * nit fix * Add more files to kdoc exemptions * Implementation part 1 * add exemption proto for todos * nit fixes * delete open issues * nit fixes * nit fixes * Add redundant exemptions check * rectify static_checks * Remove duplicate proto libraries * nit fixes * nit fixes and add redundant exemptions check to script * nit fixes * add activity to exemption * nit fixes * nit fixes * nit fixes * nit fixes * revamption part 1 * nit fixes * update branch * Revamped approach * nit fixes * add kdocs * delete open_issues.json * Fix #3317: Update old todos * nit fixes * Remove resolved todos * Repurpose todos stage 1 * Repurpose todos stage 2 * Fix #3318: Add check to ensure all todos of closed issue are resolved * nit fixes * updating todos part 3 * add tests * update more todos * add network module * update todo no. 322 * correct if condition in workflow * Remove unused imports * Add duplicate comment check condition * nit fixes * Correct exit code * Revert "Remove unused imports" This reverts commit 65ee6e9. * Revert "update todo no. 322" This reverts commit c150cdc. * Repurpose no. 322 * nit fixes * replace issue number for backend model todo * add tests * link new filed issue * nit fixes * add exemptions * nit fixes * nit fixes * nit fixes * nit fixes * nit fixes * add remaining tests for all cases * nit fixes * nit fixes * nit fixes Co-authored-by: Sparsh Agrawal <[email protected]>
- Loading branch information
1 parent
aff226b
commit 2da95a5
Showing
9 changed files
with
652 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| # Contains jobs corresponding to issue related checks. | ||
|
|
||
| name: Issue Checks | ||
|
|
||
| on: | ||
| issues: | ||
| types: [closed, reopened] | ||
|
|
||
| permissions: | ||
| issues: write | ||
|
|
||
| jobs: | ||
| script_check: | ||
| name: Closed TODO Issue Check | ||
| runs-on: ubuntu-18.04 | ||
| steps: | ||
| - uses: actions/checkout@v2 | ||
|
|
||
| - name: Set up Bazel | ||
| uses: abhinavsingh/setup-bazel@v3 | ||
| with: | ||
| version: 4.0.0 | ||
|
|
||
| - name: TODO Issue Resolved Check | ||
| id: todoIssueResolvedCheck | ||
| if: ${{ github.event.action == 'closed' }} | ||
| run: | | ||
| bazel run //scripts:todo_issue_resolved_check -- $(pwd) ${{ github.event.issue.number }} ${{ github.sha }} | ||
| - name: Reopen Issue | ||
| if: failure() && steps.todoIssueResolvedCheck.outcome == 'failure' | ||
| env: | ||
| GITHUB_TOKEN: ${{ github.token }} | ||
| run: | | ||
| gh issue reopen ${{ github.event.issue.number }} | ||
| - name: Check for duplicate comment | ||
| id: duplicateCommentCheck | ||
| if: failure() && steps.todoIssueResolvedCheck.outcome == 'failure' | ||
| env: | ||
| GITHUB_TOKEN: ${{ github.token }} | ||
| run: | | ||
| gh issue view ${{ github.event.issue.number }} --json comments --jq '.comments[-1].body' > $(pwd)/latest_comment.txt | ||
| bazel run //scripts:todo_issue_comment_check -- $(pwd) latest_comment.txt script_failures.txt | ||
| - name: Add Comment | ||
| if: failure() && steps.duplicateCommentCheck.outcome == 'failure' | ||
| env: | ||
| GITHUB_TOKEN: ${{ github.token }} | ||
| run: | | ||
| gh issue comment ${{ github.event.issue.number }} -F $(pwd)/script_failures.txt |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
61 changes: 61 additions & 0 deletions
61
scripts/src/java/org/oppia/android/scripts/todo/TodoIssueCommentCheck.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| package org.oppia.android.scripts.todo | ||
|
|
||
| import java.io.File | ||
|
|
||
| /** | ||
| * Script for ensuring that the [IssueTodosResolvedCheck] failure comment is not the same as the | ||
| * latest comment of the closed issue. | ||
| * | ||
| * Usage: | ||
| * bazel run //scripts:todo_issue_comment_check -- <path_to_directory_root> | ||
| * <path_to_latest_comment_file> <path_to_script_failure_comment_file> | ||
| * | ||
| * Arguments: | ||
| * - path_to_directory_root: directory path to the root of the Oppia Android repository. | ||
| * - path_to_latest_comment_file: file path to the latest comment body. | ||
| * - path_to_script_failure_comment_file: file path to the script failure comment body. | ||
| * | ||
| * Example: | ||
| * bazel run //scripts:todo_issue_comment_check -- $(pwd) latest_comment.txt script_failures.txt | ||
| * | ||
| * NOTE TO DEVELOPERS: The script is executed in the CI enviornment. The CI workflow provides the | ||
| * file path to the latest comment body of the closed issue and the file path to the script failure | ||
| * comment body. | ||
| */ | ||
| fun main(vararg args: String) { | ||
| val repoPath = args[0] | ||
|
|
||
| val latestCommentFilePath = args[1] | ||
|
|
||
| val failureCommentFilePath = args[2] | ||
|
|
||
| val permaLinkPrefix = "https://github.com/oppia/oppia-android/blob/" | ||
|
|
||
| // Here, we are adding 40 to account for the commit SHA-1 hash length (for context: | ||
| // https://en.wikipedia.org/wiki/SHA-1). | ||
| val compareStartIndex = permaLinkPrefix.length + 40 | ||
|
|
||
| val latestCommentContentList = File(repoPath, latestCommentFilePath).readText().trim().lines() | ||
|
|
||
| val failureCommentContentList = File(repoPath, failureCommentFilePath).readText().trim().lines() | ||
|
|
||
| if (latestCommentContentList.size != failureCommentContentList.size) { | ||
| throw Exception("NEW COMMENT SHOULD BE POSTED") | ||
| } | ||
|
|
||
| if (latestCommentContentList.first() != failureCommentContentList.first()) { | ||
| throw Exception("NEW COMMENT SHOULD BE POSTED") | ||
| } | ||
|
|
||
| for (index in 1 until latestCommentContentList.size) { | ||
| // The commit SHA can vary from workflow to workflow. This can make the permalinks different. | ||
| // Hence, we are comparing by the relative file path. | ||
| val latestCommentLineContent = latestCommentContentList[index].substring(compareStartIndex) | ||
| val failureCommentLineContent = failureCommentContentList[index].substring(compareStartIndex) | ||
| if (latestCommentLineContent != failureCommentLineContent) { | ||
| throw Exception("NEW COMMENT SHOULD BE POSTED") | ||
| } | ||
| } | ||
|
|
||
| println("LATEST COMMENT IS SAME AS THE FAILURE COMMENT") | ||
| } |
105 changes: 105 additions & 0 deletions
105
scripts/src/java/org/oppia/android/scripts/todo/TodoIssueResolvedCheck.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| package org.oppia.android.scripts.todo | ||
|
|
||
| import org.oppia.android.scripts.todo.model.Todo | ||
| import java.io.File | ||
|
|
||
| /** | ||
| * Script for ensuring that all TODOs of the closed issue are resolved. | ||
| * | ||
| * Usage: | ||
| * bazel run //scripts:todo_issue_resolved_check -- <path_to_directory_root> | ||
| * <closed_issue_number> <github_sha> | ||
| * | ||
| * Arguments: | ||
| * - path_to_directory_root: directory path to the root of the Oppia Android repository. | ||
| * - closed_issue_number: issue number of the closed issue. | ||
| * - github_sha: sha of the latest commit on the develop branch. | ||
| * | ||
| * Example: | ||
| * bazel run //scripts:todo_issue_resolved_check -- $(pwd) | ||
| * 6 77ff8361b4bde52f695ceb91aa1aab36932a94fe | ||
| * | ||
| * NOTE TO DEVELOPERS: The script is executed in the CI enviornment. | ||
| */ | ||
| fun main(vararg args: String) { | ||
| // Path of the repo to be analyzed. | ||
| val repoPath = "${args[0]}/" | ||
|
|
||
| // Issue number of the closed issue. | ||
| val closedIssueNumber = args[1] | ||
|
|
||
| val commitSha = args[2] | ||
|
|
||
| val githubPermalinkUrl = "https://github.com/oppia/oppia-android/blob/$commitSha" | ||
|
|
||
| val allTodos = TodoCollector.collectCorrectlyFormattedTodos(TodoCollector.collectTodos(repoPath)) | ||
|
|
||
| val todoIssueResolvedFailures = allTodos.filter { todo -> | ||
| checkIfTodoIssueResolvedFailure( | ||
| codeLine = todo.lineContent, | ||
| closedIssueNumber = closedIssueNumber | ||
| ) | ||
| } | ||
|
|
||
| logFailures( | ||
| todoIssueResolvedFailures = todoIssueResolvedFailures, | ||
| failureMessage = "The following TODOs are unresolved for the closed issue:" | ||
| ) | ||
|
|
||
| if (todoIssueResolvedFailures.isNotEmpty()) { | ||
| generateTodoListFile(repoPath, todoIssueResolvedFailures, githubPermalinkUrl) | ||
| throw Exception("TODO ISSUE RESOLVED CHECK FAILED") | ||
| } else { | ||
| println("TODO ISSUE RESOLVED CHECK PASSED") | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Checks whether a TODO corresponds to the closed issue. | ||
| * | ||
| * @param codeLine line content corresponding to the todo | ||
| * @param closedIssueNumber issue number of the closed issue | ||
| */ | ||
| private fun checkIfTodoIssueResolvedFailure(codeLine: String, closedIssueNumber: String): Boolean { | ||
| val parsedIssueNumberFromTodo = TodoCollector.parseIssueNumberFromTodo(codeLine) | ||
| return parsedIssueNumberFromTodo == closedIssueNumber | ||
| } | ||
|
|
||
| /** | ||
| * Generates a file containing all the todos corresponding to the closed issue. | ||
| * | ||
| * @param repoPath path of the repo to be analyzed | ||
| * @param todoIssueResolvedFailures list of all the unresolved todos corresponding to the closed | ||
| * issue. | ||
| * @param githubPermalinkUrl the GitHub url for the permalinks | ||
| */ | ||
| private fun generateTodoListFile( | ||
| repoPath: String, | ||
| todoIssueResolvedFailures: List<Todo>, | ||
| githubPermalinkUrl: String | ||
| ) { | ||
| val todoListFile = File(repoPath + "script_failures.txt") | ||
| todoListFile.appendText("The issue is reopened because of the following unresolved TODOs:\n") | ||
| todoIssueResolvedFailures.sortedWith(compareBy({ it.filePath }, { it.lineNumber })) | ||
| .forEach { todo -> | ||
| todoListFile.appendText( | ||
| "$githubPermalinkUrl/${(todo.filePath).removePrefix(repoPath)}#L${todo.lineNumber}\n" | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Logs the TODO issue resolved check failures. | ||
| * | ||
| * @param todoIssueResolvedFailures list of all the unresolved todos for the closed issue | ||
| * @param failureMessage the failure message to be logged | ||
| */ | ||
| private fun logFailures(todoIssueResolvedFailures: List<Todo>, failureMessage: String) { | ||
| if (todoIssueResolvedFailures.isNotEmpty()) { | ||
| println(failureMessage) | ||
| todoIssueResolvedFailures.sortedWith(compareBy({ it.filePath }, { it.lineNumber })).forEach { | ||
| println("- ${it.filePath}:${it.lineNumber}") | ||
| } | ||
| println() | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.