diff --git a/.github/workflows/comment_coverage_report.yml b/.github/workflows/comment_coverage_report.yml index 0cc70da29a2..4a04401e88f 100644 --- a/.github/workflows/comment_coverage_report.yml +++ b/.github/workflows/comment_coverage_report.yml @@ -48,6 +48,24 @@ jobs: if: ${{ !cancelled() }} runs-on: ubuntu-latest steps: + - name: Find the latest Code Coverage Report Comment + uses: actions/github-script@v6 + with: + script: | + const comments = await github.paginate(github.rest.issues.listComments, { + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: ${{ github.event.pull_request.number }}, + }); + + for (let i = comments.length - 1; i >= 0; i--) { + if (comments[i].body.includes("## Coverage Report")) { + const latestCodeCoverageComment = comments[i].body; + require('fs').writeFileSync('latest_code_coverage_comment.md', latestCodeCoverageComment, 'utf8'); + return + } + } + - name: Find CI workflow run for PR id: find-workflow-run uses: actions/github-script@v7 @@ -82,8 +100,28 @@ jobs: name: final-coverage-report github-token: ${{ secrets.GITHUB_TOKEN }} run-id: ${{ steps.find-workflow-run.outputs.run-id }} + + - name: Compare Current Coverage Report with the Latest Coverage Report + run: | + if [ -f latest_code_coverage_comment.md ]; then + sed -i -e '$a\' CoverageReport.md + sed -i -e '$a\' latest_code_coverage_comment.md + + if diff -B CoverageReport.md latest_code_coverage_comment.md > /dev/null; then + echo "No changes detected; skipping coverage comment." + echo "skip_coverage_comment=true" >> $GITHUB_ENV + else + echo "Changes detected; proceeding with the coverage comment." + diff CoverageReport.md latest_code_coverage_comment.md || true + echo "skip_coverage_comment=false" >> $GITHUB_ENV + fi + else + echo "No previous coverage comment found to compare; posting evaluated coverage comment." + echo "skip_coverage_comment=false" >> $GITHUB_ENV + fi - name: Upload Coverage Report as PR Comment + if: ${{ env.skip_coverage_comment == 'false' }} uses: peter-evans/create-or-update-comment@v4 with: issue-number: ${{ github.event.pull_request.number }} diff --git a/.github/workflows/stats.yml b/.github/workflows/stats.yml index 74cbbeabbed..bdcb532c03e 100644 --- a/.github/workflows/stats.yml +++ b/.github/workflows/stats.yml @@ -42,13 +42,65 @@ jobs: ENABLE_CACHING: false CACHE_DIRECTORY: ~/.bazel_cache steps: + - name: Find the latest APK & AAB Analysis Comment + uses: actions/github-script@v6 + id: find_latest_apk_aab_comment + with: + script: | + const comments = await github.paginate(github.rest.issues.listComments, { + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: ${{ matrix.prInfo.number }}, + }); + + for (let i = comments.length - 1; i >= 0; i--) { + if (comments[i].body.includes("# APK & AAB differences analysis")) { + const latestComment = comments[i]; + const commentBody = latestComment.body; + const commentDate = latestComment.created_at; + + require('fs').writeFileSync('latest_aab_comment_body.log', commentBody, 'utf8'); + core.setOutput("latest_aab_comment_created_at", commentDate); + + return + } + } + + - name: Track Commits following the latest APK & AAB Analysis Comment + uses: actions/github-script@v6 + id: track_commits + with: + script: | + const commits = await github.paginate(github.rest.pulls.listCommits, { + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: ${{ matrix.prInfo.number }}, + }); + + const latestCommentDate = "${{ steps.find_latest_apk_aab_comment.outputs.latest_aab_comment_created_at }}"; + const recentCommits = commits.filter(commit => { + const commitDate = new Date(commit.commit.committer.date); + return commitDate > new Date("${{ steps.find_latest_apk_aab_comment.outputs.latest_aab_comment_created_at }}"); + }); + + const recentCommitsCount = recentCommits.length; + if(recentCommitsCount > 0 || !latestCommentDate) { + core.setOutput("new_commits", "true"); + } else { + console.log("No new commits since the last APK & AAB report; Skipping redundant analysis."); + core.setOutput("new_commits", "false"); + } + - name: Compute PR head owner/repo reference + if: ${{ steps.track_commits.outputs.new_commits == 'true' }} env: PR_HEAD_REPO: ${{ matrix.prInfo.headRepository.name }} PR_HEAD_REPO_OWNER: ${{ matrix.prInfo.headRepositoryOwner.login }} run: | echo "PR_HEAD=$PR_HEAD_REPO_OWNER/$PR_HEAD_REPO" >> "$GITHUB_ENV" + - name: Print PR information for this run + if: ${{ steps.track_commits.outputs.new_commits == 'true' }} env: PR_BASE_REF_NAME: ${{ matrix.prInfo.baseRefName }} PR_HEAD_REF_NAME: ${{ matrix.prInfo.headRefName }} @@ -57,11 +109,13 @@ jobs: echo "PR $PR_NUMBER is merging into $PR_BASE_REF_NAME from https://github.com/$PR_HEAD branch $PR_HEAD_REF_NAME." - name: Set up JDK 11 + if: ${{ steps.track_commits.outputs.new_commits == 'true' }} uses: actions/setup-java@v1 with: java-version: 11 - name: Set up Bazel + if: ${{ steps.track_commits.outputs.new_commits == 'true' }} uses: abhinavsingh/setup-bazel@v3 with: version: 6.5.0 @@ -73,6 +127,7 @@ jobs: # benefit from incremental build performance (assuming that actions/cache aggressively removes # older caches due to the 5GB cache limit size & Bazel's large cache size). - uses: actions/cache@v2 + if: ${{ steps.track_commits.outputs.new_commits == 'true' }} id: cache with: path: ${{ env.CACHE_DIRECTORY }} @@ -87,6 +142,7 @@ jobs: # few slower CI actions around the time cache is detected to be too large, but it should # incrementally improve thereafter. - name: Ensure cache size + if: ${{ steps.track_commits.outputs.new_commits == 'true' }} env: BAZEL_CACHE_DIR: ${{ env.CACHE_DIRECTORY }} run: | @@ -105,6 +161,7 @@ jobs: fi - name: Configure Bazel to use a local cache + if: ${{ steps.track_commits.outputs.new_commits == 'true' }} env: BAZEL_CACHE_DIR: ${{ env.CACHE_DIRECTORY }} run: | @@ -117,19 +174,23 @@ jobs: # run from the latest develop rather than the base branch (which might be different for # chained PRs). - name: Check out develop repository + if: ${{ steps.track_commits.outputs.new_commits == 'true' }} uses: actions/checkout@v4 with: path: develop - name: Set up build environment + if: ${{ steps.track_commits.outputs.new_commits == 'true' }} uses: ./develop/.github/actions/set-up-android-bazel-build-environment - name: Check Bazel environment + if: ${{ steps.track_commits.outputs.new_commits == 'true' }} run: | cd develop bazel info - name: Check out base repository and branch + if: ${{ steps.track_commits.outputs.new_commits == 'true' }} env: PR_BASE_REF_NAME: ${{ matrix.prInfo.baseRefName }} uses: actions/checkout@v4 @@ -139,6 +200,7 @@ jobs: path: base - name: Check out head repository and branch + if: ${{ steps.track_commits.outputs.new_commits == 'true' }} env: PR_HEAD_REF_NAME: ${{ matrix.prInfo.headRefName }} uses: actions/checkout@v4 @@ -152,6 +214,7 @@ jobs: # up being active (due to multiple repositories being used) and this can quickly overwhelm CI # worker resources. - name: Build Oppia dev, alpha, beta, and GA (feature branch) + if: ${{ steps.track_commits.outputs.new_commits == 'true' }} run: | cd head git log -n 1 @@ -163,6 +226,7 @@ jobs: bazel shutdown - name: Build Oppia dev, alpha, beta, and GA (base branch) + if: ${{ steps.track_commits.outputs.new_commits == 'true' }} run: | cd base git log -n 1 @@ -174,6 +238,7 @@ jobs: bazel shutdown - name: Run stats analysis tool (develop branch) + if: ${{ steps.track_commits.outputs.new_commits == 'true' }} run: | cd develop git log -n 1 @@ -184,66 +249,10 @@ jobs: beta $(pwd)/oppia_beta_without_changes.aab $(pwd)/oppia_beta_with_changes.aab \ ga $(pwd)/oppia_ga_without_changes.aab $(pwd)/oppia_ga_with_changes.aab - - 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: 'stats.yml', - }); - - 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[1]; - if(!run) { - core.setFailed('Could not find a succesful workflow run'); - return; - } - console.log(run.id); - - core.setOutput('run-id', run.id); - - - name: Download previous build summary - uses: actions/download-artifact@v4 - with: - name: brief_build_summary_${{ matrix.prInfo.number }} - path: ./previous_build_logs - github-token: ${{ secrets.GITHUB_TOKEN }} - run-id: ${{ steps.find-workflow-run.outputs.run-id }} - continue-on-error: true # Ignore errors if the file doesn't exist (first run) - - - name: Compare current build summary with the previous one - id: build-comparison - run: | - if [ -f ./develop/brief_build_summary.log ]; then - echo "Comparing current and previous build summaries..." - if diff ./develop/brief_build_summary.log ./previous_build_logs/brief_build_summary.log > /dev/null; then - echo "No changes detected; skipping comment." - echo "skip_comment=true" >> $GITHUB_ENV - else - echo "Changes detected; proceeding with the comment." - echo "skip_comment=false" >> $GITHUB_ENV - fi - else - echo "No previous summary found; proceeding with the comment." - echo "skip_comment=false" >> $GITHUB_ENV - fi - - - name: Upload current build summary for future comparison - uses: actions/upload-artifact@v4 - with: - name: brief_build_summary_${{ matrix.prInfo.number }} - path: ./develop/brief_build_summary.log - # Reference: https://github.com/peter-evans/create-or-update-comment#setting-the-comment-body-from-a-file. # Also, for multi-line env values, see: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#multiline-strings. - name: Extract reports for uploading & commenting + if: ${{ steps.track_commits.outputs.new_commits == 'true' }} env: PR_NUMBER: ${{ matrix.prInfo.number }} id: compute-comment-body @@ -260,7 +269,7 @@ jobs: cp "$GITHUB_WORKSPACE/develop/full_build_summary.log" "$FULL_BUILD_SUMMARY_FILE_PATH" - name: Add build stats summary comment - if: ${{ env.skip_comment == 'false' }} + if: ${{ steps.track_commits.outputs.new_commits == 'true' }} env: PR_NUMBER: ${{ matrix.prInfo.number }} uses: peter-evans/create-or-update-comment@v1 @@ -269,6 +278,7 @@ jobs: body: ${{ steps.compute-comment-body.outputs.comment_body }} - uses: actions/upload-artifact@v4 + if: ${{ steps.track_commits.outputs.new_commits == 'true' }} with: name: ${{ env.FULL_BUILD_SUMMARY_FILE_NAME }} path: ${{ env.FULL_BUILD_SUMMARY_FILE_PATH }} diff --git a/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt b/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt index 07037abd873..e04b17f162a 100644 --- a/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt @@ -72,6 +72,7 @@ class AudioViewModel @Inject constructor( } fun loadMainContentAudio(allowAutoPlay: Boolean, reloadingContent: Boolean) { + setStateAndExplorationId(state, explorationId) hasFeedback = false loadAudio(contentId = null, allowAutoPlay, reloadingContent) } @@ -88,7 +89,12 @@ class AudioViewModel @Inject constructor( * @param allowAutoPlay If false, audio is guaranteed not to be autoPlayed. */ private fun loadAudio(contentId: String?, allowAutoPlay: Boolean, reloadingMainContent: Boolean) { - val targetContentId = contentId ?: state.content.contentId + val targetContentId = when { + !contentId.isNullOrEmpty() -> contentId + this::state.isInitialized -> state.content.contentId + else -> "" + } + val voiceoverMapping = state.recordedVoiceoversMap[targetContentId] ?: VoiceoverMapping.getDefaultInstance() @@ -106,10 +112,9 @@ class AudioViewModel @Inject constructor( selectedLanguageName.set(locale.getDisplayLanguage(locale)) when { - selectedLanguageCode.isEmpty() && languages.any { - it == defaultLanguage - } -> setAudioLanguageCode(defaultLanguage) - languages.any { it == selectedLanguageCode } -> setAudioLanguageCode(selectedLanguageCode) + selectedLanguageCode.isEmpty() && languages.contains(defaultLanguage) -> + setAudioLanguageCode(defaultLanguage) + languages.contains(selectedLanguageCode) -> setAudioLanguageCode(selectedLanguageCode) languages.isNotEmpty() -> { autoPlay = false this.reloadingMainContent = false diff --git a/app/src/sharedTest/java/org/oppia/android/app/player/audio/AudioFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/player/audio/AudioFragmentTest.kt index dec40d9a790..a5edb919920 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/player/audio/AudioFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/player/audio/AudioFragmentTest.kt @@ -11,8 +11,10 @@ import androidx.appcompat.app.AppCompatActivity import androidx.test.core.app.ActivityScenario.launch import androidx.test.core.app.ApplicationProvider import androidx.test.espresso.Espresso.onView +import androidx.test.espresso.PerformException import androidx.test.espresso.UiController import androidx.test.espresso.ViewAction +import androidx.test.espresso.ViewInteraction import androidx.test.espresso.action.ViewActions.click import androidx.test.espresso.assertion.ViewAssertions.matches import androidx.test.espresso.matcher.RootMatchers.isDialog @@ -22,9 +24,12 @@ import androidx.test.espresso.matcher.ViewMatchers.isRoot import androidx.test.espresso.matcher.ViewMatchers.withContentDescription import androidx.test.espresso.matcher.ViewMatchers.withId import androidx.test.espresso.matcher.ViewMatchers.withText +import androidx.test.espresso.util.HumanReadables +import androidx.test.espresso.util.TreeIterables import androidx.test.ext.junit.runners.AndroidJUnit4 import com.google.common.truth.Truth.assertThat import dagger.Component +import org.hamcrest.CoreMatchers.allOf import org.hamcrest.Description import org.hamcrest.Matcher import org.hamcrest.TypeSafeMatcher @@ -113,6 +118,7 @@ import org.oppia.android.util.parser.image.ImageParsingModule import org.oppia.android.util.profile.CurrentUserProfileIdIntentDecorator.extractCurrentUserProfileId import org.robolectric.annotation.Config import org.robolectric.annotation.LooperMode +import java.util.concurrent.TimeoutException import javax.inject.Inject import javax.inject.Singleton @@ -125,8 +131,7 @@ import javax.inject.Singleton @RunWith(AndroidJUnit4::class) @LooperMode(LooperMode.Mode.PAUSED) @Config( - application = AudioFragmentTest.TestApplication::class, - qualifiers = "port-xxhdpi" + application = AudioFragmentTest.TestApplication::class, qualifiers = "port-xxhdpi" ) class AudioFragmentTest { @get:Rule @@ -174,8 +179,7 @@ class AudioFragmentTest { private fun createAudioFragmentTestIntent(profileId: Int): Intent { return AudioFragmentTestActivity.createAudioFragmentTestActivity( - context, - profileId + context, profileId ) } @@ -188,14 +192,13 @@ class AudioFragmentTest { ) ).use { testCoroutineDispatchers.runCurrent() - onView(withId(R.id.audio_progress_seek_bar)) - .check( - matches( - withContentDescription( - context.getString(R.string.audio_player_seekbar_content_description) - ) + onView(withId(R.id.audio_progress_seek_bar)).check( + matches( + withContentDescription( + context.getString(R.string.audio_player_seekbar_content_description) ) ) + ) } } @@ -208,14 +211,47 @@ class AudioFragmentTest { ) ).use { testCoroutineDispatchers.runCurrent() - onView(withId(R.id.audio_language_icon)) - .check( - matches( - withContentDescription( - context.getString(R.string.audio_language_icon_content_description) + onView(withId(R.id.audio_language_icon)).check( + matches( + withContentDescription( + context.getString(R.string.audio_language_icon_content_description) + ) + ) + ) + } + } + + @Test + fun testAudioFragment_playAudio_configurationChange_checkAudioPaused() { + addMediaInfo() + launch( + createAudioFragmentTestIntent( + internalProfileId + ) + ).use { + testCoroutineDispatchers.runCurrent() + + waitForTheView( + allOf( + withId(R.id.play_pause_audio_icon), + WithNonZeroDimensionsMatcher() + ) + ).perform(click()) + testCoroutineDispatchers.runCurrent() + + onView(withId(R.id.audio_progress_seek_bar)).perform(setProgress(100)) + + onView(isRoot()).perform(orientationLandscape()) + testCoroutineDispatchers.runCurrent() + onView(withId(R.id.play_pause_audio_icon)).check( + matches( + withContentDescription( + context.getString( + R.string.audio_play_description ) ) ) + ) } } @@ -228,10 +264,16 @@ class AudioFragmentTest { ) ).use { testCoroutineDispatchers.runCurrent() - onView(withId(R.id.play_pause_audio_icon)) - .check(matches(isDisplayed())) - onView(withId(R.id.play_pause_audio_icon)) - .check(matches(withContentDescription(context.getString(R.string.audio_play_description)))) + onView(withId(R.id.play_pause_audio_icon)).check(matches(isDisplayed())) + onView(withId(R.id.play_pause_audio_icon)).check( + matches( + withContentDescription( + context.getString( + R.string.audio_play_description + ) + ) + ) + ) } } @@ -250,8 +292,15 @@ class AudioFragmentTest { onView(withId(R.id.play_pause_audio_icon)).perform(click()) testCoroutineDispatchers.runCurrent() - onView(withId(R.id.play_pause_audio_icon)) - .check(matches(withContentDescription(context.getString(R.string.audio_pause_description)))) + onView(withId(R.id.play_pause_audio_icon)).check( + matches( + withContentDescription( + context.getString( + R.string.audio_pause_description + ) + ) + ) + ) } } @@ -268,8 +317,15 @@ class AudioFragmentTest { onView(withId(R.id.audio_progress_seek_bar)).perform(setProgress(100)) testCoroutineDispatchers.runCurrent() - onView(withId(R.id.play_pause_audio_icon)) - .check(matches(withContentDescription(context.getString(R.string.audio_play_description)))) + onView(withId(R.id.play_pause_audio_icon)).check( + matches( + withContentDescription( + context.getString( + R.string.audio_play_description + ) + ) + ) + ) } } @@ -290,8 +346,15 @@ class AudioFragmentTest { onView(withId(R.id.audio_progress_seek_bar)).perform(setProgress(100)) testCoroutineDispatchers.runCurrent() - onView(withId(R.id.play_pause_audio_icon)) - .check(matches(withContentDescription(context.getString(R.string.audio_pause_description)))) + onView(withId(R.id.play_pause_audio_icon)).check( + matches( + withContentDescription( + context.getString( + R.string.audio_pause_description + ) + ) + ) + ) } } @@ -307,8 +370,15 @@ class AudioFragmentTest { onView(withId(R.id.play_pause_audio_icon)).perform(click()) onView(withId(R.id.audio_progress_seek_bar)).perform(setProgress(100)) onView(isRoot()).perform(orientationLandscape()) - onView(withId(R.id.play_pause_audio_icon)) - .check(matches(withContentDescription(context.getString(R.string.audio_pause_description)))) + onView(withId(R.id.play_pause_audio_icon)).check( + matches( + withContentDescription( + context.getString( + R.string.audio_pause_description + ) + ) + ) + ) } } @@ -330,16 +400,22 @@ class AudioFragmentTest { onView(withId(R.id.audio_language_icon)).perform(click()) testCoroutineDispatchers.runCurrent() - onView(withText(R.string.hinglish_localized_language_name)) - .inRoot(isDialog()) + onView(withText(R.string.hinglish_localized_language_name)).inRoot(isDialog()) .perform(click()) testCoroutineDispatchers.runCurrent() onView(withText("Ok")).inRoot(isDialog()).perform(click()) testCoroutineDispatchers.runCurrent() - onView(withId(R.id.play_pause_audio_icon)) - .check(matches(withContentDescription(context.getString(R.string.audio_play_description)))) + onView(withId(R.id.play_pause_audio_icon)).check( + matches( + withContentDescription( + context.getString( + R.string.audio_play_description + ) + ) + ) + ) onView(withId(R.id.audio_progress_seek_bar)).check(matches(withSeekBarPosition(0))) } } @@ -540,4 +616,51 @@ class AudioFragmentTest { override fun getApplicationInjector(): ApplicationInjector = component } + /** Returns a matcher that matches view based on non-zero width and height. */ + private class WithNonZeroDimensionsMatcher : TypeSafeMatcher() { + + override fun matchesSafely(target: View): Boolean { + val targetWidth = target.width + val targetHeight = target.height + return targetWidth > 0 && targetHeight > 0 + } + + override fun describeTo(description: Description) { + description.appendText("with non-zero width and height") + } + } + + private fun waitForTheView(viewMatcher: Matcher): ViewInteraction { + return onView(isRoot()).perform(waitForMatch(viewMatcher, 30000L)) + } + + private fun waitForMatch(viewMatcher: Matcher, millis: Long): ViewAction { + return object : ViewAction { + override fun getDescription(): String { + return "wait for a specific view with matcher <$viewMatcher> during $millis millis." + } + + override fun getConstraints(): Matcher { + return isRoot() + } + + override fun perform(uiController: UiController?, view: View?) { + checkNotNull(uiController) + uiController.loopMainThreadUntilIdle() + val startTime = System.currentTimeMillis() + val endTime = startTime + millis + + do { + if (TreeIterables.breadthFirstViewTraversal(view).any { viewMatcher.matches(it) }) { + return + } + uiController.loopMainThreadForAtLeast(50) + } while (System.currentTimeMillis() < endTime) + + // Couldn't match in time. + throw PerformException.Builder().withActionDescription(description) + .withViewDescription(HumanReadables.describe(view)).withCause(TimeoutException()).build() + } + } + } }