From 10c8e6e1a4964121c92d19e372cef68545f27a22 Mon Sep 17 00:00:00 2001 From: Sneha Datta Date: Thu, 3 Oct 2024 16:23:08 +0530 Subject: [PATCH 01/17] Fix #5404: Migrate away from onBackPressed for remaining activities (#5526) ## Explanation Fixes #5404 This PR migrates deprecated `onBackPressed` usage to `OnBackPressedDispatcher` callback in the following activities and presenters. - ProfileEditActivity - ProfileEditActivityPresenter - QuestionPlayerActivityPresenter - WalkthroughFinalFragmentPresenter ## 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: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com> Co-authored-by: Mr. 17 --- .../app/settings/profile/ProfileEditActivity.kt | 15 ++++++++++++--- .../profile/ProfileEditActivityPresenter.kt | 3 +-- .../questionplayer/QuestionPlayerActivity.kt | 14 ++++++++++---- .../QuestionPlayerActivityPresenter.kt | 3 +-- .../app/walkthrough/WalkthroughActivity.kt | 14 ++++++++++---- .../end/WalkthroughFinalFragmentPresenter.kt | 3 +-- 6 files changed, 35 insertions(+), 17 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/settings/profile/ProfileEditActivity.kt b/app/src/main/java/org/oppia/android/app/settings/profile/ProfileEditActivity.kt index c595bd367ca..2698cfe96a7 100644 --- a/app/src/main/java/org/oppia/android/app/settings/profile/ProfileEditActivity.kt +++ b/app/src/main/java/org/oppia/android/app/settings/profile/ProfileEditActivity.kt @@ -3,6 +3,7 @@ package org.oppia.android.app.settings.profile import android.content.Context import android.content.Intent import android.os.Bundle +import androidx.activity.OnBackPressedCallback import org.oppia.android.app.activity.ActivityComponentImpl import org.oppia.android.app.activity.InjectableAutoLocalizedAppCompatActivity import org.oppia.android.app.model.ProfileEditActivityParams @@ -43,17 +44,25 @@ class ProfileEditActivity : InjectableAutoLocalizedAppCompatActivity() { super.onCreate(savedInstanceState) (activityComponent as ActivityComponentImpl).inject(this) profileEditActivityPresenter.handleOnCreate() + + onBackPressedDispatcher.addCallback( + this, + object : OnBackPressedCallback(/* enabled = */ true) { + override fun handleOnBackPressed() { + this@ProfileEditActivity.handleBackPress() + } + } + ) } - override fun onBackPressed() { + private fun handleBackPress() { val args = intent.getProtoExtra( PROFILE_EDIT_ACTIVITY_PARAMS_KEY, ProfileEditActivityParams.getDefaultInstance() ) val isMultipane = args?.isMultipane ?: false if (isMultipane) { - @Suppress("DEPRECATION") // TODO(#5404): Migrate to a back pressed dispatcher. - super.onBackPressed() + finish() } else { val intent = Intent(this, ProfileListActivity::class.java) intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP) diff --git a/app/src/main/java/org/oppia/android/app/settings/profile/ProfileEditActivityPresenter.kt b/app/src/main/java/org/oppia/android/app/settings/profile/ProfileEditActivityPresenter.kt index ea67fb89474..b89f8857de2 100644 --- a/app/src/main/java/org/oppia/android/app/settings/profile/ProfileEditActivityPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/settings/profile/ProfileEditActivityPresenter.kt @@ -31,8 +31,7 @@ class ProfileEditActivityPresenter @Inject constructor( toolbar.setNavigationOnClickListener { if (isMultipane) { - @Suppress("DEPRECATION") // TODO(#5404): Migrate to a back pressed dispatcher. - activity.onBackPressed() + activity.onBackPressedDispatcher.onBackPressed() } else { val intent = Intent(activity, ProfileListActivity::class.java) intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP) diff --git a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivity.kt b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivity.kt index 70239733952..027cbd7011f 100644 --- a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivity.kt +++ b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivity.kt @@ -3,6 +3,7 @@ package org.oppia.android.app.topic.questionplayer import android.content.Context import android.content.Intent import android.os.Bundle +import androidx.activity.OnBackPressedCallback import org.oppia.android.app.activity.ActivityComponentImpl import org.oppia.android.app.activity.InjectableAutoLocalizedAppCompatActivity import org.oppia.android.app.hintsandsolution.HintsAndSolutionListener @@ -56,11 +57,16 @@ class QuestionPlayerActivity : val profileId = intent.extractCurrentUserProfileId() questionPlayerActivityPresenter.handleOnCreate(profileId) - } - override fun onBackPressed() { - showStopExplorationDialogFragment() - questionPlayerActivityPresenter.setReadingTextSizeNormal() + onBackPressedDispatcher.addCallback( + this, + object : OnBackPressedCallback(/* enabled = */ true) { + override fun handleOnBackPressed() { + showStopExplorationDialogFragment() + questionPlayerActivityPresenter.setReadingTextSizeNormal() + } + } + ) } override fun restartSession() = questionPlayerActivityPresenter.restartSession() diff --git a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityPresenter.kt b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityPresenter.kt index 523ebf2cc4d..345941e95e1 100644 --- a/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityPresenter.kt @@ -61,8 +61,7 @@ class QuestionPlayerActivityPresenter @Inject constructor( activity.setSupportActionBar(binding.questionPlayerToolbar) binding.questionPlayerToolbar.setNavigationOnClickListener { - @Suppress("DEPRECATION") // TODO(#5404): Migrate to a back pressed dispatcher. - activity.onBackPressed() + activity.onBackPressedDispatcher.onBackPressed() } retrieveReadingTextSize().observe( diff --git a/app/src/main/java/org/oppia/android/app/walkthrough/WalkthroughActivity.kt b/app/src/main/java/org/oppia/android/app/walkthrough/WalkthroughActivity.kt index 083b293bad7..d3d7a7081e3 100644 --- a/app/src/main/java/org/oppia/android/app/walkthrough/WalkthroughActivity.kt +++ b/app/src/main/java/org/oppia/android/app/walkthrough/WalkthroughActivity.kt @@ -3,6 +3,7 @@ package org.oppia.android.app.walkthrough import android.content.Context import android.content.Intent import android.os.Bundle +import androidx.activity.OnBackPressedCallback import org.oppia.android.app.activity.ActivityComponentImpl import org.oppia.android.app.activity.InjectableAutoLocalizedAppCompatActivity import org.oppia.android.app.model.ProfileId @@ -22,6 +23,15 @@ class WalkthroughActivity : super.onCreate(savedInstanceState) (activityComponent as ActivityComponentImpl).inject(this) walkthroughActivityPresenter.handleOnCreate() + + onBackPressedDispatcher.addCallback( + this, + object : OnBackPressedCallback(/* enabled = */ true) { + override fun handleOnBackPressed() { + walkthroughActivityPresenter.handleSystemBack() + } + } + ) } override fun currentPage(walkthroughPage: Int) { @@ -33,10 +43,6 @@ class WalkthroughActivity : walkthroughActivityPresenter.changePage(walkthroughPage) } - override fun onBackPressed() { - walkthroughActivityPresenter.handleSystemBack() - } - companion object { fun createWalkthroughActivityIntent(context: Context, internalProfileId: Int): Intent { diff --git a/app/src/main/java/org/oppia/android/app/walkthrough/end/WalkthroughFinalFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/walkthrough/end/WalkthroughFinalFragmentPresenter.kt index abd286f77d8..078d79af431 100644 --- a/app/src/main/java/org/oppia/android/app/walkthrough/end/WalkthroughFinalFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/walkthrough/end/WalkthroughFinalFragmentPresenter.kt @@ -103,7 +103,6 @@ class WalkthroughFinalFragmentPresenter @Inject constructor( } override fun goBack() { - @Suppress("DEPRECATION") // TODO(#5404): Migrate to a back pressed dispatcher. - activity.onBackPressed() + activity.onBackPressedDispatcher.onBackPressed() } } From 5e140e989949bee89b1b266b535608bd2f839615 Mon Sep 17 00:00:00 2001 From: Sneha Datta Date: Wed, 9 Oct 2024 15:06:52 +0530 Subject: [PATCH 02/17] Fix #5404: Migrate away from onBackPressed for RevisionCardActivity (#5548) ## Explanation Fixes #5404 This PR migrates deprecated `onBackPressed `usage to `OnBackPressedDispatcher` callback in the RevisonCardActivity and RevisionCardActivityPresenter. ## 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 --- .../topic/revisioncard/RevisionCardActivity.kt | 17 ++++++++++------- .../RevisionCardActivityPresenter.kt | 3 +-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivity.kt b/app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivity.kt index 7606c7ea5ad..1dc30c6fd9e 100644 --- a/app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivity.kt +++ b/app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivity.kt @@ -3,6 +3,7 @@ package org.oppia.android.app.topic.revisioncard import android.content.Context import android.content.Intent import android.os.Bundle +import androidx.activity.OnBackPressedCallback import org.oppia.android.app.activity.ActivityComponentImpl import org.oppia.android.app.activity.InjectableAutoLocalizedAppCompatActivity import org.oppia.android.app.model.ProfileId @@ -56,6 +57,15 @@ class RevisionCardActivity : subtopicListSize ) } + onBackPressedDispatcher.addCallback( + this, + object : OnBackPressedCallback(/* enabled = */ true) { + override fun handleOnBackPressed() { + revisionCardActivityPresenter.setReadingTextSizeMedium() + onReturnToTopicRequested() + } + } + ) } override fun handleOnOptionsItemSelected(itemId: Int) { @@ -115,13 +125,6 @@ class RevisionCardActivity : revisionCardActivityPresenter.dismissConceptCard() } - // TODO(#5404): Migrate to a back pressed dispatcher. - @Deprecated("Deprecated in Java") - override fun onBackPressed() { - revisionCardActivityPresenter.setReadingTextSizeMedium() - onReturnToTopicRequested() - } - override fun onDefaultFontSizeLoaded(readingTextSize: ReadingTextSize) { revisionCardActivityPresenter.loadRevisionCardFragment(readingTextSize) } diff --git a/app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivityPresenter.kt b/app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivityPresenter.kt index af41963e498..c77afb97738 100644 --- a/app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivityPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivityPresenter.kt @@ -83,8 +83,7 @@ class RevisionCardActivityPresenter @Inject constructor( binding.revisionCardToolbar.setNavigationOnClickListener { (activity as ReturnToTopicClickListener).onReturnToTopicRequested() fontScaleConfigurationUtil.adjustFontScale(activity, ReadingTextSize.MEDIUM_TEXT_SIZE) - @Suppress("DEPRECATION") // TODO(#5404): Migrate to a back pressed dispatcher. - activity.onBackPressed() + activity.onBackPressedDispatcher.onBackPressed() } if (!accessibilityService.isScreenReaderEnabled()) { binding.revisionCardToolbarTitle.setOnClickListener { From 63d5e5c23548b733ca23ac17475b69629bb08a2c Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Sun, 3 Nov 2024 09:45:24 +0530 Subject: [PATCH 03/17] add pull=false --- .../org/oppia/android/scripts/common/remote/GitHubService.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt index 0fdb1dd3a19..44bb40ac35d 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt @@ -29,7 +29,7 @@ interface GitHubService { * @return the list of [GitHubIssue]s read from the remote repository (as a [Call]) */ @Headers("Accept: application/vnd.github+json", "X-GitHub-Api-Version: 2022-11-28") - @GET("repos/{repo_owner}/{repo_name}/issues?direction=asc") + @GET("repos/{repo_owner}/{repo_name}/issues?direction=asc&pulls=false") fun fetchOpenIssues( @Path("repo_owner") repoOwner: String, @Path("repo_name") repoName: String, From 5806dc7a72060867997b65e368f8907c0c8d449e Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Sun, 3 Nov 2024 10:38:07 +0530 Subject: [PATCH 04/17] add pr in todo for testing --- .../oppialogger/analytics/ApplicationLifecycleObserverTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/domain/src/test/java/org/oppia/android/domain/oppialogger/analytics/ApplicationLifecycleObserverTest.kt b/domain/src/test/java/org/oppia/android/domain/oppialogger/analytics/ApplicationLifecycleObserverTest.kt index 48e1ccc01c4..fd820bfdaab 100644 --- a/domain/src/test/java/org/oppia/android/domain/oppialogger/analytics/ApplicationLifecycleObserverTest.kt +++ b/domain/src/test/java/org/oppia/android/domain/oppialogger/analytics/ApplicationLifecycleObserverTest.kt @@ -426,7 +426,7 @@ class ApplicationLifecycleObserverTest { mapOf(TEST_FEATURE_FLAG to testFeatureFlag) ) - // TODO(#5341): Replace appSessionId generation to the modified Twitter snowflake algorithm. + // TODO(#5340): Replace appSessionId generation to the modified Twitter snowflake algorithm. val sessionIdProvider = loggingIdentifierController.getAppSessionId() val sessionId = monitorFactory.waitForNextSuccessfulResult(sessionIdProvider) From c2862f370aa7f21b3ee0016e9237084e804b97f8 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Sun, 3 Nov 2024 11:18:58 +0530 Subject: [PATCH 05/17] correct the existing todo --- .../oppialogger/analytics/ApplicationLifecycleObserverTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/domain/src/test/java/org/oppia/android/domain/oppialogger/analytics/ApplicationLifecycleObserverTest.kt b/domain/src/test/java/org/oppia/android/domain/oppialogger/analytics/ApplicationLifecycleObserverTest.kt index fd820bfdaab..48e1ccc01c4 100644 --- a/domain/src/test/java/org/oppia/android/domain/oppialogger/analytics/ApplicationLifecycleObserverTest.kt +++ b/domain/src/test/java/org/oppia/android/domain/oppialogger/analytics/ApplicationLifecycleObserverTest.kt @@ -426,7 +426,7 @@ class ApplicationLifecycleObserverTest { mapOf(TEST_FEATURE_FLAG to testFeatureFlag) ) - // TODO(#5340): Replace appSessionId generation to the modified Twitter snowflake algorithm. + // TODO(#5341): Replace appSessionId generation to the modified Twitter snowflake algorithm. val sessionIdProvider = loggingIdentifierController.getAppSessionId() val sessionId = monitorFactory.waitForNextSuccessfulResult(sessionIdProvider) From 594459c4c01b6ccd9e0f64798515d3b58f5c90a8 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Thu, 7 Nov 2024 22:19:47 +0530 Subject: [PATCH 06/17] add test for check pr --- .../scripts/common/model/GitHubIssue.kt | 5 +- .../scripts/common/remote/GitHubService.kt | 5 +- .../android/scripts/todo/TodoOpenCheckTest.kt | 57 ++++++++++++++++++- 3 files changed, 61 insertions(+), 6 deletions(-) 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..65bef402ab6 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,7 @@ 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: Boolean = false // Default to false +) diff --git a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt index 44bb40ac35d..a7e8097e7c4 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt @@ -29,12 +29,13 @@ interface GitHubService { * @return the list of [GitHubIssue]s read from the remote repository (as a [Call]) */ @Headers("Accept: application/vnd.github+json", "X-GitHub-Api-Version: 2022-11-28") - @GET("repos/{repo_owner}/{repo_name}/issues?direction=asc&pulls=false") + @GET("repos/{repo_owner}/{repo_name}/issues?direction=asc") fun fetchOpenIssues( @Path("repo_owner") repoOwner: String, @Path("repo_name") repoName: String, @Header("Authorization") authorizationBearer: String, @Query("page") pageNumber: Int, - @Query("per_page") countPerPage: Int = 100 + @Query("per_page") countPerPage: Int = 100, + @Query("pulls") pulls: Boolean = false ): Call> } 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..8301a54cf18 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt @@ -84,6 +84,38 @@ class TodoOpenCheckTest { assertThat(outContent.toString().trim()).isEqualTo(TODO_CHECK_PASSED_OUTPUT_INDICATOR) } + //added by subha + @Test + fun testTodoCheck_multipleTodosPresent_allAreValid_checkShouldPass_checkPrPresent() { + setUpGitHubService( + issueNumbers = listOf(11004, 11003, 11002, 11001), + pullRequestNumbers = listOf(11001) + ) + 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(#11001): test summary 3. + todo + + + """.trimIndent() + tempFile1.writeText(testContent1) + tempFile2.writeText(testContent2) + + runScript() + + assertThat(outContent.toString().trim()).isEqualTo(TODO_SYNTAX_CHECK_FAILED_OUTPUT_INDICATOR) + } + @Test fun testTodoCheck_onlyPoorlyFormattedTodosPresent_checkShouldFail() { setUpGitHubService(issueNumbers = emptyList()) @@ -777,11 +809,30 @@ class TodoOpenCheckTest { assertThat(outContent.toString().trim()).isEqualTo(failureMessage) } - private fun setUpGitHubService(issueNumbers: List) { - val issueJsons = issueNumbers.joinToString(separator = ",") { "{\"number\":$it}" } +// private fun setUpGitHubService(issueNumbers: List) { +// val issueJsons = issueNumbers.joinToString(separator = ",") { "{\"number\":$it}" } +// val mockWebServer = MockWebServer() +// mockWebServer.enqueue(MockResponse().setBody("[$issueJsons]")) +// mockWebServer.enqueue(MockResponse().setBody("[]")) // No more issues. +// GitHubClient.remoteApiUrl = mockWebServer.url("/").toString() +// } + + private fun setUpGitHubService(issueNumbers: List, pullRequestNumbers: List = emptyList()) { + // Create JSON for issues + val issueJsons = issueNumbers.joinToString(separator = ",") { "{\"number\":$it,\"pull_request\":false}" } + + // Create JSON for pull requests + val pullRequestJsons = pullRequestNumbers.joinToString(separator = ",") { "{\"number\":$it,\"pull_request\":true}" } + + // Combine issues and pull requests into one JSON array + val combinedJsons = "[$issueJsons${if (pullRequestNumbers.isNotEmpty()) ", $pullRequestJsons" else ""}]" + + // Set up the MockWebServer val mockWebServer = MockWebServer() - mockWebServer.enqueue(MockResponse().setBody("[$issueJsons]")) + mockWebServer.enqueue(MockResponse().setBody(combinedJsons)) mockWebServer.enqueue(MockResponse().setBody("[]")) // No more issues. + + // Set the remote API URL GitHubClient.remoteApiUrl = mockWebServer.url("/").toString() } From 7b3daf3fadab099050a5b4d24ca23f7ee12ad0e6 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Fri, 8 Nov 2024 06:59:08 +0530 Subject: [PATCH 07/17] hi --- .../org/oppia/android/scripts/todo/TodoOpenCheckTest.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 8301a54cf18..c1fb598bfa1 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt @@ -81,7 +81,8 @@ class TodoOpenCheckTest { runScript() - assertThat(outContent.toString().trim()).isEqualTo(TODO_CHECK_PASSED_OUTPUT_INDICATOR) + //subha + assertThat(outContent.toString().trim()).isEqualTo(TODO_SYNTAX_CHECK_FAILED_OUTPUT_INDICATOR) } //added by subha From 040f7cc2fe5b52d8f5d7a61033415921900470b3 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Mon, 11 Nov 2024 07:57:18 +0530 Subject: [PATCH 08/17] add pull false --- .../src/java/org/oppia/android/scripts/common/GitHubClient.kt | 2 +- .../org/oppia/android/scripts/todo/TodoOpenCheckTest.kt | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) 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..d3e460aee1a 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt @@ -65,7 +65,7 @@ class GitHubClient( private fun fetchOpenIssues(pageNumber: Int): Deferred> { return CoroutineScope(scriptBgDispatcher).async { - val call = gitHubService.fetchOpenIssues(repoOwner, repoName, authorizationBearer, pageNumber) + val call = gitHubService.fetchOpenIssues(repoOwner, repoName, authorizationBearer, pageNumber,false) // Deferred blocking I/O operation to the dedicated I/O dispatcher. val response = withContext(Dispatchers.IO) { call.execute() } check(response.isSuccessful()) { 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 c1fb598bfa1..caedb095717 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt @@ -818,6 +818,7 @@ class TodoOpenCheckTest { // GitHubClient.remoteApiUrl = mockWebServer.url("/").toString() // } + //subha private fun setUpGitHubService(issueNumbers: List, pullRequestNumbers: List = emptyList()) { // Create JSON for issues val issueJsons = issueNumbers.joinToString(separator = ",") { "{\"number\":$it,\"pull_request\":false}" } From a43c39f38ed401c59391b81cf1b5601fa06cde9d Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Mon, 11 Nov 2024 08:09:41 +0530 Subject: [PATCH 09/17] add test --- .../oppia/android/scripts/common/GitHubClient.kt | 3 ++- .../android/scripts/common/model/GitHubIssue.kt | 2 +- .../android/scripts/todo/TodoOpenCheckTest.kt | 15 ++------------- 3 files changed, 5 insertions(+), 15 deletions(-) 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 d3e460aee1a..d14401d7efd 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt @@ -65,7 +65,8 @@ class GitHubClient( private fun fetchOpenIssues(pageNumber: Int): Deferred> { return CoroutineScope(scriptBgDispatcher).async { - val call = gitHubService.fetchOpenIssues(repoOwner, repoName, authorizationBearer, pageNumber,false) + val call = gitHubService + .fetchOpenIssues(repoOwner, repoName, authorizationBearer, pageNumber,false) // Deferred blocking I/O operation to the dedicated I/O dispatcher. val response = withContext(Dispatchers.IO) { call.execute() } check(response.isSuccessful()) { 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 65bef402ab6..f0005a0fd91 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 @@ -12,5 +12,5 @@ import com.squareup.moshi.JsonClass @JsonClass(generateAdapter = true) data class GitHubIssue( @Json(name = "number") val number: Int, - @Json(name = "pull_request") val pullRequest: Boolean = false // Default to false + @Json(name = "pull_request") val pullRequest: Boolean = false ) 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 caedb095717..0b57593de74 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt @@ -81,13 +81,11 @@ class TodoOpenCheckTest { runScript() - //subha - assertThat(outContent.toString().trim()).isEqualTo(TODO_SYNTAX_CHECK_FAILED_OUTPUT_INDICATOR) + assertThat(outContent.toString().trim()).isEqualTo(TODO_CHECK_PASSED_OUTPUT_INDICATOR) } - //added by subha @Test - fun testTodoCheck_multipleTodosPresent_allAreValid_checkShouldPass_checkPrPresent() { + fun testTodoCheck_PrPresent_checkShouldFail() { setUpGitHubService( issueNumbers = listOf(11004, 11003, 11002, 11001), pullRequestNumbers = listOf(11001) @@ -810,15 +808,6 @@ class TodoOpenCheckTest { assertThat(outContent.toString().trim()).isEqualTo(failureMessage) } -// private fun setUpGitHubService(issueNumbers: List) { -// val issueJsons = issueNumbers.joinToString(separator = ",") { "{\"number\":$it}" } -// val mockWebServer = MockWebServer() -// mockWebServer.enqueue(MockResponse().setBody("[$issueJsons]")) -// mockWebServer.enqueue(MockResponse().setBody("[]")) // No more issues. -// GitHubClient.remoteApiUrl = mockWebServer.url("/").toString() -// } - - //subha private fun setUpGitHubService(issueNumbers: List, pullRequestNumbers: List = emptyList()) { // Create JSON for issues val issueJsons = issueNumbers.joinToString(separator = ",") { "{\"number\":$it,\"pull_request\":false}" } From e01392b25a4d9663f69e9e740adce7f004ce6458 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Mon, 11 Nov 2024 08:11:36 +0530 Subject: [PATCH 10/17] correct klint --- .../oppia/android/scripts/common/GitHubClient.kt | 2 +- .../android/scripts/todo/TodoOpenCheckTest.kt | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) 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 d14401d7efd..ff4a72d3734 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt @@ -66,7 +66,7 @@ class GitHubClient( private fun fetchOpenIssues(pageNumber: Int): Deferred> { return CoroutineScope(scriptBgDispatcher).async { val call = gitHubService - .fetchOpenIssues(repoOwner, repoName, authorizationBearer, pageNumber,false) + .fetchOpenIssues(repoOwner, repoName, authorizationBearer, pageNumber, false) // Deferred blocking I/O operation to the dedicated I/O dispatcher. val response = withContext(Dispatchers.IO) { call.execute() } check(response.isSuccessful()) { 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 0b57593de74..27ebce75c86 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt @@ -808,15 +808,21 @@ class TodoOpenCheckTest { assertThat(outContent.toString().trim()).isEqualTo(failureMessage) } - private fun setUpGitHubService(issueNumbers: List, pullRequestNumbers: List = emptyList()) { + private fun setUpGitHubService( + issueNumbers: List, + pullRequestNumbers: List = emptyList() + ) { // Create JSON for issues - val issueJsons = issueNumbers.joinToString(separator = ",") { "{\"number\":$it,\"pull_request\":false}" } + val issueJsons = issueNumbers + .joinToString(separator = ",") { "{\"number\":$it,\"pull_request\":false}" } // Create JSON for pull requests - val pullRequestJsons = pullRequestNumbers.joinToString(separator = ",") { "{\"number\":$it,\"pull_request\":true}" } + val pullRequestJsons = pullRequestNumbers + .joinToString(separator = ",") { "{\"number\":$it,\"pull_request\":true}" } // Combine issues and pull requests into one JSON array - val combinedJsons = "[$issueJsons${if (pullRequestNumbers.isNotEmpty()) ", $pullRequestJsons" else ""}]" + val combinedJsons = + "[$issueJsons${if (pullRequestNumbers.isNotEmpty()) ", $pullRequestJsons" else ""}]" // Set up the MockWebServer val mockWebServer = MockWebServer() From 5618c2337d791fdadcfe060b8b93b49c0f8fc702 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Mon, 11 Nov 2024 08:24:05 +0530 Subject: [PATCH 11/17] remove false from fetchOpenIssue --- .../src/java/org/oppia/android/scripts/common/GitHubClient.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 ff4a72d3734..fd7cc105037 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/GitHubClient.kt @@ -65,8 +65,7 @@ class GitHubClient( private fun fetchOpenIssues(pageNumber: Int): Deferred> { return CoroutineScope(scriptBgDispatcher).async { - val call = gitHubService - .fetchOpenIssues(repoOwner, repoName, authorizationBearer, pageNumber, false) + 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()) { From f5b6e61dbce68e39eff63ec831b9358decc9cb2b Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Mon, 11 Nov 2024 13:34:18 +0530 Subject: [PATCH 12/17] modify githubissue model --- .../oppia/android/scripts/common/GitHubClient.kt | 3 ++- .../android/scripts/common/model/GitHubIssue.kt | 7 ++++++- .../android/scripts/common/remote/GitHubService.kt | 3 +-- .../android/scripts/todo/TodoOpenCheckTest.kt | 14 ++++++++------ 4 files changed, 17 insertions(+), 10 deletions(-) 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 f0005a0fd91..8c6828fbbac 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 @@ -12,5 +12,10 @@ import com.squareup.moshi.JsonClass @JsonClass(generateAdapter = true) data class GitHubIssue( @Json(name = "number") val number: Int, - @Json(name = "pull_request") val pullRequest: Boolean = false + @Json(name = "pull_request") val pullRequest: PullRequestInfo? = null +) + +@JsonClass(generateAdapter = true) +data class PullRequestInfo( + @Json(name = "url") val url: String? = null ) diff --git a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt index a7e8097e7c4..0fdb1dd3a19 100644 --- a/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt +++ b/scripts/src/java/org/oppia/android/scripts/common/remote/GitHubService.kt @@ -35,7 +35,6 @@ interface GitHubService { @Path("repo_name") repoName: String, @Header("Authorization") authorizationBearer: String, @Query("page") pageNumber: Int, - @Query("per_page") countPerPage: Int = 100, - @Query("pulls") pulls: Boolean = false + @Query("per_page") countPerPage: Int = 100 ): Call> } 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 27ebce75c86..a5779268b31 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt @@ -812,13 +812,13 @@ class TodoOpenCheckTest { issueNumbers: List, pullRequestNumbers: List = emptyList() ) { - // Create JSON for issues + // Create JSON for issues with "pull_request" set to null val issueJsons = issueNumbers - .joinToString(separator = ",") { "{\"number\":$it,\"pull_request\":false}" } + .joinToString(separator = ",") { "{\"number\":$it,\"pull_request\":null}" } - // Create JSON for pull requests + // Create JSON for pull requests with "pull_request" as an empty object val pullRequestJsons = pullRequestNumbers - .joinToString(separator = ",") { "{\"number\":$it,\"pull_request\":true}" } + .joinToString(separator = ",") { "{\"number\":$it,\"pull_request\":{}}" } // Combine issues and pull requests into one JSON array val combinedJsons = @@ -826,13 +826,15 @@ class TodoOpenCheckTest { // Set up the MockWebServer val mockWebServer = MockWebServer() - mockWebServer.enqueue(MockResponse().setBody(combinedJsons)) - mockWebServer.enqueue(MockResponse().setBody("[]")) // No more issues. + mockWebServer.enqueue(MockResponse().setBody("[$issueJsons]")) // Only issues + mockWebServer.enqueue(MockResponse().setBody(combinedJsons)) // Issues + pull requests + mockWebServer.enqueue(MockResponse().setBody("[]")) // No more issues. // Set the remote API URL GitHubClient.remoteApiUrl = mockWebServer.url("/").toString() } + private fun setUpSupportForGhAuth(authToken: String) { fakeCommandExecutor.registerHandler("gh") { _, args, outputStream, _ -> when (args) { From 42fccfb1757b91c76010c613c56c69dffa1f3ffb Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Mon, 11 Nov 2024 13:35:29 +0530 Subject: [PATCH 13/17] solved klint --- .../org/oppia/android/scripts/todo/TodoOpenCheckTest.kt | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) 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 a5779268b31..fd47f3a2bc6 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt @@ -826,15 +826,14 @@ class TodoOpenCheckTest { // Set up the MockWebServer val mockWebServer = MockWebServer() - mockWebServer.enqueue(MockResponse().setBody("[$issueJsons]")) // Only issues - mockWebServer.enqueue(MockResponse().setBody(combinedJsons)) // Issues + pull requests - mockWebServer.enqueue(MockResponse().setBody("[]")) // No more issues. + mockWebServer.enqueue(MockResponse().setBody("[$issueJsons]")) + mockWebServer.enqueue(MockResponse().setBody(combinedJsons)) + mockWebServer.enqueue(MockResponse().setBody("[]")) // Set the remote API URL GitHubClient.remoteApiUrl = mockWebServer.url("/").toString() } - private fun setUpSupportForGhAuth(authToken: String) { fakeCommandExecutor.registerHandler("gh") { _, args, outputStream, _ -> when (args) { From 2bd406a754f2b6ca5b89f3c875c04508eae1afde Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Mon, 11 Nov 2024 13:50:10 +0530 Subject: [PATCH 14/17] fix exemptions --- .../android/scripts/todo/TodoOpenCheckTest.kt | 62 +++++++++---------- 1 file changed, 31 insertions(+), 31 deletions(-) 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 fd47f3a2bc6..b6eea8a281e 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt @@ -84,37 +84,6 @@ class TodoOpenCheckTest { assertThat(outContent.toString().trim()).isEqualTo(TODO_CHECK_PASSED_OUTPUT_INDICATOR) } - @Test - fun testTodoCheck_PrPresent_checkShouldFail() { - setUpGitHubService( - issueNumbers = listOf(11004, 11003, 11002, 11001), - pullRequestNumbers = listOf(11001) - ) - 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(#11001): test summary 3. - todo - - - """.trimIndent() - tempFile1.writeText(testContent1) - tempFile2.writeText(testContent2) - - runScript() - - assertThat(outContent.toString().trim()).isEqualTo(TODO_SYNTAX_CHECK_FAILED_OUTPUT_INDICATOR) - } - @Test fun testTodoCheck_onlyPoorlyFormattedTodosPresent_checkShouldFail() { setUpGitHubService(issueNumbers = emptyList()) @@ -808,6 +777,37 @@ class TodoOpenCheckTest { assertThat(outContent.toString().trim()).isEqualTo(failureMessage) } + @Test + fun testTodoCheck_PrPresent_checkShouldFail() { + setUpGitHubService( + issueNumbers = listOf(11004, 11003, 11002, 11001), + pullRequestNumbers = listOf(11001) + ) + 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(#11001): test summary 3. + todo + + + """.trimIndent() + tempFile1.writeText(testContent1) + tempFile2.writeText(testContent2) + + runScript() + + assertThat(outContent.toString().trim()).isEqualTo(TODO_SYNTAX_CHECK_FAILED_OUTPUT_INDICATOR) + } + private fun setUpGitHubService( issueNumbers: List, pullRequestNumbers: List = emptyList() From 277fabb40604e3f31aff2bb4cbdcbad8621d6a41 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Mon, 11 Nov 2024 19:33:26 +0530 Subject: [PATCH 15/17] add lines in exemptions --- scripts/assets/todo_open_exemptions.textproto | 5 +++++ .../org/oppia/android/scripts/todo/TodoOpenCheckTest.kt | 4 ++-- 2 files changed, 7 insertions(+), 2 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/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt index b6eea8a281e..c60205120ca 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt @@ -781,7 +781,7 @@ class TodoOpenCheckTest { fun testTodoCheck_PrPresent_checkShouldFail() { setUpGitHubService( issueNumbers = listOf(11004, 11003, 11002, 11001), - pullRequestNumbers = listOf(11001) + pullRequestNumbers = listOf(11005) ) val tempFile1 = tempFolder.newFile("testfiles/TempFile1.kt") val tempFile2 = tempFolder.newFile("testfiles/TempFile2.kt") @@ -795,7 +795,7 @@ class TodoOpenCheckTest { """.trimIndent() val testContent2 = """ - // TODO(#11001): test summary 3. + // TODO(#11005): test summary 3. todo From 2c505b0a6f2b480c58850da3bb0a25cae2d139d5 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Mon, 11 Nov 2024 20:35:11 +0530 Subject: [PATCH 16/17] correct Kdoc and test --- .../android/scripts/common/model/GitHubIssue.kt | 6 ++++++ .../android/scripts/todo/TodoOpenCheckTest.kt | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) 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 8c6828fbbac..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 @@ -15,6 +15,12 @@ data class GitHubIssue( @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 c60205120ca..b2c6cfdd42a 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt @@ -803,9 +803,19 @@ class TodoOpenCheckTest { tempFile1.writeText(testContent1) tempFile2.writeText(testContent2) - runScript() + val exception = assertThrows() { runScript() } - assertThat(outContent.toString().trim()).isEqualTo(TODO_SYNTAX_CHECK_FAILED_OUTPUT_INDICATOR) + 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( From e8e2212c5782c7ae098328436a9691718b50b091 Mon Sep 17 00:00:00 2001 From: Subhajit Mallick <153619690+subhajitxyz@users.noreply.github.com> Date: Mon, 11 Nov 2024 22:33:34 +0530 Subject: [PATCH 17/17] remove unnessery comments --- .../org/oppia/android/scripts/todo/TodoOpenCheckTest.kt | 3 --- 1 file changed, 3 deletions(-) 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 b2c6cfdd42a..b5a3f783b70 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/todo/TodoOpenCheckTest.kt @@ -834,13 +834,10 @@ class TodoOpenCheckTest { val combinedJsons = "[$issueJsons${if (pullRequestNumbers.isNotEmpty()) ", $pullRequestJsons" else ""}]" - // Set up the MockWebServer val mockWebServer = MockWebServer() mockWebServer.enqueue(MockResponse().setBody("[$issueJsons]")) mockWebServer.enqueue(MockResponse().setBody(combinedJsons)) mockWebServer.enqueue(MockResponse().setBody("[]")) - - // Set the remote API URL GitHubClient.remoteApiUrl = mockWebServer.url("/").toString() }