Skip to content

Commit

Permalink
Fix #5195: Fix RestrictedApi Errors (#5348)
Browse files Browse the repository at this point in the history
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->
Fixes #5195 In the Android/Lint errors, all I can see are just NewApi
Errors, which related methods are only available on API 26 and our
current min API is 19.
Correct me if I am wrong, as discussed with Ben in a meeting, most
errors test-related file can be fixed by adding require api annotation,
and for all the changed files I think they are test related so I added
annotation. I did some investigation java.util.Time replacement method
and it requires to install third-party dependency, which I think adds
too much complexity so I proceed with the annotation way. And for the
color matcher one, I cannot find the replacement method.

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [z] 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
<!-- Delete these section if this PR does not include UI-related
changes. -->
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

![Screenshot 2024-02-21
092439](https://github.com/oppia/oppia-android/assets/74568012/2f18db96-4a1e-4c9b-ab61-4986310736b1)

---------

Co-authored-by: Adhiambo Peres <[email protected]>
  • Loading branch information
XichengSpencer and adhiamboperes authored Apr 2, 2024
1 parent d0c8b81 commit f0e9ac9
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package org.oppia.android.testing.espresso

import android.graphics.drawable.GradientDrawable
import android.os.Build
import android.view.View
import androidx.annotation.RequiresApi
import org.hamcrest.Description
import org.hamcrest.Matcher
import org.hamcrest.TypeSafeMatcher
Expand All @@ -19,6 +21,7 @@ class GenericViewMatchers {
* Returns a [Matcher] that verifies a view has a fully opaque background. The view is expected
* to have a [GradientDrawable] background.
*/
@RequiresApi(Build.VERSION_CODES.N)
fun withOpaqueBackground(): Matcher<View> = withColorBackgroundMatching(
descriptionSuffix = "an opaque background"
) { color -> color?.extractAlpha() == 0xff }
Expand All @@ -27,6 +30,7 @@ class GenericViewMatchers {
* Returns a [Matcher] with the specified description suffix and color matcher, matching against
* filled background colors of views.
*/
@RequiresApi(Build.VERSION_CODES.N)
private fun withColorBackgroundMatching(
@Suppress("SameParameterValue") descriptionSuffix: String,
colorMatcher: (Long?) -> Boolean
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.oppia.android.testing.junit

import android.os.Build
import androidx.annotation.RequiresApi
import org.junit.runner.Description
import org.junit.runner.Runner
import org.junit.runner.manipulation.Filter
Expand Down Expand Up @@ -76,6 +78,7 @@ import kotlin.reflect.KClass
* contain (thus they should be treated as undefined outside of tests that specific define their
* value via [Iteration]).
*/
@RequiresApi(Build.VERSION_CODES.N)
class OppiaParameterizedTestRunner(private val testClass: Class<*>) : Suite(testClass, listOf()) {
private val parameterizedMethods = computeParameterizedMethods()
private val selectedRunnerClass by lazy { fetchSelectedRunnerPlatformClass() }
Expand All @@ -95,6 +98,7 @@ class OppiaParameterizedTestRunner(private val testClass: Class<*>) : Suite(test

override fun getChildren(): MutableList<Runner> = childrenRunners.toMutableList()

@RequiresApi(Build.VERSION_CODES.N)
private fun computeParameterizedMethods(): Map<String, ParameterizedMethod> {
val fieldsAndParsers = fetchParameterizedFields().map { field ->
val valueParser = ParameterValue.createParserForField(field)
Expand Down Expand Up @@ -184,12 +188,14 @@ class OppiaParameterizedTestRunner(private val testClass: Class<*>) : Suite(test
}.associateBy { it.methodName }
}

@RequiresApi(Build.VERSION_CODES.N)
private fun fetchParameterizedFields(): List<Field> {
return testClass.declaredFields.mapNotNull { field ->
field.getDeclaredAnnotation(Parameter::class.java)?.let { field }
}
}

@RequiresApi(Build.VERSION_CODES.N)
private fun fetchParameterizedMethodDeclarations(): List<ParameterizedMethodDeclaration> {
return testClass.declaredMethods.mapNotNull { method ->
method.getDeclaredAnnotationsByType(Iteration::class.java).map { parameters ->
Expand All @@ -208,6 +214,7 @@ class OppiaParameterizedTestRunner(private val testClass: Class<*>) : Suite(test
}
}

@RequiresApi(Build.VERSION_CODES.N)
private fun fetchSelectedRunnerPlatformClass(): Class<*> {
return checkNotNull(testClass.getDeclaredAnnotation(SelectRunnerPlatform::class.java)) {
"All suites using OppiaParameterizedTestRunner must declare their base platform runner" +
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.oppia.android.testing.threading

import android.os.Build
import androidx.annotation.RequiresApi
import org.oppia.android.testing.time.FakeSystemClock
import java.lang.reflect.Method
import java.time.Duration
Expand Down Expand Up @@ -34,6 +36,7 @@ class TestCoroutineDispatchersRobolectricImpl @Inject constructor(
} while (hasPendingCompletableTasks())
}

@RequiresApi(Build.VERSION_CODES.O)
override fun advanceTimeBy(delayTimeMillis: Long) {
var remainingDelayMillis = delayTimeMillis
while (remainingDelayMillis > 0) {
Expand All @@ -49,6 +52,7 @@ class TestCoroutineDispatchersRobolectricImpl @Inject constructor(
}
}

@RequiresApi(Build.VERSION_CODES.O)
override fun advanceUntilIdle() {
// First, run through all tasks that are currently pending and can be run immediately.
runCurrent()
Expand All @@ -67,6 +71,7 @@ class TestCoroutineDispatchersRobolectricImpl @Inject constructor(
}
}

@RequiresApi(Build.VERSION_CODES.O)
private fun advanceToNextFutureTask(
currentTimeMillis: Long,
maxDelayMs: Long = Long.MAX_VALUE
Expand Down Expand Up @@ -94,6 +99,7 @@ class TestCoroutineDispatchersRobolectricImpl @Inject constructor(
}

/** Returns whether any of the dispatchers have any tasks to run, including in the future. */
@RequiresApi(Build.VERSION_CODES.O)
private fun hasPendingTasks(): Boolean {
return backgroundTestDispatcher.hasPendingTasks() ||
blockingTestDispatcher.hasPendingTasks() ||
Expand All @@ -107,6 +113,7 @@ class TestCoroutineDispatchersRobolectricImpl @Inject constructor(
!uiTaskCoordinator.isIdle()
}

@RequiresApi(Build.VERSION_CODES.O)
private fun getNextFutureTaskTimeMillis(timeMillis: Long): Long? {
val nextBackgroundFutureTaskTimeMills =
backgroundTestDispatcher.getNextFutureTaskCompletionTimeMillis(timeMillis)
Expand All @@ -120,6 +127,7 @@ class TestCoroutineDispatchersRobolectricImpl @Inject constructor(
return futureTimes.firstOrNull()
}

@RequiresApi(Build.VERSION_CODES.O)
private fun getNextUiThreadFutureTaskTimeMillis(timeMillis: Long): Long? {
return uiTaskCoordinator.getNextUiThreadFutureTaskTimeMillis(timeMillis)
}
Expand All @@ -139,6 +147,7 @@ class TestCoroutineDispatchersRobolectricImpl @Inject constructor(
idleMethod.invoke(shadowUiLooper)
}

@RequiresApi(Build.VERSION_CODES.O)
fun getNextUiThreadFutureTaskTimeMillis(timeMillis: Long): Long? {
val nextScheduledTime = nextScheduledTimeMethod.invoke(shadowUiLooper) as Duration
val delayMs = nextScheduledTime.toMillis()
Expand Down

0 comments on commit f0e9ac9

Please sign in to comment.