Improve usability of assertions related to nodes which could be deactivated (lazy layout pool) [187188981]

Fixed

Feature Request

Status Update

No update yet.

Description

an...@google.com

created issue #1

May 4, 2021 05:47PM

Initially this bug was only about allowing to assert that node is not visible for any reason In my use case I don't care for what reason the node is not displayed. Either it is because the node is not composed at all, or it was composed, but wasn't placed. This behaviour in LazyColumn is not guaranteed as prefetching can decide to precompose an item in advance. But currently we have

assertIsNotDisplayed() which is crashing if the node is not composed
assertDoesNotExist() which is crashing if the node composed, but not placed And there is no way to write it on my own because fetchSemanticsNode with boolean is internal

Aside from that we should have a solid strategy on how we work with deactivated items(the ones from lazy list reuse pool):

Do we filter out them by default?
Do we still provide an api to gather such nodes? There are tests in SubcomposeLayout and lazy which are exactly testing the reuse pool where it is needed
Do we need isPlaced() matcher in the public api as mentioned in #11

Comments

pa...@google.com <pa...@google.com> Sep 8, 2021 04:22PM

Reassigned to je...@google.com.

je...@google.com <je...@google.com> #2Mar 31, 2022 11:47AM

Never got around to posting the stacktraces + repro, sorry!

Simple repro:

@Composable
fun NoAssociatedStateIssueRepro() {
    val state = rememberModalBottomSheetState(ModalBottomSheetValue.HalfExpanded)
    ModalBottomSheetLayout(
        sheetState = state,
        sheetContent = { Box(Modifier.height(56.dp) }
    ) {

    }
}

The same crash can be provoked by having an empty sheet content:

@Composable
fun EmptySheetContentIssueRepro() {
    val state = rememberModalBottomSheetState(ModalBottomSheetValue.HalfExpanded)
    ModalBottomSheetLayout(
        sheetState = state,
        sheetContent = { }
    ) {

    }
}

This will crash with

java.lang.IllegalArgumentException: The initial value must have an associated anchor.
        at androidx.compose.material.SwipeableState.ensureInit$material_debug(Swipeable.kt:139)
        at androidx.compose.material.SwipeableKt$swipeable$3.invoke(Swipeable.kt:579)
        at androidx.compose.material.SwipeableKt$swipeable$3.invoke(Swipeable.kt:571)
        at androidx.compose.ui.ComposedModifierKt$materialize$result$1.invoke(ComposedModifier.kt:75)
        at androidx.compose.ui.ComposedModifierKt$materialize$result$1.invoke(ComposedModifier.kt:70)
        at androidx.compose.ui.Modifier$Element$DefaultImpls.foldIn(Modifier.kt:107)
        at androidx.compose.ui.ComposedModifier.foldIn(ComposedModifier.kt:47)
        at androidx.compose.ui.CombinedModifier.foldIn(Modifier.kt:149)
        at androidx.compose.ui.ComposedModifierKt.materialize(ComposedModifier.kt:70)
        at androidx.compose.ui.layout.LayoutKt$materializerOf$1.invoke-Deg8D_g(Layout.kt:174)
        at androidx.compose.ui.layout.LayoutKt$materializerOf$1.invoke(Layout.kt:173)
        at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.kt:130)
        at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.kt:46)
        at androidx.compose.material.ModalBottomSheetKt$ModalBottomSheetLayout$2.invoke-jYbf7pk(ModalBottomSheet.kt:399)
        at androidx.compose.material.ModalBottomSheetKt$ModalBottomSheetLayout$2.invoke(ModalBottomSheet.kt:269)
        at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.kt:149)
        at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.kt:46)
        at androidx.compose.material.ModalBottomSheetKt$BottomSheetStack$1$1$placeable$1.invoke(ModalBottomSheet.kt:344)
        at androidx.compose.material.ModalBottomSheetKt$BottomSheetStack$1$1$placeable$1.invoke(ModalBottomSheet.kt:344)
        at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.kt:121)
        at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.kt:46)
        at androidx.compose.ui.layout.SubcomposeLayoutState$subcompose$2$1.invoke(SubcomposeLayout.kt:167)
        at androidx.compose.ui.layout.SubcomposeLayoutState$subcompose$2$1.invoke(SubcomposeLayout.kt:167)
        at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.kt:121)
        at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.kt:46)
        at androidx.compose.runtime.ComposerKt.invokeComposable(Composer.kt:3422)
        at androidx.compose.runtime.ComposerImpl.composeContent$runtime_debug(Composer.kt:2604)
        at androidx.compose.runtime.CompositionImpl.composeContent(Composition.kt:348)
        at androidx.compose.runtime.Recomposer.composeInitial$runtime_debug(Recomposer.kt:697)
        at androidx.compose.runtime.ComposerImpl$CompositionContextImpl.composeInitial$runtime_debug(Composer.kt:3028)
        at androidx.compose.runtime.ComposerImpl$CompositionContextImpl.composeInitial$runtime_debug(Composer.kt:3028)
        at androidx.compose.runtime.CompositionImpl.setContent(Composition.kt:304)
        at androidx.compose.ui.layout.SubcomposeLayoutState.subcomposeInto(SubcomposeLayout.kt:184)
        at androidx.compose.ui.layout.SubcomposeLayoutState.access$subcomposeInto(SubcomposeLayout.kt:100)
        at androidx.compose.ui.layout.SubcomposeLayoutState$subcompose$2.invoke(SubcomposeLayout.kt:160)
        at androidx.compose.ui.layout.SubcomposeLayoutState$subcompose$2.invoke(SubcomposeLayout.kt:158)
        at androidx.compose.runtime.snapshots.SnapshotStateObserver.withNoObservations(SnapshotStateObserver.kt:137)
        at androidx.compose.ui.node.OwnerSnapshotObserver.withNoSnapshotReadObservation$ui_debug(OwnerSnapshotObserver.kt:49)
        at androidx.compose.ui.node.LayoutNode.withNoSnapshotReadObservation$ui_debug(LayoutNode.kt:1086)
        at androidx.compose.ui.layout.SubcomposeLayoutState.subcompose(SubcomposeLayout.kt:158)
        at androidx.compose.ui.layout.SubcomposeLayoutState.subcompose(SubcomposeLayout.kt:152)

an...@google.com <an...@google.com> #3Mar 31, 2022 01:23PM

Thanks Jossi, we'll take a look into it :)

ra...@google.com <ra...@google.com> #4May 25, 2022 07:34PM

Resetting assignee to `None` in preparation of the triage bash on Thursday June 17th. Please reach out to nickanthony@ if you have any questions.

Message last modified on May 25, 2022 07:35PM

je...@google.com <je...@google.com> #5May 26, 2022 09:37AM

Any updates on this? It still crashes in 1.2.0-beta02

an...@google.com <an...@google.com> #6May 26, 2022 04:46PM

No updates for this?

ra...@google.com <ra...@google.com> #7May 26, 2022 05:58PM

It's still a problem in 1.2.1...

je...@google.com <je...@google.com> #8May 27, 2022 12:22PM

There are no updates to share on this yet. We'll update when we have news :)

cl...@google.com <cl...@google.com> #9Sep 14, 2022 11:57AM

Reassigned to an...@google.com.

Any update on this issue

Fatal Exception: java.lang.IllegalArgumentException: The initial value must have an associated anchor. at androidx.compose.material.SwipeableState.ensureInit$material_release(Swipeable.kt:138) at androidx.compose.material.SwipeableKt$swipeable$3.invoke(Swipeable.kt:594) at androidx.compose.material.SwipeableKt$swipeable$3.invoke(Swipeable.kt:573) at androidx.compose.ui.ComposedModifierKt$materialize$result$1.invoke(ComposedModifier.kt:278) at androidx.compose.ui.ComposedModifierKt$materialize$result$1.invoke(ComposedModifier.kt:273) at androidx.compose.ui.Modifier$Element.foldIn(Modifier.java:110) at androidx.compose.ui.ComposedModifier.foldIn(ComposedModifier.kt) at androidx.compose.ui.CombinedModifier.foldIn(Modifier.kt:152) at androidx.compose.ui.CombinedModifier.foldIn(Modifier.kt:152) at androidx.compose.ui.CombinedModifier.foldIn(Modifier.kt:152) at androidx.compose.ui.CombinedModifier.foldIn(Modifier.kt:152) at androidx.compose.ui.CombinedModifier.foldIn(Modifier.kt:152) at androidx.compose.ui.CombinedModifier.foldIn(Modifier.kt:152) at androidx.compose.ui.CombinedModifier.foldIn(Modifier.kt:152) at androidx.compose.ui.CombinedModifier.foldIn(Modifier.kt:152) at androidx.compose.ui.CombinedModifier.foldIn(Modifier.kt:152) at androidx.compose.ui.CombinedModifier.foldIn(Modifier.kt:152) at androidx.compose.ui.ComposedModifierKt.materialize(ComposedModifier.kt:273) at androidx.compose.ui.layout.LayoutKt$materializerOf$1.invoke-Deg8D_g(Layout.kt:226) at androidx.compose.ui.layout.LayoutKt$materializerOf$1.invoke(Layout.kt:225) at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:116) at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:34) at androidx.compose.animation.AnimatedContentKt$$InternalSyntheticOutline$53$ecb30c9002b8026b405a53f2ba709cfaa8a2c7e1404b01422d70647ebaa2f34c$1.m(AnimatedContentKt.java:2) at androidx.compose.material.SurfaceKt$Surface$1.invoke(Surface.kt:38) at androidx.compose.material.SurfaceKt$Surface$1.invoke(Surface.kt:117) at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:107) at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:34) at androidx.compose.runtime.CompositionLocalKt.CompositionLocalProvider(CompositionLocal.kt:228) at androidx.compose.material.SurfaceKt.Surface-F-jzlyU(Surface.kt:114) at androidx.compose.material.ModalBottomSheetKt$ModalBottomSheetLayout$1.invoke(ModalBottomSheet.kt:341) at androidx.compose.material.ModalBottomSheetKt$ModalBottomSheetLayout$1.invoke(ModalBottomSheet.kt:326) at androidx.compose.runtime.internal.ComposableLambdaImpl.invoke(ComposableLambda.jvm.kt:116) at androidx.compose.runtime.internal.ComposableLambdaImpl$invoke$1.invoke(ComposableLambda.jvm.kt:127) at androidx.compose.runtime.internal.ComposableLambdaImpl$invoke$1.invoke(ComposableLambda.jvm.kt:127) at androidx.compose.runtime.RecomposeScopeImpl.compose(RecomposeScopeImpl.kt:145) at androidx.compose.runtime.ComposerImpl.recomposeToGroupEnd(Composer.kt:2351) at androidx.compose.runtime.ComposerImpl.skipCurrentGroup(Composer.kt:2618) at androidx.compose.runtime.ComposerImpl$doCompose$2$5.invoke(Composer.kt:3205) at androidx.compose.runtime.ComposerImpl$doCompose$2$5.invoke(Composer.kt:3183) at androidx.compose.runtime.SnapshotStateKt__DerivedStateKt.observeDerivedStateRecalculations(DerivedState.kt:252) at androidx.compose.runtime.SnapshotStateKt.observeDerivedStateRecalculations(SnapshotStateKt.java:1) at androidx.compose.runtime.ComposerImpl.doCompose(Composer.kt:3183) at androidx.compose.runtime.ComposerImpl.recompose$runtime_release(Composer.kt:3148) at androidx.compose.runtime.CompositionImpl.recompose(Composition.kt:746) at androidx.compose.runtime.Recomposer.performRecompose(Recomposer.kt:876) at androidx.compose.runtime.Recomposer.access$performRecompose(Recomposer.kt:107) at androidx.compose.runtime.Recomposer$runRecomposeAndApplyChanges$2$2.invoke(Recomposer.kt:485) at androidx.compose.runtime.Recomposer$runRecomposeAndApplyChanges$2$2.invoke(Recomposer.kt:454) at androidx.compose.ui.platform.AndroidUiFrameClock$withFrameNanos$2$callback$1.doFrame(AndroidUiFrameClock.android.kt:34) at androidx.compose.ui.platform.AndroidUiDispatcher.performFrameDispatch(AndroidUiDispatcher.android.kt:109) at androidx.compose.ui.platform.AndroidUiDispatcher.access$performFrameDispatch(AndroidUiDispatcher.android.kt:41) at androidx.compose.ui.platform.AndroidUiDispatcher$dispatchCallback$1.doFrame(AndroidUiDispatcher.android.kt:69) at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1106) at android.view.Choreographer.doCallbacks(Choreographer.java:866) at android.view.Choreographer.doFrame(Choreographer.java:792) at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1092) at android.os.Handler.handleCallback(Handler.java:938) at android.os.Handler.dispatchMessage(Handler.java:99) at android.os.Looper.loopOnce(Looper.java:226) at android.os.Looper.loop(Looper.java:313) at android.app.ActivityThread.main(ActivityThread.java:8669) at java.lang.reflect.Method.invoke(Method.java) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:571) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1135)

an...@google.com <an...@google.com> #10Sep 16, 2022 01:43PM

Reassigned to je...@google.com.

Still happening in version 1.7.0. Even with adding an empty item in the sheetContent block.

an...@google.com <an...@google.com> #11May 23, 2023 03:28PM

No updates to share yet. We're working on the underlying APIs at the moment to address this properly.

an...@google.com <an...@google.com> May 24, 2023 05:01PM

Reassigned to an...@google.com.

rh...@gmail.com <rh...@gmail.com> #12Jul 5, 2023 09:33PM

Project: platform/frameworks/support
Branch: androidx-main

commit 3f94eba802caebc95f8f9055ffb572f6ee72ca2e
Author: Levi Albuquerque <levima@google.com>
Date: Thu Mar 16 15:15:20 2023

Migrating BottomDrawer to use SwipeableV2

Move BottomDrawer to use the new SwipeableV2 APIS.

Relnote: Update BottomDrawer internals to use the new SwipeableV2 APIs. Because of this BottomDrawerState will now only have APIS defined at the class level, it won't inherit methods/properties from SwipeableState. We're using composition with an internal SwipeableV2State. Offset is now a nullable floating point property, the current value and a swipe target value can still be accessed through currentValue and targetValue properties. The previous class level methods such as open/expand/close and properties such as isOpen/isClosed continue to be supported.

Test: Updated tests and added new ones.
Fixes: 178529942
Fixes: 220676296

Change-Id: Iad40c08ca8f48afbc451191c788e80584e874b98

M compose/material/material/api/public_plus_experimental_current.txt
M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/DrawerTest.kt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/Drawer.kt

https://android-review.googlesource.com/2494217

Message last modified on Jul 11, 2023 03:35PM

an...@google.com <an...@google.com> #13Jul 26, 2023 06:30PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.compose.material:material:1.5.0-alpha03

ae...@google.com <ae...@google.com> #14Aug 11, 2023 01:15AM

At this stage I think we should only be making assertIsDisplayed/assertIsNotDisplayed monotonically more lenient, as they're so widely used that making either of them more strict is likely to break real-world tests and prevent people from upgrading Compose.

The way they are exact inverse of each other means we cannot make one of them more lenient without making the other one more strict. So in WIP patch http://aosp/2703759, to address a different problem, I added an internal strict: Boolean parameter to allow their behavior to start to diverge. I suggest the same approach could be used to make assertIsNotDisplayed more lenient to fix this issue.

ks...@gmail.com <ks...@gmail.com> #15Aug 19, 2023 07:02PM

Hello team, we've recently encountered this issue while upgrading from Compose 1.4.3 to Compose 1.5.0-beta03. (I couldn't test against the stable version of Compose 1.5 due to AGP and compileSdkVersion requirements, but I haven't found any relevant fixes between the 1.5.0-beta03 and the stable 1.5.0 release.)

For example, the issue manifests in the following ways:

assertDoesNotExist fails because a cached node still exists.
assertIsDisplayed fails because the test framework detects more than one displayed node, including cached nodes.
Tests checking the node count within the tree report inaccurate results due to cached nodes.

Could you please take a look to investigate whether this constitutes a regression? While the workaround with the node placement check (e.g., #comment12) works, this issue leads to numerous test failures with the current lazy list usages. In any case, I would greatly appreciate it if this issue could be prioritized given the impact on testing. Also, I wondered if this could be related to the one reported here: https://issuetracker.google.com/issues/278367269. Thanks for taking a look.

Message last modified on Aug 19, 2023 07:05PM

me...@thomaskeller.biz <me...@thomaskeller.biz> #16Sep 7, 2023 02:22PM

This is a very serious regression that hit me as well when updating to a newer Compose version. IMHO the new optimization is not even dependent on the runtime version of the library, but was somehow introduced in the new Compose Compiler version 1.5.3, which needs to be used when the project's Kotlin version is updated to 1.9.10, because when I downgrade the runtime version of compose to my previous version (BOM 2023.04.01) the error persists.

Please, fix this issue as it's hard to figure out why LazyRow / LazyColumn testing suddenly breaks for no obvious reason.

ks...@gmail.com <ks...@gmail.com> #17Sep 15, 2023 08:44PM

It does look like it is a regression or at least a significant behavior change introduced in Compose 1.5. Although there was no response/acknowledgment to this FR, I found this issue, which has been closed as Won't Fix.

ap...@google.com <ap...@google.com> #18Oct 13, 2023 09:14PM

Project: platform/frameworks/support
Branch: androidx-main

commit 78d5dc8c9b42c32b0c8518c72181c19620a71c05
Author: Andrey Kulikov <andreykulikov@google.com>
Date: Thu Oct 12 18:29:24 2023

Filter out the deactivated semantic nodes by default and introduce assertIsDeactivated()

The children of SubcomposeLayout which are retained to be reused in future are considered deactivated. We were returning them along with the active nodes in all the semantics api which was hard to reason about as from the perspective of the users such nodes are not visible. Now we will filter out them by default, plus we introduce new APIs to be able to assert that the node is deactivated.

Test: modified affected tests to use the new function instead
Bug: 187188981
Bug: 302117973
Relnote: The children of SubcomposeLayout (and layouts like LazyColumn based on it) which are retained to be reused in future are considered deactivated. New assertIsDeactivated() test API was introduced to test such nodes. The rests of the test apis will filter out deactivated nodes by default.
Change-Id: I2ef84fb2ed578238bb20c07655c475df6fb8dbd0

M compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/lazy/grid/LazyGridSlotsReuseTest.kt
M compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/lazy/list/LazyListSlotsReuseTest.kt
M compose/foundation/foundation/src/androidInstrumentedTest/kotlin/androidx/compose/foundation/lazy/list/LazyNestedScrollingTest.kt
M compose/ui/ui-test/api/current.txt
M compose/ui/ui-test/api/restricted_current.txt
M compose/ui/ui-test/src/commonMain/kotlin/androidx/compose/ui/test/SemanticsNodeInteraction.kt
M compose/ui/ui-test/src/commonMain/kotlin/androidx/compose/ui/test/TestOwner.kt
M compose/ui/ui/api/current.txt
M compose/ui/ui/api/restricted_current.txt
M compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/layout/SubcomposeLayoutTest.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/LayoutInfo.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/LayoutNode.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/semantics/SemanticsOwner.kt
M paging/paging-compose/src/androidInstrumentedTest/kotlin/androidx/paging/compose/LazyPagingItemsTest.kt
M tv/tv-foundation/src/androidTest/java/androidx/tv/foundation/lazy/grid/LazyGridSlotsReuseTest.kt
M tv/tv-foundation/src/androidTest/java/androidx/tv/foundation/lazy/list/LazyListSlotsReuseTest.kt

https://android-review.googlesource.com/2787059

an...@team.bumble.com <an...@team.bumble.com> #19Oct 16, 2023 11:01AM

Thanks for resolving this issue! In which release can we expect the fix to be available for use?

an...@google.com <an...@google.com> #20Oct 17, 2023 10:42AM

Marked as fixed.

We will leave a comment here when it is released. Most likely it will be released on 1st of November.

Filed separate bug

b/305906909

for making assertIsNotDisplayed to not crash on a not composed node
And

b/305905580

for providing an api for getting a count of visible items (see

comment #11

)

Message last modified on Oct 17, 2023 10:42AM

ap...@google.com <ap...@google.com> #21Dec 22, 2023 03:26PM

Project: platform/frameworks/support
Branch: androidx-main

commit df7e5823b5e0ad1a1da88a71b44708597848769f
Author: Andrey Kulikov <andreykulikov@google.com>
Date: Wed Dec 20 14:18:39 2023

Filter out deactivated nodes from SemanticNodes.children

In aosp/2787059 we started filtering out the deactivated nodes so users can't see them in their tests. However one use case was missed:

rule.onNodeWithTag("list")
.onChildren().assertCountEquals(expectedCount)

This code will still see the deactivated nodes as we are not filtering out deactivated nodes in the list returned by SemanticNode.children.

Test: new test in SubcomposeLayoutTest
Bug: 187188981
Bug: 305905580
Fixes: 317202262
Change-Id: I4fd3d6248fc5a24bdadfdd5c60afe31386fcf7d7

M compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/layout/SubcomposeLayoutTest.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/semantics/SemanticsNode.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/semantics/SemanticsOwner.kt

https://android-review.googlesource.com/2885026

be...@gmail.com <be...@gmail.com> #22Feb 9, 2024 09:02AM

Thank you very much for providing this fix!

So do I understand that right, to make sure that a node does not exist or is not displayed, we can use assertIsDeactivated()? So assertIsDeactivated() basically means assertDoesNotExistOrIsNotDisplayed()?

Message last modified on Feb 9, 2024 09:04AM

an...@google.com <an...@google.com> #23Feb 9, 2024 01:17PM

no, just a regular `assertDoesNotExist` will now work fine even for items of lazy lists which are internally kept alive for further reuse (and currently deactivated).

assertIsNotDisplayed() is still needed for nodes, which are not deactivated, but not displayed because it is composed, but not placed, or clipped.

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