Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Compose Compiler plugin update causing compose instrumented test to fail #736

Open
devPalacio opened this issue Jul 18, 2024 · 5 comments
Open

Compose Compiler plugin update causing compose instrumented test to fail #736

devPalacio opened this issue Jul 18, 2024 · 5 comments
Labels
bug Something isn't working Platform: Android

Comments

@devPalacio
Copy link

devPalacio commented Jul 18, 2024 •
edited
Loading

Gradle Version

8.7

AGP Version

8.5.0

Code Minifier/Optimizer

None

Version

4.10.0

Sentry SDK Version

7.12.0

Steps to Reproduce

I have a simple compose test that started failing when updating Sentry from 4.5.1/7.9.0 to the latest version. It's a fairly basic AndroidComposeRule test that renders a search bar composable with the test tag searchbar, and using hasParent asserts that it's focused. I believe this regression was introduced with the K2 compiler changes.

A workaround I found is to use hasAnyAncestor but I'd like to understand why the tree changed and if it was intentional.

Kotlin Version: 1.9.23
Compose BOM: 2024.05.00
Compose Compiler: 1.5.13

Expected Result

Test passes. Here's the semantic nodes of the passing test on 4.5.1/7.9.0

Printing with useUnmergedTree = 'false'
Node #1 at (l=0.0, t=210.0, r=1080.0, b=357.0)px
|-Node #2 at (l=0.0, t=210.0, r=1080.0, b=357.0)px, Tag: 'searchbar'
 IsTraversalGroup = 'true'
  |-Node #7 at (l=43.0, t=210.0, r=1069.0, b=357.0)px
    EditableText = 'query'
    TextSelectionRange = 'TextRange(0, 0)'
    ImeAction = 'Default'
    Focused = 'true'
    SentryTag = 'SearchBar'
    Actions = [GetTextLayoutResult, SetText, InsertTextAtCursor, SetSelection, PerformImeAction, OnClick, OnLongClick, PasteText, RequestFocus, MagnifierPositionInRoot]
    MergeDescendants = 'true'

Actual Result

Here's the result on the latest Sentry versions

Printing with useUnmergedTree = 'false'
Node #1 at (l=0.0, t=210.0, r=1080.0, b=357.0)px
|-Node #2 at (l=0.0, t=210.0, r=1080.0, b=357.0)px, Tag: 'searchbar'
 IsTraversalGroup = 'true'
 SentryTag = 'NavigationBar'
  |-Node #4 at (l=11.0, t=284.0, r=43.0, b=284.0)px
  | SentryTag = 'SearchBar'
  |-Node #5 at (l=43.0, t=210.0, r=1069.0, b=357.0)px
    SentryTag = 'SearchBar'
     |-Node #6 at (l=43.0, t=210.0, r=1069.0, b=357.0)px
     | SentryTag = 'SearchBar'
     |-Node #7 at (l=43.0, t=210.0, r=1069.0, b=357.0)px
       EditableText = 'query'
       TextSelectionRange = 'TextRange(0, 0)'
       ImeAction = 'Default'
       Focused = 'true'
       SentryTag = 'SearchBar'
       Actions = [GetTextLayoutResult, SetText, InsertTextAtCursor, SetSelection, PerformImeAction, OnClick, OnLongClick, PasteText, RequestFocus, MagnifierPositionInRoot]
       MergeDescendants = 'true'

Test is failing with

java.lang.AssertionError: Failed to assert the following: (Focused = 'true')
Reason: Expected exactly '1' node but could not find any node that satisfies: ((SetText is defined) && (hasParentThat(TestTag = 'searchbar')))
@devPalacio devPalacio added bug Something isn't working Platform: Android labels Jul 18, 2024
@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jul 18, 2024
@devPalacio
Copy link
Author

devPalacio commented Jul 18, 2024 •
edited
Loading

I'm also seeing another test failing that's asserting BottomSheetContent assertIsNotDisplayed. This specific test is being run against Robolectric in case that's relevant. I do not have a workaround for this failure.

Semantics tree on 4.5.1 (Test passes)

D/ChromeViewTest: printToLog:
Printing with useUnmergedTree = 'false'
Node #1 at (l=0.0, t=56.0, r=320.0, b=470.0)px
 |-Node #2 at (l=0.0, t=56.0, r=320.0, b=470.0)px
   IsTraversalGroup = 'true'
   SentryTag = 'ChromeView'
    |-Node #39 at (l=0.0, t=56.0, r=320.0, b=112.0)px
    | SentryTag = 'ChromeView'
    |  |-Node #41 at (l=0.0, t=56.0, r=320.0, b=112.0)px, Tag: 'TopChromeContent'
    |     |-Node #42 at (l=0.0, t=56.0, r=320.0, b=112.0)px
    |       IsTraversalGroup = 'true'
    |       SentryTag = 'NavigationBar'
    |        |-Node #45 at (l=12.0, t=68.0, r=44.0, b=100.0)px
    |        | Role = 'Button'
    |        | Focused = 'false'
    |        | ContentDescription = '[Back]'
    |        | Actions = [OnClick, RequestFocus]
    |        | MergeDescendants = 'true'
    |        |-Node #48 at (l=72.0, t=67.0, r=72.0, b=102.0)px
    |        | Text = '[]'
    |        | Focused = 'false'
    |        | SentryTag = 'TopChromeContent'
    |        | Actions = [SetTextSubstitution, ShowTextSubstitution, ClearTextSubstitution, GetTextLayoutResult, OnClick, RequestFocus]
    |        | MergeDescendants = 'true'
    |        |-Node #50 at (l=276.0, t=68.0, r=308.0, b=100.0)px
    |          Role = 'Button'
    |          Focused = 'false'
    |          ContentDescription = '[More options]'
    |          Actions = [OnClick, RequestFocus]
    |          MergeDescendants = 'true'
    |-Node #5 at (l=0.0, t=386.0, r=320.0, b=682.0)px
      IsTraversalGroup = 'true'
      Actions = [Expand]
       |-Node #7 at (l=0.0, t=386.0, r=320.0, b=682.0)px
         IsTraversalGroup = 'true'
          |-Node #9 at (l=136.0, t=386.0, r=184.0, b=402.0)px
          | SentryTag = 'DragHandle'
          |-Node #10 at (l=0.0, t=402.0, r=320.0, b=462.0)px, Tag: 'BottomChromeContent'
          | IsTraversalGroup = 'true'
          | SentryTag = 'BottomChromeContent'
          |  |-Node #12 at (l=16.0, t=410.0, r=304.0, b=454.0)px
          |    DesignInfoProvider = 'androidx.constraintlayout.compose.Measurer@49e29201'
          |-Node #15 at (l=0.0, t=470.0, r=320.0, b=682.0)px, Tag: 'BottomSheetContent'
            IsTraversalGroup = 'true'
            VerticalScrollAxisRange = 'ScrollAxisRange(value=0.0, maxValue=0.0, reverseScrolling=false)'
            CollectionInfo = 'androidx.compose.ui.semantics.CollectionInfo@46d890b7'
            Actions = [IndexForKey, ScrollBy, ScrollToIndex]
             |-Node #19 at (l=0.0, t=470.0, r=320.0, b=538.0)px, Tag: '2131361949'
             |  |-Node #20 at (l=0.0, t=470.0, r=320.0, b=538.0)px
             |    Focused = 'false'
             |    SentryTag = 'PreviewActionsContent'
             |    ContentDescription = '[Rename]'
             |    Text = '[Rename]'
             |    Actions = [OnClick, RequestFocus, SetTextSubstitution, ShowTextSubstitution, ClearTextSubstitution, GetTextLayoutResult]
             |    MergeDescendants = 'true'
             |-Node #25 at (l=0.0, t=542.0, r=320.0, b=610.0)px, Tag: '2131361926'
             |  |-Node #26 at (l=0.0, t=542.0, r=320.0, b=610.0)px
             |    Focused = 'false'
             |    SentryTag = 'PreviewActionsContent'
             |    ContentDescription = '[Duplicate]'
             |    Text = '[Duplicate]'
             |    Actions = [OnClick, RequestFocus, SetTextSubstitution, ShowTextSubstitution, ClearTextSubstitution, GetTextLayoutResult]
             |    MergeDescendants = 'true'
             |-Node #31 at (l=0.0, t=614.0, r=320.0, b=682.0)px, Tag: '2131361942'
                |-Node #32 at (l=0.0, t=614.0, r=320.0, b=682.0)px
                  Focused = 'false'
                  SentryTag = 'PreviewActionsContent'
                  ContentDescription = '[Move]'
                  Text = '[Move]'
                  Actions = [OnClick, RequestFocus, SetTextSubstitution, ShowTextSubstitution, ClearTextSubstitution, GetTextLayoutResult]
                  MergeDescendants = 'true'

4.10.0 (Test fails)

D/ChromeViewTest: printToLog:
Printing with useUnmergedTree = 'false'
Node #1 at (l=0.0, t=56.0, r=320.0, b=470.0)px
 |-Node #2 at (l=0.0, t=56.0, r=320.0, b=470.0)px
   IsTraversalGroup = 'true'
   SentryTag = 'BottomActionSheet'
    |-Node #37 at (l=0.0, t=56.0, r=320.0, b=470.0)px
    | SentryTag = 'ChromeView'
    |  |-Node #38 at (l=0.0, t=56.0, r=320.0, b=56.0)px
    |  | SentryTag = 'ChromeView'
    |  |-Node #39 at (l=0.0, t=56.0, r=320.0, b=112.0)px
    |  | SentryTag = 'ChromeView'
    |  |  |-Node #41 at (l=0.0, t=56.0, r=320.0, b=112.0)px, Tag: 'TopChromeContent'
    |  |    SentryTag = 'TopChromeContent'
    |  |     |-Node #42 at (l=0.0, t=56.0, r=320.0, b=112.0)px
    |  |       IsTraversalGroup = 'true'
    |  |       SentryTag = 'NavigationBar'
    |  |        |-Node #45 at (l=12.0, t=68.0, r=44.0, b=100.0)px
    |  |        | Role = 'Button'
    |  |        | Focused = 'false'
    |  |        | ContentDescription = '[Back]'
    |  |        | Actions = [OnClick, RequestFocus]
    |  |        | MergeDescendants = 'true'
    |  |        |-Node #48 at (l=72.0, t=67.0, r=72.0, b=102.0)px
    |  |        | Text = '[]'
    |  |        | Focused = 'false'
    |  |        | SentryTag = 'TopChromeContent'
    |  |        | Actions = [SetTextSubstitution, ShowTextSubstitution, ClearTextSubstitution, GetTextLayoutResult, OnClick, RequestFocus]
    |  |        | MergeDescendants = 'true'
    |  |        |-Node #50 at (l=276.0, t=68.0, r=308.0, b=100.0)px
    |  |          Role = 'Button'
    |  |          Focused = 'false'
    |  |          ContentDescription = '[More options]'
    |  |          Actions = [OnClick, RequestFocus]
    |  |          MergeDescendants = 'true'
    |  |-Node #52 at (l=0.0, t=56.0, r=320.0, b=386.0)px
    |    SentryTag = 'Scrim'
    |-Node #5 at (l=0.0, t=386.0, r=320.0, b=682.0)px
      IsTraversalGroup = 'true'
      Actions = [Expand]
       |-Node #7 at (l=0.0, t=386.0, r=320.0, b=682.0)px
         IsTraversalGroup = 'true'
          |-Node #9 at (l=136.0, t=386.0, r=184.0, b=402.0)px
          | SentryTag = 'DragHandle'
          |-Node #10 at (l=0.0, t=402.0, r=320.0, b=462.0)px, Tag: 'BottomChromeContent'
          | IsTraversalGroup = 'true'
          | SentryTag = 'BottomChromeContent'
          |  |-Node #12 at (l=16.0, t=410.0, r=304.0, b=454.0)px
          |    DesignInfoProvider = 'androidx.constraintlayout.compose.Measurer@46d890b7'
          |    SentryTag = 'BottomChromeContent'
          |     |-Node #13 at (l=16.0, t=410.0, r=304.0, b=454.0)px
          |     | SentryTag = 'BottomChromeContent'
          |     |-Node #14 at (l=304.0, t=410.0, r=304.0, b=454.0)px
          |       SentryTag = 'BottomChromeContent'
          |-Node #15 at (l=0.0, t=462.0, r=320.0, b=682.0)px, Tag: 'BottomSheetContent'
            IsTraversalGroup = 'true'
            VerticalScrollAxisRange = 'ScrollAxisRange(value=0.0, maxValue=0.0, reverseScrolling=false)'
            CollectionInfo = 'androidx.compose.ui.semantics.CollectionInfo@6e8c03a7'
            SentryTag = 'ActionItemsColumn'
            Actions = [IndexForKey, ScrollBy, ScrollToIndex]
             |-Node #19 at (l=0.0, t=470.0, r=320.0, b=538.0)px, Tag: '2131361949'
             | SentryTag = '<anonymous>'
             |  |-Node #20 at (l=0.0, t=470.0, r=320.0, b=538.0)px
             |    Focused = 'false'
             |    SentryTag = 'UniversalActionRow'
             |    ContentDescription = '[Rename]'
             |    Text = '[Rename]'
             |    Actions = [OnClick, RequestFocus, SetTextSubstitution, ShowTextSubstitution, ClearTextSubstitution, GetTextLayoutResult]
             |    MergeDescendants = 'true'
             |-Node #25 at (l=0.0, t=542.0, r=320.0, b=610.0)px, Tag: '2131361926'
             | SentryTag = '<anonymous>'
             |  |-Node #26 at (l=0.0, t=542.0, r=320.0, b=610.0)px
             |    Focused = 'false'
             |    SentryTag = 'UniversalActionRow'
             |    ContentDescription = '[Duplicate]'
             |    Text = '[Duplicate]'
             |    Actions = [OnClick, RequestFocus, SetTextSubstitution, ShowTextSubstitution, ClearTextSubstitution, GetTextLayoutResult]
             |    MergeDescendants = 'true'
             |-Node #31 at (l=0.0, t=614.0, r=320.0, b=682.0)px, Tag: '2131361942'
               SentryTag = '<anonymous>'
                |-Node #32 at (l=0.0, t=614.0, r=320.0, b=682.0)px
                  Focused = 'false'
                  SentryTag = 'UniversalActionRow'
                  ContentDescription = '[Move]'
                  Text = '[Move]'
                  Actions = [OnClick, RequestFocus, SetTextSubstitution, ShowTextSubstitution, ClearTextSubstitution, GetTextLayoutResult]
                  MergeDescendants = 'true'

Assert failed: The component is displayed!
java.lang.AssertionError: Assert failed: The component is displayed!

@kahest kahest moved this from Needs Discussion to Needs Investigation in Mobile & Cross Platform SDK Jul 18, 2024
@kahest
Copy link
Member

kahest commented Jul 18, 2024

Hey @devPalacio thanks for the detailed report, much appreciated. We'll take a look and follow up here. cc @markushi

@markushi
Copy link
Member

@devPalacio thanks for the detailed report, sorry to hear it's not working as expected.
The changes were introduced in #716 and #720 . Alongside supporting K2, we also simplified how Modifier.sentryTag() was injected, covering a wider set of Kotlin code paths than before whilst being more performant too - this likely caused a regression.

Just to confirm: Your Composables behave correctly when running on a device, it's "only" your test code which doesn't work as expected, right?

@devPalacio
Copy link
Author

Yea the composables don't seem to have any behavior change in app, just the node matching in tests has changed. I found a workaround for the second failing test I mentioned. I had to switch my matcher from onNodeWithTag to onNodeWithText since the semantic tree changed.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jul 26, 2024
@devPalacio
Copy link
Author

One thing I'm noticing is the tree is not being trimmed as effectively with the newer version of Sentry. Things like zero height spacers seem to be preserved since they now have a SentryTag applied. With this deeper hierarchy, is there any concerns that performance of larger composables will be affected negatively?

@getsantry getsantry bot removed the status in GitHub Issues with 👀 3 Aug 6, 2024
@stefanosiano stefanosiano moved this from Needs Investigation to Backlog in Mobile & Cross Platform SDK Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working Platform: Android
Projects
GitHub Issues with 👀 3
Status: No status
Mobile & Cross Platform SDK
Status: Backlog
Milestone
No milestone
Development

No branches or pull requests
3 participants
@markushi @kahest @devPalacio