search

getsentry / sentry-android-gradle-plugin

Gradle plugin for Sentry Android. Upload proguard, debug files, and more.
https://docs.sentry.io/platforms/android/gradle/
MIT License
141 stars 32 forks source link

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

Open devPalacio opened 1 month ago

devPalacio commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

markushi commented 1 month ago

@devPalacio thanks for the detailed report, sorry to hear it's not working as expected. The changes were introduced in https://github.com/getsentry/sentry-android-gradle-plugin/pull/716 and https://github.com/getsentry/sentry-android-gradle-plugin/pull/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 commented 1 month ago

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.

devPalacio commented 1 month ago

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?