TASK-1828886-3: EmbeddedData tables tests#99
Conversation
5e93f8e to
ed4ddc9
Compare
ed4ddc9 to
1d93c9c
Compare
| // verify columns | ||
| columnValues.keys.forEach { | ||
| waitForNodes(it.uppercase(), count = 2) | ||
| } // despite there is only one table with column names, test sees two of them |
There was a problem hiding this comment.
Not sure if each check for table item deserves the same comment in the code. What do you think about extracting the logic somewhere else and use it like waitForTableNode(...)? Then we won't need to use count = 2 and additional comments in many places.
There was a problem hiding this comment.
good idea, it will increase code readibility
| } | ||
| } | ||
|
|
||
| private fun ComposeUiTest.allDescendantsOf(testTag: String) = |
There was a problem hiding this comment.
To be consistent with Compose APIs this function should be declared on SemanticsNodeInteractionsProvider and should start with on, e.g. onAllDescendantsOf(..).
| .onFirst().performScrollTo().performClick() | ||
| } | ||
|
|
||
| private fun SemanticsNodeInteractionCollection.findFirstWithText(text: String) = |
|
|
||
| private fun ComposeUiTest.performTextInput(testTag: String, inputText: String) { | ||
| waitUntilAtLeastOneExists(hasTestTag(testTag)) | ||
| onAllNodes(hasAnyAncestor(hasTestTag(testTag))) |
There was a problem hiding this comment.
Can't we reuse allDescendantsOf function here?
|
|
||
| private fun ComposeUiTest.performClick(testTag: String) { | ||
| waitUntilAtLeastOneExists(hasTestTag(testTag)) | ||
| onAllNodes(hasAnyAncestor(hasTestTag(testTag))) |
There was a problem hiding this comment.
Can't we reuse allDescendantsOf function here?
| // adding new record via popup | ||
| onNodeWithText("+ Add cars").performScrollTo().performClick() | ||
| waitForNode("Add Record") | ||
| allDescendantsOf("ModalViewContainer").let { nodes -> |
There was a problem hiding this comment.
nodes seems to be too general - what about modal?
There was a problem hiding this comment.
Btw, what do you think about using run or with instead of let? In that case we won't need to declare nodes everywhere, however it might be more problematic to find out on which this we are operating on.
There was a problem hiding this comment.
modal seems to be most readable for me (despite its actual a collection)
| this.filter(hasText(text)).onFirst() | ||
|
|
||
| fun ComposeUiTest.waitUntilAtLeastOneExists( | ||
| private fun ComposeUiTest.waitUntilAtLeastOneExists( |
There was a problem hiding this comment.
In fact this should be named waitUntilExactlyOneExists, and this is already implemented in ComposeUiTest, except that the original implementation does not receive nodes collection.
However, I think we still might be able to use the original one:
waitUntilExactlyOneExists(
hasAnyAncestorWithTag(carsReadonlyFieldGroup)
and hasAnyAncestorWithTag("field_group_item_2")
and hasText("Ford")
)
or even below should be enough:
waitUntilExactlyOneExists(
hasAnyAncestorWithTag("field_group_item_2") and hasText("Ford")
)
There was a problem hiding this comment.
sure it should be renamed to waitUntilExactlyOneExists
regarding rest of proposed changes:
I'm using this function in 3 places.
line 117 seems like good use of proposed solution
but in 225 and 320 I've already got the nodes so calling searching on hasAnyAncestorWithTag again would not be readable.
moreover when we want to change line 117 it cannot be
waitUntilExactlyOneExists(
hasAnyAncestorWithTag("field_group_item_2") and hasText("Ford")
)
because we have 2 repeating views with the same date and need to choose which one we are searching in
(there is "field_group_template_[Cars repeating view editable]" and "field_group_template_[Cars repeating view readonly]" and both has "Ford" inside)
| onAllNodes(hasContentDescription("Edit item 1")).onFirst().performScrollTo().performClick() | ||
| waitForNode("Edit Record") | ||
|
|
||
| allDescendantsOf("ModalViewContainer").let { nodes -> |
There was a problem hiding this comment.
nodes seems to be too general - what about modal?
| // verify table title | ||
| waitForNode("Cars editable table with popup") | ||
| // verify columns | ||
| columnValues.keys.forEach { waitForNodes(it.uppercase(), count = 2) } // despite there is only one table with column names, test sees two of them |
There was a problem hiding this comment.
| @@ -31,7 +36,7 @@ class EmbeddedDataTest : ComposeTest(PegaVersion.v24_2_2) { | |||
|
|
|||
| @Test | |||
| fun test_embedded_data_repeating_view() = runComposeUiTest { | |||
There was a problem hiding this comment.
@marekpelc-pega Overall, good job with these tests, as I feel like they cover a lot, but more complex scenarios are difficult to read, and the Compose API doesn't help much.
In the meantime, I found a pretty interesting library for writing automated tests that works with Compose and seems quite powerful and appreciated by the community: https://github.com/KasperskyLab/Kaspresso
Maybe it'll be worth trying it someday :)
| onAllNodes(hasAnyAncestor(hasTestTag(testTag))) | ||
|
|
||
| private fun SemanticsNodeInteractionCollection.filterDescendantsOf(testTag: String) = | ||
| filter(hasAnyAncestor(hasTestTag(testTag))) |
There was a problem hiding this comment.
I think that hasAnyAncestor(hasTestTag(testTag) deserves a reusable function: hasAnyAncestorWithTag(..) and then we could inline filterDescentatsOf and allDescendantsOf.
There was a problem hiding this comment.
not sure if its necessary, its just 2 calls in 2 helpers, and even after extracting this calls to separate function it will not fit 1 line
lukaszgajewski-pega
left a comment
There was a problem hiding this comment.
Just finished reviewing your changes 😉 I added a few comments - nothing urgent, more like small things to think about.
Most of them are minor and mainly related to code readability rather than logic or the tests themselves. The tests run quite well overall. I executed them locally, and they passed without issues. There was a bit of work involved, but everything looks solid 🙂
ece95fb to
61f6e4c
Compare
| const label = configProps.label ?? ""; | ||
| const showLabel = configProps.showLabel || this.DETAILS_TEMPLATES.includes(template) || this.props.showLabel; | ||
|
|
||
| const showLabel = (configProps.showLabel || this.DETAILS_TEMPLATES.includes(template)) ?? this.props.showLabel; |
There was a problem hiding this comment.
I think right side of this expression (after ?? operator) will never be used.

No description provided.