Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions PiPSampleActivityTest_No_PiP
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package com.example.android.pip

import android.view.View
import android.widget.TextView
import androidx.test.core.app.ActivityScenario.launch
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.LargeTest
import com.google.common.truth.Truth.assertThat
import org.junit.Test
import org.junit.runner.RunWith

@RunWith(AndroidJUnit4::class)
@LargeTest
class PiPSampleActivityTest {

/**
* Verifies that the timer starts and updates the time.
*/
@Test
fun startTimer_updatesTime() {
val scenario = launch(PiPSampleActivity::class.java)

scenario.onActivity { activity ->
val startBtn = activity.findViewById<View>(R.id.start_or_pause)
val timeText = activity.findViewById<TextView>(R.id.time)

// Initial state
assertThat(timeText.text.toString()).isEqualTo("00:00:00")

// Start timer
startBtn.performClick()
}

// Wait for async update
Thread.sleep(1500)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using Thread.sleep() in UI tests is an anti-pattern that leads to flaky and slow tests. The test can fail if the async operation takes longer than the sleep duration, and it unnecessarily slows down your test suite. Please replace this with a proper synchronization mechanism, such as Espresso Idling Resources, or at least a polling loop that checks for the condition with a timeout.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix


scenario.onActivity { activity ->
val timeText = activity.findViewById<TextView>(R.id.time)

// Time should now be updated
assertThat(timeText.text.toString()).isNotEqualTo("00:00:00")
}
}

/**
* Verifies that the Clear button resets the timer.
*/
@Test
fun clearTimer_resetsTime() {
val scenario = launch(PiPSampleActivity::class.java)

scenario.onActivity { activity ->
val startBtn = activity.findViewById<View>(R.id.start_or_pause)
val clearBtn = activity.findViewById<View>(R.id.clear)
val timeText = activity.findViewById<TextView>(R.id.time)

startBtn.performClick()
clearBtn.performClick()

// Time should reset
assertThat(timeText.text.toString()).isEqualTo("00:00:00")
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This test has a potential race condition. It clicks 'start' and then 'clear' immediately within the same UI thread task. This doesn't guarantee that the timer has actually started counting before it's cleared. To make this test more robust, you should first verify that the timer is running and then test the clear functionality. The suggested change refactors the test to wait for the timer to start before clearing it. Note that Thread.sleep is used here for demonstration, but it should be replaced with a proper synchronization mechanism as mentioned in the other comment.

        scenario.onActivity { activity ->
            activity.findViewById<View>(R.id.start_or_pause).performClick()
        }

        // Wait for the timer to have started.
        // NOTE: Using Thread.sleep() here makes the test flaky and slow.
        // This should be replaced by a proper synchronization mechanism like IdlingResource.
        Thread.sleep(1500)

        scenario.onActivity { activity ->
            val timeText = activity.findViewById<TextView>(R.id.time)
            assertThat(timeText.text.toString()).isNotEqualTo("00:00:00")

            activity.findViewById<View>(R.id.clear).performClick()
            assertThat(timeText.text.toString()).isEqualTo("00:00:00")
        }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix

}
}
Original file line number Diff line number Diff line change
@@ -1,57 +1,64 @@
/*
* Copyright 2023 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.example.android.pip
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if this file should be deleted entirely? I don't think testing that timers work correctly is relevant to a developer wanting to see what tests they should implement if they implement PiP.


import androidx.test.core.app.launchActivity
import androidx.test.espresso.Espresso.onView
import androidx.test.espresso.action.ViewActions.click
import androidx.test.espresso.assertion.ViewAssertions.matches
import androidx.test.espresso.matcher.ViewMatchers.withId
import androidx.test.espresso.matcher.ViewMatchers.withText
import android.view.View
import android.widget.TextView
import androidx.test.core.app.ActivityScenario.launch
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.LargeTest
import com.google.common.truth.Truth.assertThat
import org.hamcrest.Matchers.not
import org.junit.Test
import org.junit.runner.RunWith

@RunWith(AndroidJUnit4::class)
@LargeTest
class PiPSampleActivityTest {

/**
* Verifies that the timer starts and updates the time.
*/
@Test
fun start() {
launchActivity<PiPSampleActivity>().use {
onView(withId(R.id.time)).check(matches(withText("00:00:00")))
onView(withId(R.id.start_or_pause)).perform(click())
onView(withId(R.id.time)).check(matches(not(withText("00:00:00"))))
fun startTimer_updatesTime() {
val scenario = launch(PiPSampleActivity::class.java)

scenario.onActivity { activity ->
val startBtn = activity.findViewById<View>(R.id.start_or_pause)
val timeText = activity.findViewById<TextView>(R.id.time)

// Initial state
assertThat(timeText.text.toString()).isEqualTo("00:00:00")

// Start timer
startBtn.performClick()
}

// Wait for async update
Thread.sleep(1500)

scenario.onActivity { activity ->
val timeText = activity.findViewById<TextView>(R.id.time)

// Time should now be updated
assertThat(timeText.text.toString()).isNotEqualTo("00:00:00")
}
}

/**
* Verifies that the Clear button resets the timer.
*/
@Test
fun pip() {
launchActivity<PiPSampleActivity>().use { scenario ->
scenario.onActivity { activity ->
assertThat(activity.isInPictureInPictureMode).isFalse()
}
onView(withId(R.id.pip)).perform(click())
scenario.onActivity { activity ->
assertThat(activity.isInPictureInPictureMode).isTrue()
}
fun clearTimer_resetsTime() {
val scenario = launch(PiPSampleActivity::class.java)

scenario.onActivity { activity ->
val startBtn = activity.findViewById<View>(R.id.start_or_pause)
val clearBtn = activity.findViewById<View>(R.id.clear)
val timeText = activity.findViewById<TextView>(R.id.time)

startBtn.performClick()
clearBtn.performClick()

// Time should reset
assertThat(timeText.text.toString()).isEqualTo("00:00:00")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,13 @@
android:name=".PiPSampleActivity"
android:configChanges="screenSize|smallestScreenSize|screenLayout|orientation"
android:exported="true"
android:supportsPictureInPicture="true"
android:launchMode="singleTask"
android:theme="@style/PiPAppTheme"
tools:targetApi="26" />

<activity
android:name=".PiPMovieActivity"
android:configChanges="screenSize|smallestScreenSize|screenLayout|orientation"
android:supportsPictureInPicture="true"
android:launchMode="singleTask"
android:theme="@style/PiPAppTheme"
tools:targetApi="o" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@

package com.example.android.pip

import android.app.PictureInPictureParams
import android.app.PictureInPictureUiState
import android.content.res.Configuration
import android.graphics.Rect
import android.os.Build
import android.os.Bundle
import android.support.v4.media.MediaMetadataCompat
Expand All @@ -28,14 +25,12 @@ import android.support.v4.media.session.MediaSessionCompat
import android.support.v4.media.session.PlaybackStateCompat
import android.text.util.Linkify
import android.util.Log
import android.util.Rational
import android.view.View
import androidx.activity.ComponentActivity
import androidx.annotation.RequiresApi
import androidx.core.view.WindowCompat
import androidx.core.view.WindowInsetsCompat
import androidx.core.view.WindowInsetsControllerCompat
import androidx.core.view.doOnLayout
import com.example.android.pip.databinding.PipMovieActivityBinding
import com.example.android.pip.widget.MovieView

Expand Down Expand Up @@ -90,11 +85,6 @@ class PiPMovieActivity : ComponentActivity() {
binding.movie.getVideoResourceId(),
)
}

override fun onMovieMinimized() {
// The MovieView wants us to minimize it. We enter Picture-in-Picture mode now.
minimize()
}
}

override fun onCreate(savedInstanceState: Bundle?) {
Expand All @@ -107,11 +97,6 @@ class PiPMovieActivity : ComponentActivity() {
} catch (e: Exception) {
Log.w("PiP", "Failed to add links", e)
}
binding.pip.setOnClickListener { minimize() }

// Configure parameters for the picture-in-picture mode. We do this at the first layout of
// the MovieView because we use its layout position and size.
binding.movie.doOnLayout { updatePictureInPictureParams() }

// Set up the video; it automatically starts.
binding.movie.setMovieListener(movieListener)
Expand Down Expand Up @@ -175,58 +160,6 @@ class PiPMovieActivity : ComponentActivity() {
}
}

override fun onPictureInPictureModeChanged(
isInPictureInPictureMode: Boolean, newConfig: Configuration,
) {
super.onPictureInPictureModeChanged(isInPictureInPictureMode, newConfig)
if (isInPictureInPictureMode) {
// Hide the controls in picture-in-picture mode.
binding.movie.hideControls()
} else {
// Show the video controls if the video is not playing
if (!binding.movie.isPlaying) {
binding.movie.showControls()
}
}
}

@RequiresApi(35)
override fun onPictureInPictureUiStateChanged(pipState: PictureInPictureUiState) {
super.onPictureInPictureUiStateChanged(pipState)
if (pipState.isTransitioningToPip) {
binding.movie.hideControls()
}
}


private fun updatePictureInPictureParams(): PictureInPictureParams {
// Calculate the aspect ratio of the PiP screen.
val aspectRatio = Rational(binding.movie.width, binding.movie.height)
// The movie view turns into the picture-in-picture mode.
val visibleRect = Rect()
binding.movie.getGlobalVisibleRect(visibleRect)
val params = PictureInPictureParams.Builder()
.setAspectRatio(aspectRatio)
// Specify the portion of the screen that turns into the picture-in-picture mode.
// This makes the transition animation smoother.
.setSourceRectHint(visibleRect)

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {
// The screen automatically turns into the picture-in-picture mode when it is hidden
// by the "Home" button.
params.setAutoEnterEnabled(true)
}
return params.build().also {
setPictureInPictureParams(it)
}
}

/**
* Enters Picture-in-Picture mode.
*/
private fun minimize() {
enterPictureInPictureMode(updatePictureInPictureParams())
}

/**
* Adjusts immersive full-screen flags depending on the screen orientation.
Expand Down
Loading
Loading