Status Update
Comments
pa...@google.com <pa...@google.com>
je...@google.com <je...@google.com> #2
So IIUC, you're looking for something like assertDoesNotExistOrIsNotDisplayed()
.
I think this could be solved by providing matchers to express "exists" and "isDisplayed", so you can write assert(not(isDisplayed()) or not(exists()))
. I'll have to look back in time to see why we didn't provide those matchers in the first place.
an...@google.com <an...@google.com> #3
`assert(not(isDisplayed())`
as isDisplayed is true when it is displayed. and I want a negation of it. but current IsNotDisplayed() != !isDisplayed() as it fails if the node does not exist. but I don't care why exactly it is not displayed
ra...@google.com <ra...@google.com> #4
I found a way to do this using public API. It is not elegant, but users could use this while we come up with a better fix:
fun SemanticsNodeInteraction.assertNotDisplayed() {
try {
// If the node does not exist, it implies that it is also not displayed.
assertDoesNotExist()
} catch (e: AssertionError) {
// If the node exists, we need to assert that it is not displayed
assertIsNotDisplayed()
}
}
We can similarly use assertNotPlaced to determine if the node is composed but not placed:
fun SemanticsNodeInteraction.assertNotPlaced() {
try {
// If the node does not exist, it implies that it is also not placed.
assertDoesNotExist()
} catch (e: AssertionError) {
// If the node exists, we need to assert that it is not placed.
val errorMessageOnFail = "Assert failed: The component is placed!"
if (fetchSemanticsNode().layoutInfo.isPlaced) {
throw AssertionError(errorMessageOnFail)
}
}
}
je...@google.com <je...@google.com> #5
Sharing some of the thoughts here that were formed yesterday:
My thinking is that we can define a few levels of "existence" of a node:
- Does not exist: not in the semantics tree.
- Exists, but not placed: is in the semantics tree, but isPlaced = false. A few unanswered questions: Do cached nodes end up here? Should they? Can we recognize cached nodes?
- Placed, but not visible: in the semantics tree, isPlaced = true, but the visible region is empty.
- Partially visible: composed, placed, visible, but only X% visible (includes not visible (X=0) and fully visible (X=100)).
- Fully visible: everything (but also thinking of a parameter to indicate that "visible region == parent size" must be treated as fully visible too)
Potentially there could be "cached" in between exists and placed, but that requires that we can recognize cached nodes. I expect that is an implementation detail though, but not sure.
Currently, the negated versions of each of the above (as far as they exist in the current API) do not pass if the node does not exist, which I think was the original problem here, and which we should change while we're reshaping this API.
For the visibility assertions we need to be careful with any form of occlusion and alpha values, which makes it a bit tricky to implement. For example, what about a node that's not hidden by anything, but the draw translates it and half of it is clipped off. 100% or 50% visible? I'm tempted to use the bounds as the source of truth, which would make this case 100% visible.
an...@google.com <an...@google.com> #6
ra...@google.com <ra...@google.com> #7
I agree that this is not ideal, but don't think it is super hacky. We don't have an API to check if an item exists, but we have APIs to assert whether a node exists or not. So this solution just catches the assertion and turns it into a conditional API. Users can use something like this in their codebase for now. There is only one line in the try block so it shouldn't have any false positives:
fun SemanticsNodeInteraction.isNodePresent(): Boolean {
try {
assertExists()
} catch (e: AssertionError) {
return false
}
return true
}
Meanwhile we have aosp/2105330 which does not use this hack, and we can land it as experimental API to unblock users. However if we want to hold off and think about this more before landing that change, I think we can take the time to do so, because users have a workaround.
But as a longer term fix, maybe we should provide APIs at both levels:
Lower level API val SemanticsNodeInteraction.isNodePresent : Boolean
Higher level API fun SemanticsNodeInteraction.assertPlaced(): SemanticsNodeInteraction
and fun SemanticsNodeInteraction.assertNotPlaced()
je...@google.com <je...@google.com> #8
Actually if you use onAllNodes(someMatcher())
you can use fetchSemanticsNodes()
which will return a list that can be empty. That is what we mostly recommend for doing these kind of checks. For example, in DemoTest we implemented SemanticsNodeInteractionCollection.isNotEmpty()
isNodePresent()
.
cl...@google.com <cl...@google.com> #9
Andrey is there a clear path forward here we could take or is this worth putting on the back burner?
an...@google.com <an...@google.com> #10
an...@google.com <an...@google.com> #11
Update: there are also usecases when users want to know the amount of visible children to assert it with the expected one. The current workaround could be something like:
rule.onAllNodesWithText("item", substring = true)
.filter(isPlaced())
.assertCountEquals(1)
...
fun isPlaced() = SemanticsMatcher("isPlaced") { node -> node.layoutInfo.isPlaced }
But we need to provide an official api for this use case as well
an...@google.com <an...@google.com>
rh...@gmail.com <rh...@gmail.com> #12
Can we ask what is the use case for returning these cached nodes?
They do not affect the UI thus it seems we do not need to assert on these cached nodes when testing the UI.
With each Compose update more and more optimizations are being put into these Lazy type views.
Thus it's causing our otherwise accurate UI tests to break more and more.
Our team is confused as to why we have this complicated workaround code for simple assertions.
Our workaround
We overrode every onNode
* method we used to filter out these very unexpected nodes.
Extension Methods
// region Cache Patch
/** A patched version of [ComposeTestRule.onNode] that filters out cached views from lazy views. */
fun ComposeTestRule.onNodePatched(matcher: SemanticsMatcher) =
onNode(matcher and isNotCached())
/** A patched version of [ComposeTestRule.onNodeWithText] that filters out cached views from lazy views. */
fun ComposeTestRule.onNodeWithTextPatched(text: String, substring: Boolean = false, ignoreCase: Boolean = false, useUnmergedTree: Boolean = false) =
onAllNodesWithText(text = text, substring = substring, ignoreCase = ignoreCase, useUnmergedTree = useUnmergedTree)
.filterToOneNotCached()
/** A patched version of [ComposeTestRule.onNodeWithTag] that filters out cached views from lazy views. */
fun ComposeTestRule.onNodeWithTagPatched(testTag: String, useUnmergedTree: Boolean = false) =
onAllNodesWithTag(testTag = testTag, useUnmergedTree = useUnmergedTree)
.filterToOneNotCached()
/** A patched version of [ComposeTestRule.onNodeWithContentDescription] that filters out cached views from lazy views. */
fun ComposeTestRule.onNodeWithContentDescriptionPatched(label: String) =
onAllNodesWithContentDescription(label = label)
.filterToOneNotCached()
/** A patched version of [ComposeTestRule.onAllNodesWithText] that filters out cached views from lazy views. */
fun ComposeTestRule.onAllNodesWithTextPatched(text: String) =
onAllNodesWithText(text = text)
.filterOutCached()
/** A patched version of [ComposeTestRule.onAllNodesWithContentDescription] that filters out cached views from lazy views. */
fun ComposeTestRule.onAllNodesWithContentDescriptionPatched(label: String) =
onAllNodesWithContentDescription(label = label)
.filterOutCached()
/** Filters out cached views from lazy views and expects 1 or 0 results. */
fun SemanticsNodeInteractionCollection.filterToOneNotCached() =
filterToOne(isNotCached())
/** Filters out cached views from lazy views and expects 0 or more results. */
fun SemanticsNodeInteractionCollection.filterOutCached() =
filter(isNotCached())
/**
* Matches against only nodes that are not "cached" by lazy lists. This allows us to filter out
* nodes that do not appear in the UI but are reported by Compose's testing framework. These cached
* nodes cause issues because they will cause assertIsDisplayed to fail due to 2 nodes with the same
* values are reportedly displayed, but one is displayed and the other is cached. Cached nodes also
* cause assertDoesNotExist to fail because the cached node that does not exist on the UI does exist
* according to the test framework.
*
* This matcher is used in the methods above to globally filter out these very unexpected nodes.
* We hope Compose stops reporting these cached nodes in a future update and we can remove this patch.
*
* https://issuetracker.google.com/issues/187188981
*/
fun isNotCached() = SemanticsMatcher("isNotCached") { node -> node.layoutInfo.isPlaced }
// endregion
an...@google.com <an...@google.com> #13
Sorry about that. Agree there are no good reasons to return such cached nodes in most cases. Basically it is only really needed when you test this exact caching logic (some of our lazy layout related tests do that).
I believe they should be filtered out by default with some extra api allowing to get the unfiltered list of nodes. This bug will be dedicated to this work.
ae...@google.com <ae...@google.com> #14
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 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> #15
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.,
me...@thomaskeller.biz <me...@thomaskeller.biz> #16
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> #17
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
ap...@google.com <ap...@google.com> #18
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
an...@team.bumble.com <an...@team.bumble.com> #19
an...@google.com <an...@google.com> #20
Filed separate bug
And
ap...@google.com <ap...@google.com> #21
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
be...@gmail.com <be...@gmail.com> #22
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()
?
an...@google.com <an...@google.com> #23
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> #24
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.tv:tv-foundation:1.0.0-alpha11
Description
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
Aside from that we should have a solid strategy on how we work with deactivated items(the ones from lazy list reuse pool):