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.

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

feat: Render element tree in query error messages #1378

Merged
merged 24 commits into from
Apr 27, 2023

Conversation

stevehanson
Copy link
Contributor

@stevehanson stevehanson commented Mar 31, 2023

Summary

Closes #1375.

This commit updates the error message to the ByText query so that it outputs the virtual DOM, which can help the developer pinpoint the error without having to rerun the test with a debug() statement.

Update: this PR now outputs the element tree for all query errors, not just ByText.

Before

Unable to find an element with text: /HELLOOO/

After

Unable to find an element with text: /HELLOOO/

<Text>
  My component
</Text>

Screenshot

Here is an example of an error when an element does not match because it is hidden (in this case, accessibilityElementsHidden: true). The relevant prop is included. The virtual DOM also is formatted with syntax highlighting:

screenshot of test output with syntax highlighting of the virtual DOM

Preserving visibility-related props

By default, the byText query only matches visible elements, so any props that hide an element are relevant to the test failure. For that reason, this PR preserves any props that hide an element. I followed the logic from isSubtreeInaccessible to determine how to do this:

  • Preserve: accessibilityElementsHidden=true
    • does not preserve prop if false
  • Preserve: accessibilityViewIsModal=true
    • does not preserve prop if false
  • Preserve: importantForAccessibility="no-hide-descendants"
    • does not preserve prop if has other value
  • Preserve: style: { display: 'none' }
    • flattens the styles first, does not preserve any other styles

Test plan

I've added unit tests for the mapVisibilityRelatedProps function as well as snapshot tests for the output of getByText with all of the above conditions. The snapshots are a little ugly since the formatted virtual DOM includes special characters for the syntax highlighting.

  • I did not test performance. Since this only runs when the test errors, it seems like any performance penalty would be worth the cost, but let me know if you'd like me to benchmark this.
  • I also did not test this on a large virtual DOM tree to see what it would look like. Let me know if that is desired, and I can include the output

@thymikee
Copy link
Member

thymikee commented Apr 1, 2023

Love it! I think it would be helpful to also show "testID" prop

@stevehanson
Copy link
Contributor Author

Thanks for taking a look, @thymikee! I'm happy to include testID if we think that is useful. I'm curious what criteria we should use to determine which props to include. I was planning to open PRs for the other queries, and my thinking was to only include props that are directly relevant to the failures, since that line seemed like one that users would intuitively understand. So the byTestID query would include testId, byLabelText would include accessibilityLabel and accessibilityLabelledBy, etc. Are there some props, like testId, that you think should always be included when we render the virtual DOM?

@thymikee
Copy link
Member

thymikee commented Apr 3, 2023

@stevehanson sounds great. Whichever helps debuggability and doesn't clutter the output is nice. Ultimately we can expose mapping functionality to the users, so they can configure it per use case or test run. Nevertheless, good defaults are going to be crucial for this functionality to be net-helpful :)

@mdjastrzebski
Copy link
Member

@stevehanson I've support the general idea around this PR. It seems helpful to report element tree to the user on query failure. That being said, the unfiltered element tree can be quite long, especially when all props are included, and some are especially noisy (e.g. style, default touch event handlers, etc). The approach you have taken so far, which allowing only accessibility related ones seems generally good direction. However, I think we should include a little bit more.

  • all a11y props you added so far
  • testID
  • nativeID
  • accessibilityLabel
  • accessbilityLabeledBy
  • accessibilityRole
  • accessbilityHint
  • accessibilityState
  • accessibilityValue
  • placeholder
  • value
  • defaultValue

The reason for including these, is that a large tree without additional context might be hard to navigate, it will just be a deeply nested tree of Views with occasionals Text, TextInputs, RNSafeArea, etc. Adding the above props, if present, should at a lot of semantic context that would allow user to navigate the element tree better. That would be regardless of the query used, i.e. we should display testIDs also for *ByText queries, etc.

@mdjastrzebski
Copy link
Member

mdjastrzebski commented Apr 4, 2023

Other things to watch out for:

  • getByText is invoked multiple times inside findByText. Triggering tree formatting on every failure seem unnecessary and costly, as it's normal that some of the initial getByText invokations will be failed. We should explore possibility to generate element on onTimeout event for findBy* as well as keeping in working for getBy* on first failure. This could be achieved by having internalGetBy (not printing element tree) and exported getBy (priniting element tree).
  • Write tests for all query types (getBy, getAllBy, queryBy, ...) to make sure all work as intended.
  • We might think about some smarter error messages IFF the query failed but the element is hidden rather that not present at all. That would be simple to determine, as we could simply run the query second time this time with includeHiddenElements: true

src/queries/text.ts Outdated Show resolved Hide resolved
@stevehanson
Copy link
Contributor Author

stevehanson commented Apr 4, 2023

@mdjastrzebski thank you for the review!

However, I think we should include a little bit more.

I totally agree with this suggestion, and am mostly aligned with the props you suggested. I wired up the mapProps from this PR in a real project yesterday and found what you mentioned -- since most of the elements end up being <View />, stripping too many props can make it hard to make sense of the DOM.

One open question on my mind is if every query should render the same props. accessibilityState and accessibilityValue, for example, add 82 extra lines to the output in my example below but would never affect a byText query passing or failing, unless I'm mistaken. I don't feel like they will aid much in understanding the output, either, since there should be a corresponding accessibilityRole, label, or text, that makes those elements understandable, without needing to see the specific state or value.

It does seem like we would need those props for other queries, like maybe byRole or byLabel, in addition to maybe some other props, like focusable, collapsable, accessible. I'm just trying to think through how the output can be kept as small as possible while including everything relevant or helpful. I'm definitely open to including them for now if you feel strongly about this but wanted to bring this up for discussion. accessibilityState and accessibilityValue are probably the only ones I personally feel on the fence about for byText.

Here are some sample DOM trees from a small real app that uses React Navigation with multiple stacks, including bottom tabs. Just including this as a relevant data point to this discussion:

Debug output: all props except style (571 lines)
<RNCSafeAreaView
  edges={
    Array [
      "bottom",
      "left",
      "right",
      "top",
    ]
  }
>
  <RNCSafeAreaProvider
    onInsetsChange={[Function anonymous]}
  >
    <RNSScreenStack>
      <RNSScreen
        collapsable={false}
        forwardedRef={[Function handleRef]}
        gestureResponseDistance={
          Object {
            "bottom": -1,
            "end": -1,
            "start": -1,
            "top": -1,
          }
        }
        nativeBackButtonDismissalEnabled={false}
        onAppear={[Function onAppear]}
        onDisappear={[Function onDisappear]}
        onDismissed={[Function onDismissed]}
        onHeaderBackButtonClicked={[Function onHeaderBackButtonClicked]}
        onNativeDismissCancelled={[Function onNativeDismissCancelled]}
        onTransitionProgress={[Function anonymous]}
        onWillDisappear={[Function onWillDisappear]}
        replaceAnimation="push"
        stackPresentation="push"
        swipeDirection="horizontal"
      >
        <RNSScreenStackHeaderConfig
          backButtonInCustomView={false}
          backgroundColor="rgb(255, 255, 255)"
          color="#111"
          direction="ltr"
          disableBackButtonMenu={false}
          hidden={true}
          hideBackButton={false}
          hideShadow={true}
          largeTitleHideShadow={false}
          title="Tabs"
          titleColor="#282828"
          topInsetEnabled={false}
          translucent={false}
        />
        <View
          accessibilityElementsHidden={false}
          importantForAccessibility="auto"
        >
          <View>
            <View>
              <RNSScreenNavigationContainer>
                <RNSScreen
                  activityState={2}
                  collapsable={false}
                  forwardedRef={[Function handleRef]}
                  gestureResponseDistance={
                    Object {
                      "bottom": -1,
                      "end": -1,
                      "start": -1,
                      "top": -1,
                    }
                  }
                >
                  <View
                    accessibilityElementsHidden={false}
                    importantForAccessibility="auto"
                  >
                    <View>
                      <View>
                        <RNSScreenStack>
                          <RNSScreen
                            collapsable={false}
                            forwardedRef={[Function handleRef]}
                            gestureResponseDistance={
                              Object {
                                "bottom": -1,
                                "end": -1,
                                "start": -1,
                                "top": -1,
                              }
                            }
                            nativeBackButtonDismissalEnabled={false}
                            onAppear={[Function onAppear]}
                            onDisappear={[Function onDisappear]}
                            onDismissed={[Function onDismissed]}
                            onHeaderBackButtonClicked={[Function onHeaderBackButtonClicked]}
                            onNativeDismissCancelled={[Function onNativeDismissCancelled]}
                            onTransitionProgress={[Function anonymous]}
                            onWillDisappear={[Function onWillDisappear]}
                            replaceAnimation="push"
                            stackPresentation="push"
                            swipeDirection="horizontal"
                          >
                            <RNSScreenStackHeaderConfig
                              backButtonInCustomView={false}
                              backgroundColor="rgb(255, 255, 255)"
                              color="#111"
                              direction="ltr"
                              disableBackButtonMenu={false}
                              hidden={true}
                              hideBackButton={false}
                              largeTitleHideShadow={false}
                              title="Home"
                              titleColor="#282828"
                              topInsetEnabled={false}
                              translucent={false}
                            />
                            <View
                              accessibilityElementsHidden={true}
                              importantForAccessibility="no-hide-descendants"
                            >
                              <View>
                                <RCTScrollView
                                  bounces={true}
                                  contentContainerStyle={
                                    Array [
                                      Object {
                                        "flexGrow": 1,
                                        "paddingVertical": 20,
                                      },
                                      Object {
                                        "paddingHorizontal": 20,
                                      },
                                      undefined,
                                    ]
                                  }
                                  contentInsetAdjustmentBehavior="automatic"
                                  keyboardShouldPersistTaps="handled"
                                  showsVerticalScrollIndicator={false}
                                  testID="HomeScreen"
                                >
                                  <View>
                                    <Text
                                      accessibilityRole="header"
                                    >
                                      Good Morning, 
                                      <Text>
                                        Dale
                                      </Text>
                                    </Text>
                                    <Text>
                                      Welcome to the Example app. Content will be coming soon!
                                    </Text>
                                    <View
                                      accessibilityRole="link"
                                      accessibilityState={
                                        Object {
                                          "busy": undefined,
                                          "checked": undefined,
                                          "disabled": undefined,
                                          "expanded": undefined,
                                          "selected": undefined,
                                        }
                                      }
                                      accessibilityValue={
                                        Object {
                                          "max": undefined,
                                          "min": undefined,
                                          "now": undefined,
                                          "text": undefined,
                                        }
                                      }
                                      accessible={true}
                                      collapsable={false}
                                      focusable={true}
                                      onBlur={[Function onBlur]}
                                      onClick={[Function onClick]}
                                      onFocus={[Function onFocus]}
                                      onResponderGrant={[Function onResponderGrant]}
                                      onResponderMove={[Function onResponderMove]}
                                      onResponderRelease={[Function onResponderRelease]}
                                      onResponderTerminate={[Function onResponderTerminate]}
                                      onResponderTerminationRequest={[Function onResponderTerminationRequest]}
                                      onStartShouldSetResponder={[Function onStartShouldSetResponder]}
                                    >
                                      <Text>
                                        Go to article!
                                      </Text>
                                    </View>
                                    <View>
                                      <Text>
                                        Some text
                                      </Text>
                                    </View>
                                  </View>
                                </RCTScrollView>
                              </View>
                            </View>
                          </RNSScreen>
                          <RNSScreen
                            collapsable={false}
                            forwardedRef={[Function handleRef]}
                            gestureResponseDistance={
                              Object {
                                "bottom": -1,
                                "end": -1,
                                "start": -1,
                                "top": -1,
                              }
                            }
                            nativeBackButtonDismissalEnabled={false}
                            onAppear={[Function onAppear]}
                            onDisappear={[Function onDisappear]}
                            onDismissed={[Function onDismissed]}
                            onHeaderBackButtonClicked={[Function onHeaderBackButtonClicked]}
                            onNativeDismissCancelled={[Function onNativeDismissCancelled]}
                            onTransitionProgress={[Function anonymous]}
                            onWillDisappear={[Function onWillDisappear]}
                            replaceAnimation="push"
                            stackPresentation="push"
                            swipeDirection="horizontal"
                          >
                            <RNSScreenStackHeaderConfig
                              backButtonInCustomView={false}
                              backgroundColor="rgb(255, 255, 255)"
                              color="#111"
                              direction="ltr"
                              disableBackButtonMenu={false}
                              hidden={false}
                              hideBackButton={false}
                              largeTitleHideShadow={false}
                              title=""
                              titleColor="#282828"
                              topInsetEnabled={false}
                              translucent={false}
                            />
                            <View
                              accessibilityElementsHidden={false}
                              importantForAccessibility="auto"
                            >
                              <View>
                                <RCTScrollView
                                  bounces={true}
                                  contentContainerStyle={
                                    Array [
                                      Object {
                                        "flexGrow": 1,
                                        "paddingVertical": 20,
                                      },
                                      Object {
                                        "paddingHorizontal": 20,
                                      },
                                      undefined,
                                    ]
                                  }
                                  contentInsetAdjustmentBehavior="automatic"
                                  keyboardShouldPersistTaps="handled"
                                  showsVerticalScrollIndicator={false}
                                  testID="ArticleScreen"
                                >
                                  <View>
                                    <Text
                                      accessibilityRole="header"
                                    >
                                      Article Heading
                                    </Text>
                                    <Text>
                                      Some text
                                    </Text>
                                  </View>
                                </RCTScrollView>
                              </View>
                            </View>
                          </RNSScreen>
                        </RNSScreenStack>
                      </View>
                    </View>
                  </View>
                </RNSScreen>
              </RNSScreenNavigationContainer>
              <View
                collapsable={false}
                onLayout={[Function handleLayout]}
                pointerEvents="auto"
              >
                <View
                  pointerEvents="none"
                />
                <View
                  accessibilityRole="tablist"
                >
                  <View
                    accessibilityRole="button"
                    accessibilityState={
                      Object {
                        "busy": undefined,
                        "checked": undefined,
                        "disabled": undefined,
                        "expanded": undefined,
                        "selected": true,
                      }
                    }
                    accessibilityStates={
                      Array [
                        "selected",
                      ]
                    }
                    accessibilityValue={
                      Object {
                        "max": undefined,
                        "min": undefined,
                        "now": undefined,
                        "text": undefined,
                      }
                    }
                    accessible={true}
                    collapsable={false}
                    focusable={true}
                    onBlur={[Function onBlur]}
                    onClick={[Function onClick]}
                    onFocus={[Function onFocus]}
                    onResponderGrant={[Function onResponderGrant]}
                    onResponderMove={[Function onResponderMove]}
                    onResponderRelease={[Function onResponderRelease]}
                    onResponderTerminate={[Function onResponderTerminate]}
                    onResponderTerminationRequest={[Function onResponderTerminationRequest]}
                    onStartShouldSetResponder={[Function onStartShouldSetResponder]}
                  >
                    <View>
                      <View>
                        <View
                          accessible={false}
                        >
                          <Image
                            accessibilityIgnoresInvertColors={false}
                            source={
                              Object {
                                "process": [Function process],
                              }
                            }
                          />
                        </View>
                      </View>
                      <View>
                        <View
                          accessible={false}
                        >
                          <Image
                            accessibilityIgnoresInvertColors={false}
                            source={
                              Object {
                                "process": [Function process],
                              }
                            }
                          />
                        </View>
                      </View>
                    </View>
                    <Text>
                      Home
                    </Text>
                  </View>
                  <View
                    accessibilityRole="button"
                    accessibilityState={
                      Object {
                        "busy": undefined,
                        "checked": undefined,
                        "disabled": undefined,
                        "expanded": undefined,
                        "selected": false,
                      }
                    }
                    accessibilityStates={Array []}
                    accessibilityValue={
                      Object {
                        "max": undefined,
                        "min": undefined,
                        "now": undefined,
                        "text": undefined,
                      }
                    }
                    accessible={true}
                    collapsable={false}
                    focusable={true}
                    onBlur={[Function onBlur]}
                    onClick={[Function onClick]}
                    onFocus={[Function onFocus]}
                    onResponderGrant={[Function onResponderGrant]}
                    onResponderMove={[Function onResponderMove]}
                    onResponderRelease={[Function onResponderRelease]}
                    onResponderTerminate={[Function onResponderTerminate]}
                    onResponderTerminationRequest={[Function onResponderTerminationRequest]}
                    onStartShouldSetResponder={[Function onStartShouldSetResponder]}
                  >
                    <View>
                      <View>
                        <View
                          accessible={false}
                        >
                          <Image
                            accessibilityIgnoresInvertColors={false}
                            source={
                              Object {
                                "process": [Function process],
                              }
                            }
                          />
                        </View>
                      </View>
                      <View>
                        <View
                          accessible={false}
                        >
                          <Image
                            accessibilityIgnoresInvertColors={false}
                            source={
                              Object {
                                "process": [Function process],
                              }
                            }
                          />
                        </View>
                      </View>
                    </View>
                    <Text>
                      Search
                    </Text>
                  </View>
                  <View
                    accessibilityRole="button"
                    accessibilityState={
                      Object {
                        "busy": undefined,
                        "checked": undefined,
                        "disabled": undefined,
                        "expanded": undefined,
                        "selected": false,
                      }
                    }
                    accessibilityStates={Array []}
                    accessibilityValue={
                      Object {
                        "max": undefined,
                        "min": undefined,
                        "now": undefined,
                        "text": undefined,
                      }
                    }
                    accessible={true}
                    collapsable={false}
                    focusable={true}
                    onBlur={[Function onBlur]}
                    onClick={[Function onClick]}
                    onFocus={[Function onFocus]}
                    onResponderGrant={[Function onResponderGrant]}
                    onResponderMove={[Function onResponderMove]}
                    onResponderRelease={[Function onResponderRelease]}
                    onResponderTerminate={[Function onResponderTerminate]}
                    onResponderTerminationRequest={[Function onResponderTerminationRequest]}
                    onStartShouldSetResponder={[Function onStartShouldSetResponder]}
                  >
                    <View>
                      <View>
                        <View
                          accessible={false}
                        >
                          <Image
                            accessibilityIgnoresInvertColors={false}
                            source={
                              Object {
                                "process": [Function process],
                              }
                            }
                          />
                        </View>
                      </View>
                      <View>
                        <View
                          accessible={false}
                        >
                          <Image
                            accessibilityIgnoresInvertColors={false}
                            source={
                              Object {
                                "process": [Function process],
                              }
                            }
                          />
                        </View>
                      </View>
                    </View>
                    <Text>
                      Discover
                    </Text>
                  </View>
                  <View
                    accessibilityRole="button"
                    accessibilityState={
                      Object {
                        "busy": undefined,
                        "checked": undefined,
                        "disabled": undefined,
                        "expanded": undefined,
                        "selected": false,
                      }
                    }
                    accessibilityStates={Array []}
                    accessibilityValue={
                      Object {
                        "max": undefined,
                        "min": undefined,
                        "now": undefined,
                        "text": undefined,
                      }
                    }
                    accessible={true}
                    collapsable={false}
                    focusable={true}
                    onBlur={[Function onBlur]}
                    onClick={[Function onClick]}
                    onFocus={[Function onFocus]}
                    onResponderGrant={[Function onResponderGrant]}
                    onResponderMove={[Function onResponderMove]}
                    onResponderRelease={[Function onResponderRelease]}
                    onResponderTerminate={[Function onResponderTerminate]}
                    onResponderTerminationRequest={[Function onResponderTerminationRequest]}
                    onStartShouldSetResponder={[Function onStartShouldSetResponder]}
                  >
                    <View>
                      <View>
                        <View
                          accessible={false}
                        >
                          <Image
                            accessibilityIgnoresInvertColors={false}
                            source={
                              Object {
                                "process": [Function process],
                              }
                            }
                          />
                        </View>
                      </View>
                      <View>
                        <View
                          accessible={false}
                        >
                          <Image
                            accessibilityIgnoresInvertColors={false}
                            source={
                              Object {
                                "process": [Function process],
                              }
                            }
                          />
                        </View>
                      </View>
                    </View>
                    <Text>
                      Profile
                    </Text>
                  </View>
                </View>
              </View>
            </View>
          </View>
        </View>
      </RNSScreen>
    </RNSScreenStack>
  </RNCSafeAreaProvider>
</RNCSafeAreaView>
Debug output: visibility props, testID, some accessibility-related (173 lines)

This output is a lot easier to parse. It includes most of the props listed in the PR review, except acessibilityState, accessibilityValue,placeholder, value, defaultValue, nativeId.

<RNCSafeAreaView>
  <RNCSafeAreaProvider>
    <RNSScreenStack>
      <RNSScreen>
        <RNSScreenStackHeaderConfig />
        <View
          accessibilityElementsHidden={false}
          importantForAccessibility="auto"
        >
          <View>
            <View>
              <RNSScreenNavigationContainer>
                <RNSScreen>
                  <View>
                    <View>
                      <View>
                        <RNSScreenStack>
                          <RNSScreen>
                            <RNSScreenStackHeaderConfig />
                            <View
                              accessibilityElementsHidden={true}
                              importantForAccessibility="no-hide-descendants"
                            >
                              <View>
                                <RCTScrollView
                                  testID="HomeScreen"
                                >
                                  <View>
                                    <Text
                                      accessibilityRole="header"
                                    >
                                      Good Morning, 
                                      <Text>
                                        Dale
                                      </Text>
                                    </Text>
                                    <Text>
                                      Welcome to the Example app. Content will be coming soon!
                                    </Text>
                                    <View
                                      accessibilityRole="link"
                                    >
                                      <Text>
                                        Go to article!
                                      </Text>
                                    </View>
                                    <View>
                                      <Text>
                                        Some text
                                      </Text>
                                    </View>
                                  </View>
                                </RCTScrollView>
                              </View>
                            </View>
                          </RNSScreen>
                          <RNSScreen>
                            <RNSScreenStackHeaderConfig />
                            <View>
                              <View>
                                <RCTScrollView
                                  testID="ArticleScreen"
                                >
                                  <View>
                                    <Text
                                      accessibilityRole="header"
                                    >
                                      Article Heading
                                    </Text>
                                    <Text>
                                      Some text
                                    </Text>
                                  </View>
                                </RCTScrollView>
                              </View>
                            </View>
                          </RNSScreen>
                        </RNSScreenStack>
                      </View>
                    </View>
                  </View>
                </RNSScreen>
              </RNSScreenNavigationContainer>
              <View>
                <View />
                <View
                  accessibilityRole="tablist"
                >
                  <View
                    accessibilityRole="button"
                  >
                    <View>
                      <View>
                        <View>
                          <Image />
                        </View>
                      </View>
                      <View>
                        <View>
                          <Image />
                        </View>
                      </View>
                    </View>
                    <Text>
                      Home
                    </Text>
                  </View>
                  <View
                    accessibilityRole="button"
                  >
                    <View>
                      <View>
                        <View>
                          <Image />
                        </View>
                      </View>
                      <View>
                        <View>
                          <Image />
                        </View>
                      </View>
                    </View>
                    <Text>
                      Search
                    </Text>
                  </View>
                  <View
                    accessibilityRole="button"
                  >
                    <View>
                      <View>
                        <View>
                          <Image />
                        </View>
                      </View>
                      <View>
                        <View>
                          <Image />
                        </View>
                      </View>
                    </View>
                    <Text>
                      Discover
                    </Text>
                  </View>
                  <View
                    accessibilityRole="button"
                  >
                    <View>
                      <View>
                        <View>
                          <Image />
                        </View>
                      </View>
                      <View>
                        <View>
                          <Image />
                        </View>
                      </View>
                    </View>
                    <Text>
                      Profile
                    </Text>
                  </View>
                </View>
              </View>
            </View>
          </View>
        </View>
      </RNSScreen>
    </RNSScreenStack>
  </RNCSafeAreaProvider>
</RNCSafeAreaView>

With no mapProps function, the output was 1272 lines long. VS Code truncated this output, though I was able to see the full output when running the test in my terminal. I didn't include the output here.

@mdjastrzebski
Copy link
Member

re. accessibilityState, accessibilityValue: in case of these we could filter out undefined props/empty objects.

So a part from your example:

accessibilityState={
  Object {
    "busy": undefined,
    "checked": undefined,
    "disabled": undefined,
    "expanded": undefined,
    "selected": false,
  }
}
accessibilityValue={
  Object {
    "max": undefined,
    "min": undefined,
    "now": undefined,
    "text": undefined,
  }
}

Could become

// all `undefined props has been skipped
accessibilityState={
  Object {
    "selected": false,
  }
}
// accessibilityValue={Object { }} not to be displayed because of all undefined props`

@mdjastrzebski
Copy link
Member

Re above two examples: the first one seems too verbose, there are many things added by RN itself, like lots of event handlers on Pressables, empty values for accessibilityStates and accessibilityValues, etc,

On the other hand the filtered our example looked much more lean, but missined some useful props, like accessibilityState={{ selected: true }}, title, etc.

IMO we should strive for practical balance here, adding elements that might help user reference the output views to his own components. We can tweak the list later on, as changes to the exact error output should not be considered a breaking change.

@stevehanson
Copy link
Contributor Author

@mdjastrzebski your proposal of filtering out undefined object values and not printing those props altogether if all values are undefined seems like a huge improvement. I'll go ahead and move forward with updating this PR with the comments from your review, including what we just discussed. Thanks again!

@stevehanson
Copy link
Contributor Author

@mdjastrzebski I believe I have implemented all of your suggestions, so this should be ready for re-review. Thank you again for all of your guidance on this PR 🙂

onTimeout(error);
const result = onTimeout(error);
if (result) {
error = result;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could probably use extra eyes on this change. When a custom onTimeout was passed to waitFor, we did not previously allow for this to alter the returned error. This change makes it so the error returned by the custom onTimeout is the error that is ultimately reported.

This change was needed for our findBy optimizations to only print the DOM when the timeout is reached.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also just sharing the DOM Testing Library implementation here for comparison.

It looks like that library has a default onTimeout that takes the error and appends the DOM to the message in a similar way. The difference being that in DOM Testing Library, if a custom onTimeout is specified in waitFor, then that custom onTimeout would be passed the error without the DOM in the message, and the DOM would never be appended to the output.

My implementation adds the DOM output to the error before passing it to the custom onTimeout. It seems like the best approach is likely to update this PR to more closely match the DOM Testing Library approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in 2a9fb5c. The behavior should now match DOM Testing Library where if a custom onTimeout is passed, that function receives the error without the element tree. The element tree is only added in the default onTimeout if no custom one is passed.

@mdjastrzebski
Copy link
Member

@stevehanson looks promising, as the PR is quite large I will need to spend some time reviewing it. So far I've reviewed makeQueries.ts and refactored slightly your code.

I wonder if we could find way to avoid these weird numbers popping in the error messages:

�[36m<Text�[39m

@mdjastrzebski
Copy link
Member

I've managed to remove console color control codes from the error output.

@stevehanson
Copy link
Contributor Author

@mdjastrzebski it seems the recent commit removes syntax highlighting not just for the tests but for the actual output. I'm not sure if that was an intended side effect, but I find the syntax highlighting to be quite helpful when parsing large DOM trees.

Before After
colors-before colors-after

@mdjastrzebski
Copy link
Member

mdjastrzebski commented Apr 5, 2023

@stevehanson That change was intended but you are right, colors might help users analyse the errors. The reason it bothered me was that all error snapshots contained color control characters.

I've thing I've found a good solution here. The coloring of output will depend on process.env.COLORS !== false, this is based on RTLs approach. By default the element tree output will be colored, but for our own tests we will set process.env.COLORS = false (in jest-setup.ts) in order to generate clean snapshots.

@stevehanson hope that works for both of us :-)

@stevehanson stevehanson changed the title feat: Render virtual DOM in byText error message feat: Render element tree in query error messages Apr 5, 2023
@stevehanson
Copy link
Contributor Author

I just wanted to provide an update that I was out of the office last week but am back now and plan to help get this completed this week.

The main task I am aware of that is left is to update the onTimeout logic to more closely match DOM Testing Library (comment), but I will also look over this again to see what else needs to be done and am open to any other feedback.

@stevehanson
Copy link
Contributor Author

stevehanson commented Apr 19, 2023

UPDATE: I found that the reason the element tree is not printing is because when we modify the message in onTimeout to add the element tree, we are not also modifying the message that is printed on the first line of the stacktrace. It turns out that is what ends up ultimately being printed. Adding the following line in onTimeout works so we now see the element tree in test failures: error.stack = error.stack?.replace(oldMessage, error.message);. Or we can use the copyStackTrace utility that already exists in the codebase. We likely also want to update our findBy tests to specifically look at the stacktrace if that is where the error the user sees comes from. This however does not resolve the issue with the stacktrace including the findBy frame, which is also an issue on main.


I'm noticing a couple interesting things. If I add and run the following failing test:

test('sample', async () => {
  const { findByText } = render(<View />);
  await findByText(/foo/);
});

I notice two issues: the output does not include the element tree like expected, and the wrong frame is highlighted in the stack trace:

Screenshot 2023-04-18 at 11 06 35 PM

If I catch the error in the test and call console.error(error.message), the element tree is indeed included, so it just is not printing for some reason:

output of console.error, shows element tree

I think this is why our tests for findBy are passing, even though the actual output, which is what we ultimately care about, is incorrect. The element tree was being printed prior to this commit (even on our latest commit, if you remove our custom onTimeout and go back to printing the DOM every time the query runs in findBy, then the element tree shows properly).

The other issue I pointed out is that the stacktrace highlights the frame from makeQueries instead of the frame from the test failure (i.e. findBy in makeQueries is not removed from the stacktrace). This same issue is also present in main. I'm not sure if this affects any builds.

Potential solution

I dug around the waitFor code a bit and see that it creates an error prior to any async logic because that apparently causes some issues with the stacktrace. I tried doing something similar in findByQuery in makeQueries, and it does fix the issue, but I don't yet fully understand why. I will explore this more to see if there is a simpler solution and to get a better grasp of this but wanted to surface this in case any more regular contributors have some initial thoughts.

Here's the solution that worked for me:

  function findByQuery(instance: ReactTestInstance) {
    let error: Error; // instantiate the error that will ultimately be returned from the timeout

    return function findFn(
      predicate: Predicate,
      queryOptions?: Options & WaitForOptions,
      {
        onTimeout = (e: unknown) => {
          const timeoutError = e as Error;
          if (timeoutError.message) {
            // take just the message from the timeout error and return our error instead
            // we don't care about the timeoutError stacktrace since we called waitFor from here.
            error.message = formatErrorMessage(timeoutError.message, true);
          }

          return error;
        },
        ...waitForOptions
      }: WaitForOptions = {}
    ) {
      error = new ErrorWithStack('FIND_BY_ERROR', findFn); // initialize the error here, removing `findFn` from the trace
      const deprecatedWaitForOptions =
        extractDeprecatedWaitForOptions(queryOptions);

      // append formatted DOM to final error

      return waitFor(
        () =>
          getByQuery(instance, { printElementTree: true })(
            predicate,
            queryOptions
          ),
        {
          ...deprecatedWaitForOptions,
          ...waitForOptions,
          onTimeout,
        }
      );
    };
  }

Here is the now-correct failure output with the above code:

correct test failure output for a findBy query. Shows element tree and stacktrace

Other things that did not work

I also tried some other approaches, but nothing else I tried worked. The first thing I tried was manually removing findBy from the stacktrace of the error that we catch in our onTimeout:

onTimeout = (e: unknown) => {
  const error = e as Error;

  if (error.message) {
    error.message = formatErrorMessage(error.message, true);
  }

  // remove findFn from stacktrace
  if (Error.captureStackTrace) {
    Error.captureStackTrace(error, findFn);
  }

  return error;
}

This had a weird effect. The element tree now prints, but the text is muted. Additionally, no stack trace is shown.

output with element tree muted and not stacktrace

I can push up the solution I shared above but wanted to first leave this comment and gather feedback while I also continue to explore this, because I feel like there might be a better solution.

@stevehanson
Copy link
Contributor Author

I just had a realization and prepended an update to my comment above with more info.

@mdjastrzebski
Copy link
Member

mdjastrzebski commented Apr 20, 2023

@stevehanson I think your solution looks promising. The output looks good. Apply this as a code change.

@stevehanson
Copy link
Contributor Author

I think your solution looks promising. The output looks good. Apply this as a code change.

@mdjastrzebski great! I can push up the code from my previous comment if that is what you are referring to here, but I wanted to make sure you also saw my commit from yesterday (2a9fb5c). That commit is overall a simpler solution with less potential for regression -- when a timeout occurs, we just append the element tree to the message and also replace the original message on error.stack. I verified that this does fix the error with the DOM not being output when a failure occurs, however it does not fix the error I was seeing with the findBy frame still being present in the trace. Since that issue is also present in main, it seems preferable to fix it in a separate PR and keep this one focused on printing the element tree.

I did try findBy in a real project using the latest RN Testing Library and see that even though the findFn frame is still present, my application code is what gets highlighted, so it seems like a smaller issue than what I was seeing when running tests from this codebase. I wonder if Jest or Node has some functionality to not highlight frames from external dependencies or something like that:

findBy waitFor
image Screenshot 2023-04-20 at 4 38 45 PM

@stevehanson
Copy link
Contributor Author

@mdjastrzebski I just wanted to check in to see if there is anything else needed from me to push this PR forward. It was unclear to me from the most recent comment which solution you preferred I go with to resolve the findBy* element tree issue. The currently pushed up solution seems to me to be the simplest approach that resolves the issue, but I'm happy to consider alternatives. Thanks!

Copy link
Member

@mdjastrzebski mdjastrzebski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 Looks awesome!

@stevehanson thank you kindly for submitting this PR and all the hard work into polishing it. Our users and I really appreciate it.

@codecov
Copy link

codecov bot commented Apr 27, 2023

Codecov Report

Patch coverage: 98.97% and project coverage change: +0.17 🎉

Comparison is base (5f770cb) 96.14% compared to head (f5f64d5) 96.31%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1378      +/-   ##
==========================================
+ Coverage   96.14%   96.31%   +0.17%     
==========================================
  Files          50       51       +1     
  Lines        3317     3475     +158     
  Branches      492      519      +27     
==========================================
+ Hits         3189     3347     +158     
  Misses        128      128              
Impacted Files Coverage Δ
src/waitFor.ts 77.29% <83.33%> (+0.82%) ⬆️
src/helpers/format-default.ts 100.00% <100.00%> (ø)
src/helpers/format.ts 100.00% <100.00%> (ø)
src/helpers/host-component-names.tsx 100.00% <100.00%> (ø)
src/queries/makeQueries.ts 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@mdjastrzebski mdjastrzebski merged commit f7c8400 into callstack:main Apr 27, 2023
9 checks passed
@mdjastrzebski
Copy link
Member

This PR has been released in v12.1.0 🚀

hyochan pushed a commit to dooboolab-community/react-native-iap that referenced this pull request Jun 22, 2023
….2 (#2445)

[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[@testing-library/react-native](https://callstack.github.io/react-native-testing-library)
([source](https://togithub.com/callstack/react-native-testing-library))
| [`12.0.1` ->
`12.1.2`](https://renovatebot.com/diffs/npm/@testing-library%2freact-native/12.0.1/12.1.2)
|
[![age](https://badges.renovateapi.com/packages/npm/@testing-library%2freact-native/12.1.2/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/@testing-library%2freact-native/12.1.2/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/@testing-library%2freact-native/12.1.2/compatibility-slim/12.0.1)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/@testing-library%2freact-native/12.1.2/confidence-slim/12.0.1)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>callstack/react-native-testing-library</summary>

###
[`v12.1.2`](https://togithub.com/callstack/react-native-testing-library/releases/tag/v12.1.2)

[Compare
Source](https://togithub.com/callstack/react-native-testing-library/compare/v12.1.1...v12.1.2)

#### What's Changed

#### Fixes

- fix: pointer events evaluation by
[@&#8203;mdjastrzebski](https://togithub.com/mdjastrzebski) in
[callstack/react-native-testing-library#1395
- fix: non-editable wrapped TextInput events by
[@&#8203;TMaszko](https://togithub.com/TMaszko) in
[callstack/react-native-testing-library#1385
- fix: support `onXxx` event name for TextInput event checks by
[@&#8203;mdjastrzebski](https://togithub.com/mdjastrzebski) in
[callstack/react-native-testing-library#1404

#### Docs, Chores, etc

- docs: add config example for pnpm by
[@&#8203;yjose](https://togithub.com/yjose) in
[callstack/react-native-testing-library#1400
- chore: move/remove deprecation functions by
[@&#8203;mdjastrzebski](https://togithub.com/mdjastrzebski) in
[callstack/react-native-testing-library#1402
- refactor: component tree dead code by
[@&#8203;mdjastrzebski](https://togithub.com/mdjastrzebski) in
[callstack/react-native-testing-library#1403
- refactor: `fireEvent` cleanup by
[@&#8203;mdjastrzebski](https://togithub.com/mdjastrzebski) in
[callstack/react-native-testing-library#1401

#### New Contributors

- [@&#8203;yjose](https://togithub.com/yjose) made their first
contribution in
[callstack/react-native-testing-library#1400
👏
- [@&#8203;TMaszko](https://togithub.com/TMaszko) made their first
contribution in
[callstack/react-native-testing-library#1385
👏

**Full Changelog**:
callstack/react-native-testing-library@v12.1.1...v12.1.2

###
[`v12.1.1`](https://togithub.com/callstack/react-native-testing-library/releases/tag/v12.1.1)

[Compare
Source](https://togithub.com/callstack/react-native-testing-library/compare/v12.1.0...v12.1.1)

#### What's Changed

#### Fixes

- fix: remove incorrect dependencies by
[@&#8203;mdjastrzebski](https://togithub.com/mdjastrzebski) in
[callstack/react-native-testing-library#1399

**Full Changelog**:
callstack/react-native-testing-library@v12.1.0...v12.1.1

###
[`v12.1.0`](https://togithub.com/callstack/react-native-testing-library/releases/tag/v12.1.0)

[Compare
Source](https://togithub.com/callstack/react-native-testing-library/compare/v12.0.1...v12.1.0)

#### What's Changed

##### Improvements

- feat: Render element tree in query error messages by
[@&#8203;stevehanson](https://togithub.com/stevehanson) in
[callstack/react-native-testing-library#1378

##### Bugfixes

- Proper stack trace for findBy\* and findAllBy\* queries by
[@&#8203;mdjastrzebski](https://togithub.com/mdjastrzebski) in
[callstack/react-native-testing-library#1394

#### New Contributors

- [@&#8203;stevehanson](https://togithub.com/stevehanson) made their
first contributions in
[#&#8203;1377](https://togithub.com/callstack/react-native-testing-library/issues/1377),
[#&#8203;1378](https://togithub.com/callstack/react-native-testing-library/issues/1378)
and
[#&#8203;1390](https://togithub.com/callstack/react-native-testing-library/issues/1390)
👏

##### Chores, docs, deps, etc

- Fix broken link in contributing.md by
[@&#8203;stevehanson](https://togithub.com/stevehanson) in
[callstack/react-native-testing-library#1377
- chore: update deps 2023-04-04 by
[@&#8203;mdjastrzebski](https://togithub.com/mdjastrzebski) in
[callstack/react-native-testing-library#1380
- Fix typo in "derived" in v12 migration guide by
[@&#8203;CodingItWrong](https://togithub.com/CodingItWrong) in
[callstack/react-native-testing-library#1376
- chore: fix migration guide role prop naming by
[@&#8203;mdjastrzebski](https://togithub.com/mdjastrzebski) in
[callstack/react-native-testing-library#1382
- fix: "Edit this Page" link in docs results in 404 by
[@&#8203;stevehanson](https://togithub.com/stevehanson) in
[callstack/react-native-testing-library#1390
- refactor: remove stale tests by
[@&#8203;mdjastrzebski](https://togithub.com/mdjastrzebski) in
[callstack/react-native-testing-library#1392
- chore: experiments app by
[@&#8203;mdjastrzebski](https://togithub.com/mdjastrzebski) in
[callstack/react-native-testing-library#1391

**Full Changelog**:
callstack/react-native-testing-library@v12.0.1...v12.1.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/dooboolab-community/react-native-iap).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS45OC40IiwidXBkYXRlZEluVmVyIjoiMzUuMTMxLjAiLCJ0YXJnZXRCcmFuY2giOiJtYWluIn0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@stevehanson stevehanson deleted the text-error-message branch July 14, 2023 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Output is not helpful when ByText query fails
3 participants