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

Migration guide from @testing-library/react-native is missing some breaking changes #477

Closed
TAGraves opened this issue Aug 2, 2020 · 13 comments
Labels
bug Something isn't working compat: testing-library

Comments

@TAGraves
Copy link
Collaborator

TAGraves commented Aug 2, 2020

Hey all,

Stoked to see the two react-native testing libraries have finally merged! At Root, we have a fair number of testing library tests in our react native project. I tried upgrading @testing-library/react-native from v6 to v7 and about 20% of our tests are failing after following the upgrade guide.

Another engineer will be taking a closer look at the upgrade (I was just trying to get an idea of the effort involved), but looking through the errors I see at least the following undocumented breaking changes:

getByLabelText(...).getProp is not a function (see https://www.native-testing-library.com/docs/next/api-test-instance#getprop) -- this one is obviously replaced with .props instead

baseElement is undefined (see https://www.native-testing-library.com/docs/next/api-main#baseelement)

parentNode is undefined (see https://www.native-testing-library.com/docs/api-test-instance#parentnode) -- this one is really important for us as we use it a lot for testing styles in cases where we can guarantee what the "native" parent will be. From a quick check I'm not sure if there is a new equivalent (.parent behaves differently since there's no concept of a NativeTestInstance now)

string precision options are missing (see https://www.native-testing-library.com/docs/next/api-queries#precision)

If there are existing ways to migrate from these old patterns to the new ones, we'd love to hear it and see it added to the migration guide. If not -- we'd likely be happy to contribute to add some of this functionality back in!

@TAGraves TAGraves added the question Further information is requested label Aug 2, 2020
@thymikee
Copy link
Member

thymikee commented Aug 2, 2020

Hey! I think we can definitely add some of these back. I'm not so sure about parentNode because it smells like testing implementation details.

I'll be away from keyboard till next week, but I'm sure you can discuss it with @mdjastrzebski @Esemesek and @cross19xx and figure out the way forward 🙂

@TAGraves
Copy link
Collaborator Author

TAGraves commented Aug 3, 2020

I'm not so sure about parentNode because it smells like testing implementation details.

Yes, I agree. I think we can probably get around it in our codebase by adding a matcher that recursively checks parents for the appropriate styles.

@manuelmhtr
Copy link

Also: asJSON does not exist. It need to be renamed to toJSON.

@artdent
Copy link

artdent commented Aug 13, 2020

Also the getNodeText helper function, https://github.com/testing-library/native-testing-library/blob/master/src/lib/get-node-text.js, which could be replaced by a thin wrapper around the internal getChildrenAsText helper.

@thymikee
Copy link
Member

Hey @TAGraves would you be so kind and create some action points / issues for us to implement? For example baseElement sounds like something that should be fairly easy to start with. Then we could add string precision API, because why not. The more compat with Testing Library, the better. I hope it's understandable that we started from a different place, but we have all the good intentions :)

I also noticed that you were a collaborator to the second library, so if you'd like to gain more privileges in this repo, please give me a shout at Discord (thymikee#1279) :)

@thymikee thymikee added bug Something isn't working compat: testing-library and removed question Further information is requested labels Aug 17, 2020
@mdjastrzebski
Copy link
Member

re parentNode: I've added "Ability to traverse host component tree easily (discussion)" #520.

@dephiros
Copy link

+1 for baseElement/Container. We use that at our company as a way to get at top level component which is useful in some cases where you don't have any specific text/content to query for

@dephiros
Copy link

dephiros commented Sep 25, 2020

Another behavior that I noticed while doing migration is:

  • Before the query would return NativeTestInstance and it now return ReactTestInstance. This is already documented in the migration doc
  • However, this does break existing tests in our codebase because our tests expect to query on the same element seeing in debug() which would have type View, Text... whereas ReactTestInstance's type the React component itself. This seems to go against the philosophy of not testing the implementation.
  • The use case for us is to query the output style on a component

Appreciate direction if I am not using the new lib correctly

@mdjastrzebski
Copy link
Member

@dephiros Please post sample test code of how are you using debug().

AFAIK debug is intended to output to console. Did you actually mean toJSON()?

@thymikee
Copy link
Member

I think we could introduce something similar to NativeTestInstance, will need to think about it.

@dephiros
Copy link

Thank you for your response
@mdjastrzebski , I meant debug
This is a simplify example of the test that used to pass pre-v7. To get this test pass in the new version, we had to modify the test to check the prop of child(which is now an instance of ChildrenComponent instead of view. To me, checking style would follow closer to the not testing implementation details philosophy

const ChildComponent = ({color, text}) => {
  return <View style={{color}}></View>
}

const TestComponent = ({items}) => {
  return (
    <View testID='container'>
      {items.map((item) => (<ChildComponent key={item.color} {...item} />))}
    </View>
  );
}

fdescribe('Test', () => {
  const items = [
    {color: 'red'},
    {color: 'blue'},
    {color: 'yellow'},
  ];
  it('should output the right color', () => {
    const view = render(<TestComponent items={items} />);
    view.debug();
    const container = view.getByTestId('container');
    const children = container.children;
    children.forEach((child, index) => {
      expect(child).toHaveStyle(items[index]);
    });
  })
});

view.debug() (very similar to the previous version except the previous version has the extra wrapper)

    <View
      testID="container"
    >
      <View
        style={
          Object {
            "color": "red",
          }
        }
      />
      <View
        style={
          Object {
            "color": "blue",
          }
        }
      />
      <View
        style={
          Object {
            "color": "yellow",
          }
        }
      />
    </View>

@mikeduminy
Copy link
Contributor

mikeduminy commented Oct 12, 2020

Here is an attempt to track all the functionality that IMO we should try to get in for v8

Missing functionality in v7 that was present in v6:

Bugs in v7:

Misc:

  • add Guiding Principles to docs
  • mocks for TouchableOpacity and TouchableHighlight (not present in react-native preset, affecting snapshots)

What we can probably ignore until we have further information:

  • NativeTestInstance (parity with v6): eg. getProp, parentNode

@mdjastrzebski
Copy link
Member

Closing as stale.

In case anyone involved considers any of the above mentioned points as a bug or missing feature, please fill a separate pull request for each item.

@mdjastrzebski mdjastrzebski closed this as not planned Won't fix, can't repro, duplicate, stale Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compat: testing-library
Projects
None yet
Development

No branches or pull requests

7 participants