Please add instructions how to test decorated methods

[saidkholov]

Example 1:

@track(props => {
  return { page: props.isNew ? 'new' : 'existing' };
})

The only way to test this decorator is to extract the callback function.

Example 2: There is no way to jest.spyOn() on the decorated method or override a decorated method.

[saidkholov]

looks like a setter is missing in makeClassMemberDecorator.js

[tizmagik]

I haven’t had a use case for testing these methods as they’re mostly a simple function of props. Happy to explore a PR if you have an approach to make these more testable.

[robmojosix]

@tizmagik A use case might be to test that the method called track with data? How would we test that?

[robmojosix]

Also, if we can’t stub the decorators then won’t react-tracking get called on every test assertion that uses it? Ideally I don’t want this to be called as part of my component tests.

[tizmagik]

Ah yea, so for the tests we actually just mock out react-tracking. If you're using Jest, you can define a shared mock in __mocks__/react-tracking.js that's just a pass-through decorator, like so:

/* __mocks__/react-tracking.js */

module.exports = () => c => c;

If you wanted to assert that react-tracking is called with a particular object (or, at all) you could do something like:

// UNTESTED, but you get the idea...

const reactTrackingMock = jest.fn(c => c);

jest.setMock('react-tracking', reactTrackingMock);

it('calls track', () => {
  mount(<MyComponent />)
  expect(reactTrackingMock).toHaveBeenCalled();
});

---

To your point, this should be something we go over in the docs. If the above works for you, and you're up for it, a PR would be great!

[saidkholov]

Yes, this might work. I’ll test the idea and do a PR to update the docs.

[robmojosix]

@saidkholov @tizmagik in the end my team used another tracking provider to mock the dispatch call. This was simple to do plus you don't mock out the entire library.

I've created a PR if this is something that works for everyone: https://github.com/NYTimes/react-tracking/pull/114

[saidkholov]

The PR looks great, thanks @robmojosix . I'll give this approach a try. On the other hand, to test decorated methods within the class I ended up passing a mocked tracking object to a component like this:

const trackEvent = jest.fn();
<Component 
  tracking={{
    trackEvent
  }}
/>
expect(trackEvent).toHaveBeenCalledWith(args)

Works wether decorators are used or tracking is dispatched via props.

P.S. If one is testing a fully wrapped component(track()(Component)), they can verify all the arguments passed to trackEvent including the contextual data that belongs to the component.

[saidkholov]

The only problem that I'm still having is coverage within function passed to a decorator. Istanbul seems to ignore it. Looks like it is an issue for istanbul repository.

 @track(({ date }) => ({
    action: "action",
    label: "label",
    date
}))

Exporting the function and testing it separately doesn't look good. Please let me know, if you know how to overcome it:)

[tizmagik]

Thanks very much for the PR @robmojosix -- I'll take a look!

@saidkholov I like the approach of passing in a tracking prop directly. If that works for your use case, that might be worth documenting as a PR. As for the istanbul coverage issue, I haven't looked into this myself, but found this issue which suggests giving babel-plugin-istanbul a try. Maybe worth a shot? If you do try it out, please post back with what you find. 🙏

[robmojosix]

@saidkholov if you can assert that your mock was called with args doesn’t this cover your decorated methods?

Checking it was called means the decorator fired and checking the args means you are testing the logic of the decorator.

[saidkholov]

Sorry my bad, I didn't check coverage stats after I mocked tracking object the way I mentioned. It is looking good now. Created a PR https://github.com/NYTimes/react-tracking/pull/115

[robmojosix]

Cool, @saidkholov I've just looked over the PR :) This is a really simple and quick way to test tracking 👍

As you mentioned before; I'm not sure this will cover your decorated class methods - these won't call your stub.

Another thought is that this will bypass react-tracking's use of the context API which will only matter if you need to test accumulative tracking over multiple components. However, you might reserve this for feature or more e2e style tests.

Lastly, this pattern means user's code has react-tracking's props mapped within every file we need to test. If react-tracking change their API we have to change every test file where this pattern is used.

These are only minor points. If this pattern works for you and your team you should definitely use it.

[tizmagik]

All good points @robmojosix -- thank you very much for your input. And thanks @saidketchman for the PR.

I'm hesitating about #114 because I don't want to increase the surface of react-tracking, especially since this can be implemented in userland relatively easily. And I think #115 is a good start but @robmojosix raised some good points there.

I think the right solution may be some combination of these two approaches. So if we take Jest as the assumed test framework, maybe there's a recipe there for how to mock out react-tracking when a particular test doesn't care to assert about its tracking concerns, but then also a provide a clean way to assert on tracking concerns a la #114 in the case where the test does want to.

I'm kind of swamped with things at work right now but hope to get to this soon. In the mean time, if you all have any other ideas or approaches, by all means please chime in or add to your PRs.

Thanks again for your thoughts here everyone, I really appreciate it!

[robmojosix]

Completely agree @tizmagik I'll close my PR. I think @saidkholov 's docs-only PR is better placed to be tweaked into what we want :)

[saidketchman]

@tizmagik getting credit for something I didn't do hahaha. Wrong Said :P

[saidkholov]

Thanks for your feedback, everyone. Totally agree with @robmojosix. Good notes.

[carterthayer]

How do I test my decorated functions? I have react-tracking mocked as per @tizmagik's comment:

/* __mocks__/react-tracking.js */

module.exports = () => c => c;

But I have been getting TypeError: dive.instance(. . .).handleClick is not a function when trying to call the decorated function in my test

Example:

@track({ component: "Sample" })
class Sample extends Component {
    // most of the component left out

    @track((_, state) => ({action: "handleClick", state: state})))
    handleClick = () => {
       // do something
    };
}

it("should do a thing", () => {
    const wrapper = shallow(
      <Sample  />,
      {
        disableLifecycleMethods: true
      }
    );
    const dive = wrapper.dive();

    // TypeError
    dive.instance().clear();
});

Works perfectly fine with the @track removed from the handleClick function

[tizmagik]

Ah, this is because that simplified mock (module.exports = () => c > c) won't actually work for decorated class methods (only for decorating the class itself or wrapping function components).

Here's a slightly more sophisticated mock that should work for what you're trying to do @carterthayer

const PropTypes = require('prop-types');

const mockTrackEvent = jest.fn();

module.exports.TrackingPropType = PropTypes.shape();

module.exports = () => (...clsOrMethod) => {
  if (clsOrMethod.length === 1) {
    // decorating a class
    const [cls] = clsOrMethod;
    cls.defaultProps = {
      ...cls.defaultProps,
      tracking: {
        trackEvent: mockTrackEvent,
      },
    };
    return cls;
  }

  // decorating a method
  return clsOrMethod[1].initializer;
};

module.exports.mockTrackEvent = mockTrackEvent;

I've been meaning to provide this as importable within react-tracking via something like: import track, { mockTrackEvent, TrackingPropType } from 'react-tracking/mock'; but haven't had a chance yet. PRs welcome! 😁

[tizmagik]

Closing this issue in favor of #134