Finalize the default scrol animation spec for BringIntoViewSpec on Tv [332674072]

Fixed

Bug

Status Update

No update yet.

Description

an...@google.com

created issue #1

Apr 3, 2024 12:39PM

As part of aosp/2998197 we made a change to automatically chose the appropriate tv specific BringIntoViewSpec implementation on tv devices for our main components like LazyRow or HorizontalPager.

However, as was discussed in

https://docs.google.com/document/d/1StJcbbIALEfyTddogznrMXmz3a21xUMhPYwWi08fXWI/edit and previous documents we still need to finalize what animation spec do we use on tv. Using tween() specs for bring into view animations is not entirely correct as specs with final duration can only be used when we know the final animation delta in advance, while the nature of bring into view mechanism is different: we scroll in the needed direction without knowing the final delta and are always ready to adapt to the new reported delta (for example when more items are composed by lazy layouts in between of the currently visible items and the requested target item). Because of that in the non tv implementation we were using `spring` based animation specs, which are better suited for animations with dynamic targets.

For those reasons, we are not even sure that the animation spec should be customizable at all, we will be promoting BringIntoViewSpec api in the next release and we would be in favour of removing this property from the api at all before doing so.

What I propose is we try to reevaluate the defaults we have. Try to understand why it was needed for you to update this animation spec and maybe our default is just incorrect, so we need to update it for both tv an non tv targets.

Comments

rv...@google.com <rv...@google.com> #2Apr 4, 2024 01:14PM

Are you running it as an instrumented AndroidTest? If not can you give that a try?

rv...@google.com <rv...@google.com> #3May 14, 2024 03:34PM

No, I am running it as unit-test. Maybe will do later. Currently it has been solved by creating fake Log.java within test/android/util.

ap...@google.com <ap...@google.com> #4May 30, 2024 04:48PM

Getting the same issue in our JVM tests. The test is for a data mapper where input data is provided using PagingData.from(...) and the output is checked using asSnapshot().

This test works fine as a JVM test on 3.2.

Stacktrace:


java.lang.UnsatisfiedLinkError: 'boolean android.util.Log.isLoggable(java.lang.String, int)'
	at android.util.Log.isLoggable(Native Method)
	at androidx.paging.PagingLogger.isLoggable(PagingLogger.android.kt:29)
	at androidx.paging.PagingDataPresenter$collectFrom$2$1.emit(PagingDataPresenter.kt:521)
	at androidx.paging.PagingDataPresenter$collectFrom$2$1.emit(PagingDataPresenter.kt:118)
	at kotlinx.coroutines.flow.internal.SafeCollectorKt$emitFun$1.invoke(SafeCollector.kt:11)
	at kotlinx.coroutines.flow.internal.SafeCollectorKt$emitFun$1.invoke(SafeCollector.kt:11)
	at kotlinx.coroutines.flow.internal.SafeCollector.emit(SafeCollector.kt:113)
	at kotlinx.coroutines.flow.internal.SafeCollector.emit(SafeCollector.kt:82)
	at androidx.paging.CachedPageEventFlow$downstreamFlow$1$2.emit(CachedPageEventFlow.kt:105)
	at androidx.paging.CachedPageEventFlow$downstreamFlow$1$2.emit(CachedPageEventFlow.kt:102)
	at kotlinx.coroutines.flow.FlowKt__LimitKt$takeWhile$lambda$6$$inlined$collectWhile$1.emit(Limit.kt:140)
	at kotlinx.coroutines.flow.SubscribedFlowCollector.emit(Share.kt)
	at kotlinx.coroutines.flow.SharedFlowImpl.collect$suspendImpl(SharedFlow.kt:392)
	at kotlinx.coroutines.flow.SharedFlowImpl$collect$1.invokeSuspend(SharedFlow.kt)
	at _COROUTINE._BOUNDARY._(CoroutineDebugging.kt:42)
	at androidx.paging.SingleRunner.runInIsolation(SingleRunner.kt:49)
	at androidx.paging.testing.PagerFlowSnapshotKt$asSnapshot$4$collectPagingData$1$1.invokeSuspend(PagerFlowSnapshot.kt:119)
	at kotlinx.coroutines.flow.FlowKt__MergeKt$mapLatest$1.invokeSuspend(Merge.kt:213)
	at kotlinx.coroutines.flow.internal.ChannelFlowTransformLatest$flowCollect$3$1$2.invokeSuspend(Merge.kt:30)
	at kotlinx.coroutines.flow.internal.ChannelFlow$collectToFun$1.invokeSuspend(ChannelFlow.kt:56)
	at androidx.paging.testing.PagerFlowSnapshotKt$asSnapshot$4$collectPagingData$1.invokeSuspend(PagerFlowSnapshot.kt:117)
	at com.theguardian.myguardian.followed.feed.list.ListContentUiStateTest$when data is emitted from source then it is mapped to blueprint rows$1.invokeSuspend(ListContentUiStateTest.kt:73)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt$runTest$2$1$1.invokeSuspend(TestBuilders.kt:316)
Caused by: java.lang.UnsatisfiedLinkError: 'boolean android.util.Log.isLoggable(java.lang.String, int)'
	at android.util.Log.isLoggable(Native Method)
	at androidx.paging.PagingLogger.isLoggable(PagingLogger.android.kt:29)
	at androidx.paging.PagingDataPresenter$collectFrom$2$1.emit(PagingDataPresenter.kt:521)
	at androidx.paging.PagingDataPresenter$collectFrom$2$1.emit(PagingDataPresenter.kt:118)
	at kotlinx.coroutines.flow.internal.SafeCollectorKt$emitFun$1.invoke(SafeCollector.kt:11)
	at kotlinx.coroutines.flow.internal.SafeCollectorKt$emitFun$1.invoke(SafeCollector.kt:11)
	at kotlinx.coroutines.flow.internal.SafeCollector.emit(SafeCollector.kt:113)
	at kotlinx.coroutines.flow.internal.SafeCollector.emit(SafeCollector.kt:82)
	at androidx.paging.CachedPageEventFlow$downstreamFlow$1$2.emit(CachedPageEventFlow.kt:105)
	at androidx.paging.CachedPageEventFlow$downstreamFlow$1$2.emit(CachedPageEventFlow.kt:102)
	at kotlinx.coroutines.flow.FlowKt__LimitKt$takeWhile$lambda$6$$inlined$collectWhile$1.emit(Limit.kt:140)
	at kotlinx.coroutines.flow.SubscribedFlowCollector.emit(Share.kt)
	at kotlinx.coroutines.flow.SharedFlowImpl.collect$suspendImpl(SharedFlow.kt:392)
	at kotlinx.coroutines.flow.SharedFlowImpl$collect$1.invokeSuspend(SharedFlow.kt)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
	at kotlinx.coroutines.test.TestDispatcher.processEvent$kotlinx_coroutines_test(TestDispatcher.kt:24)
	at kotlinx.coroutines.test.TestCoroutineScheduler.tryRunNextTaskUnless$kotlinx_coroutines_test(TestCoroutineScheduler.kt:99)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt$runTest$2$1$workRunner$1.invokeSuspend(TestBuilders.kt:322)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
	at kotlinx.coroutines.EventLoopImplBase.processNextEvent(EventLoop.common.kt:277)
	at kotlinx.coroutines.BlockingCoroutine.joinBlocking(Builders.kt:95)
	at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking(Builders.kt:69)
	at kotlinx.coroutines.BuildersKt.runBlocking(Unknown Source)
	at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking$default(Builders.kt:48)
	at kotlinx.coroutines.BuildersKt.runBlocking$default(Unknown Source)
	at kotlinx.coroutines.test.TestBuildersJvmKt.createTestResult(TestBuildersJvm.kt:10)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt.runTest-8Mi8wO0(TestBuilders.kt:310)
	at kotlinx.coroutines.test.TestBuildersKt.runTest-8Mi8wO0(Unknown Source)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt.runTest-8Mi8wO0(TestBuilders.kt:168)
	at kotlinx.coroutines.test.TestBuildersKt.runTest-8Mi8wO0(Unknown Source)
	at kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt.runTest-8Mi8wO0$default(TestBuilders.kt:160)
	at kotlinx.coroutines.test.TestBuildersKt.runTest-8Mi8wO0$default(Unknown Source)
	at com.theguardian.myguardian.followed.feed.list.ListContentUiStateTest.when data is emitted from source then it is mapped to blueprint rows(ListContentUiStateTest.kt:44)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
	at java.base/java.lang.reflect.Method.invoke(Unknown Source)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.mockito.internal.runners.DefaultInternalRunner$1$1.evaluate(DefaultInternalRunner.java:55)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.mockito.internal.runners.DefaultInternalRunner$1.run(DefaultInternalRunner.java:100)
	at org.mockito.internal.runners.DefaultInternalRunner.run(DefaultInternalRunner.java:107)
	at org.mockito.internal.runners.StrictRunner.run(StrictRunner.java:42)
	at org.mockito.junit.MockitoJUnitRunner.run(MockitoJUnitRunner.java:163)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.runTestClass(JUnitTestClassExecutor.java:112)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:58)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:40)
	at org.gradle.api.internal.tasks.testing.junit.AbstractJUnitTestClassProcessor.processTestClass(AbstractJUnitTestClassProcessor.java:60)
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:52)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
	at java.base/java.lang.reflect.Method.invoke(Unknown Source)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94)
	at jdk.proxy1/jdk.proxy1.$Proxy2.processTestClass(Unknown Source)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker$2.run(TestWorker.java:176)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60)
	at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:113)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:65)
	at worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69)
	at worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)

ap...@google.com <ap...@google.com> #5Jun 10, 2024 02:14PM

It seems the problem here is using paging-runtime 3.2.x along with paging-common 3.3.0.

You likely see this error here because paging-common 3.3.0 (along with paging-compose / paging-testing) was commonized into kmp structure.

In the stacktrace you'll notice the file name PagingLogger.android.kt:29 has an android.kt suffix, meaning it was intended only for android platform (androidTests).

Re:comment#4 can you try running this in an androidTest and see if it passes, or upload a sample app?

We should probably add a constraint to ensure paging-common 3.3.0 is used with paging-runtime 3.3.0

rv...@google.com <rv...@google.com> #6Jun 10, 2024 02:18PM

Marked as fixed.

Hey!

You were right, there were different minor versions - paging-runtime 3.2.1 and paging-compose 3.3.0. However, updating both to 3.3.0 didn't fix the test issue either.

The tests do pass when running in an androidTest.

Does this mean that all paging related tests now need to run as androidTests instead of jvm tests? That'd be a bummer with the cost and time of instrumented tests :(

na...@google.com <na...@google.com> #7Jul 10, 2024 05:10PM

re comment#6 can you paste a sample test here?

Issue 332674072