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

Implement Shallow Testing #2393

Closed
sebmarkbage opened this issue Oct 21, 2014 · 30 comments · Fixed by #2497
Closed

Implement Shallow Testing #2393

sebmarkbage opened this issue Oct 21, 2014 · 30 comments · Fixed by #2497

Comments

@sebmarkbage
Copy link
Collaborator

We need a way to test the output of a single level React component without resolving it all the way down to the bottom layer (whatever that is).

I'm thinking something like this:

var shallowRenderer = ReactTestUtils.createRenderer();
shallowRenderer.render(<SomeComponent />);
shallowRenderer.attachRef('myRefName', someMock);

var result = shallowRenderer.getRenderOutput();
expect(result.type).toBe('div');
expect(result.props.children).toEqual([
  <span className="child1" />,
  <span className="child2" />
]);

shallowRenderer.render(<SomeComponent aNew="Prop" />);

var updatedResult = shallowRenderer.getRenderOutput();
expect(updatedResult.type).toBe('a');

updatedResult.props.onClick(mockEvent);

var updatedResultCausedByClick = shallowRenderer.getRenderOutput();
expect(updatedResult.props.className).toBe('was-clicked');

var instance = shallowRenderer.getMountedInstance();
instance.customFunction();

var updatedResultCausedByCustomFunction = shallowRenderer.getRenderOutput();
expect(updatedResultCausedByCustomFunction.props.className).toBe('custom');

Basically, this fix needs to go through the entire life-cycle of ReactCompositeComponent. It just needs to bail out whenever it would've continued rendering.

graue added a commit that referenced this issue Nov 17, 2014
Basic shallow rendering support (#2393)
@sebmarkbage sebmarkbage reopened this Nov 17, 2014
@sebmarkbage
Copy link
Collaborator Author

Reopened so that we can track the remaining features (refs etc.)

@graue
Copy link
Contributor

graue commented Nov 17, 2014

Weird, I thought GitHub only closed issues if you used language like "fixes #NNNN", not if you simply mentioned the issue at all. Sorry about that.

@sophiebits
Copy link
Collaborator

The PR text included "WIP to fix #2393."

@graue
Copy link
Contributor

graue commented Nov 25, 2014

Is the rest (refs support) blocked by #1373 or can more work be done now? @sebmarkbage

@gaearon
Copy link
Collaborator

gaearon commented Apr 27, 2015

Any reason this doesn't support setState other than that it's not implemented yet?
I was hoping to do something like

var shallowRenderer = ReactTestUtils.createRenderer();
var instance = shallowRenderer.render(<SomeComponent />);

var result = shallowRenderer.getRenderOutput();
expect(result.type).toBe('div');

instance.handleClick(); // will call setState inside

result = shallowRenderer.getRenderOutput();
expect(result.type).toBe('span'); // imagine it returns <span> with different state

@graue
Copy link
Contributor

graue commented Apr 27, 2015

@gaearon, that's supposed to work. Can you post a minimal repro?

@gaearon
Copy link
Collaborator

gaearon commented Apr 27, 2015

@graue How do you do this? render does not return an instance.

@gaearon
Copy link
Collaborator

gaearon commented Apr 27, 2015

It works if I use renderer._instance._instance so I think renderer could just return that from render.
(I also have to shim document because React expects it to be defined for some reason.)

@necolas
Copy link
Contributor

necolas commented May 9, 2015

Finding this quite useful. But it has been a bit inconvenient to have null among the children when an element is conditionally rendered.

@sebmarkbage
Copy link
Collaborator Author

The idea was to make the React.Children.forEach/map helpers ignore null which could help with that. Perhaps we could have React.Children(element.props.children) return an iterable which traverses nested arrays and ignores null.

@sophiebits
Copy link
Collaborator

@sebmarkbage Really? Allowing nulls was a change intentionally introduced a long time ago. In #3650 I have an implementation of toArray.

@sebmarkbage
Copy link
Collaborator Author

Allowing empty children (null, undefined, boolean) was a controversial topic, where we eventually landed on null/boolean/undefined being passed through. That way you could explicitly treat an empty slot as something special. This landed very early on before we really started seeing pattern developing.

However, IMO, we've seen that this leads to inconsistent implementations where null slots will break and is not possible to use. It is unclear whether a set of children allows nullable slots, and if they do, it is unclear what that means. For example, in a grid layout component, does an empty slot mean that there should be an empty column in the grid, or that it should be skipped?

Therefore, a lot of our components end up with explicit null checks. Meaning boilerplate, and inconsistent behavior when they're forgotten. Therefore, I think the default should be to filter out nulls which is consistent with the behavior of the built-ins components.

It is still possible to get access to the raw data if you have a different type of children in mind where empty slots is meaningful.

@davidgilbertson
Copy link
Contributor

How should this work with the TestUtils functions. For example, this fails:

shallowRenderer.render(<Section />);
reactModule = shallowRenderer.getRenderOutput();

let mySidebar = reactModule.props.children[1].props.children[0];
expect(mySidebar.props.className).to.equal('sidebar'); // Yep

let mySidebars = TestUtils.scryRenderedDOMComponentsWithClass(reactModule, 'sidebar'); // Also tried reactModule._instance...
expect(mySidebars.length).to.equal(1); // Nope

I can't see any other info on getMountedInstance() mentioned at the start of this issue, is this what I'm missing?

My end goal is simply to test that if I pass in 10 articles then 10 components get rendered. Any help would be awesome.

@graue
Copy link
Contributor

graue commented May 16, 2015

Hi David, the scry and find functions are meant to be used with the output of renderIntoDocument, not shallow rendering. React doesn't currently provide a helper that will traverse ReactElement trees for you. Your options for now are to describe the structure of the tree explicitly as in lines 4-5 of your example, or write your own helper module to find descendants you care about.

getMountedInstance() hasn't been implemented yet. Refer to the docs section on test utilities for what's actually in React.

@davidgilbertson
Copy link
Contributor

Cool, thanks for the reply @graue. I've returned to just using renderIntoDocument since the components I'm testing (and their children) don't have state/stores so don't really need mocking out anyway.
One last (maybe stupid) question, what does scry mean?

@trevordmiller
Copy link

So, I am trying to use Shallow Rendering and have it mostly working...the only thing left I can’t figure out is the recommended way to mock events and how to get children with events to pass. I've attempted to use the method directly from the import in the test (<a onClick={Accordion.toggle}>). This seems dirty…is there a way to just say <a onClick={Function}> or something? I could post this on Stack Overflow, but I have had very little luck finding good information about Shallow Rendering online. If anyone has experience with shallow rendering and can give me some help you will be my hero...

Source code:
https://github.com/trevordmiller/shallow-rendering-example/tree/master/src/components/Accordion

screen shot 2015-09-01 at 10 15 32 pm

screen shot 2015-09-01 at 10 22 20 pm

@mcmire
Copy link

mcmire commented Sep 9, 2015

If I have an issue with shallow rendering, should I create a new issue, or post it here?

@sophiebits
Copy link
Collaborator

New issue please.

@glenjamin
Copy link
Contributor

@trevordmiller you might find that https://github.com/glenjamin/skin-deep helps in providing higher-level assertions for the sort of thing you're doing there.

In that particular case, I would avoid asserting on the event handler of the anchor in that test - but a different test would want to call props.onClick() to test the handler.

@graue
Copy link
Contributor

graue commented Sep 23, 2015

@davidgilbertson Super late reply, but "scry" is a fancy word for "find". I'm not sure why we don't just use "find". Maybe to avoid confusion between the plural and singular forms, whose names would otherwise differ only by one "s".

@sebmarkbage
Copy link
Collaborator Author

scry is because of legacy internal FB APIs. No reason.

@johnnyreilly
Copy link

Like @trevordmiller I'm keenly on the lookout for a way to correctly unit test event handling in React code. Is there any documentation that anyone could point me to? I've looked at https://github.com/glenjamin/skin-deep but it didn't seem to have something that scratches my particular itch.

Here's my scenario (cut down):

    const shallowRenderer = TestUtils.createRenderer();
    shallowRenderer.render(<Problem alert={ alerts[0] } />);

    const problem = shallowRenderer.getRenderOutput();
    expect(problem.type).toBe('div');
    expect(problem.props.className).toBe('col-md-2');
    expect(problem.props.children).toEqualAsObject(
      <div className={ 'summary panel panel-danger danger' } onClick={ null }>
      </div>

Where Problem looks like this:

class Problem extends React.Component {
  constructor(props) {
    super(props);
    this._onClick = this._onClick.bind(this);
  }

  render() {
    const { alert } = this.props;
    const cssClass = cellStateToCssClass(alert.cellState);
    return (
      <div className="col-md-2">
        <div className={ 'summary panel panel-' + cssClass + ' ' + cssClass } onClick={ this._onClick }>
      </div>
    );
  }

  _onClick() {
    this.context.router.transitionTo(this.props.alert.routeName, this.props.alert.routeParams);
  }
}

I'd like to be able to assert that onClick is wired up for my component but I'm at a loss as to how to do that....

@glenjamin
Copy link
Contributor

What you actually want is to assert that clicking interacts with the router.

Pass in a fake context with a router mock (try Simon) then call div.props.onClick()

Apologies for the brevity, on a phone.

@johnnyreilly
Copy link

Hi @glenjamin,

Thanks for responding. I'm not too clear on how to mock successfully when it comes to context. Digging through the source of ReactTestUtils I came up with this:

  it('onClick triggers transitionTo - work in progress', () => {
    const { alerts } = stubSummaryStore.getState_with_CompanyConnectionStore_errors_then_warnings();
    const [ alert ] = alerts;
    const shallowRenderer = TestUtils.createRenderer();
    const transitionToSpy = jasmine.createSpy('transitionTo');
    const mockContext = {
      router: { transitionTo: transitionToSpy }
    };
    shallowRenderer.render(<Problem alert={ alerts[0] } />, mockContext);

    const problem = shallowRenderer.getRenderOutput();
    const onClick = problem.props.children.props.onClick;
    onClick();
    expect(transitionToSpy).toHaveBeenCalledWith(alert.routeName, alert.routeParams);
  });

However this dies a death with:

× 1 test failed

FAILED TESTS:
  summary.Problem
    × onClick triggers transitionTo - work in progress
      PhantomJS 1.9.8 (Windows 8 0.0.0)
    TypeError: 'undefined' is not an object (evaluating 'this.context.router.transitionTo')
        at _onClick (C:/Users/johnr/AppData/Local/Temp/64efa2ad5770dae8b59608cc4646f32a.browserify:38280 <- src/components/summary/Problem.js:9:0)
        at E:/Source/Aggregation/Main/Trayport.Aggregation.WebAdmin/node_modules/karma-phantomjs-shim/shim.js:32
        at :1
        at C:/Users/johnr/AppData/Local/Temp/64efa2ad5770dae8b59608cc4646f32a.browserify:40773 <- E:/Source/Aggregation/Main/Trayport.Aggregation.WebAdmin/test/components/summary/Problem.tests.js:44:4

Which makes me think I'm getting the context mocking wrong. Do you know how you successfully mock context with shallowRenderer? I'm using Jasmine (as you probably guessed!) I have a feeling I'm close...

@glenjamin
Copy link
Contributor

Are you using React 0.13? You're probably hitting the difference between parent & owner context.

Skin deep has a wrapper that helps with this - there's a context example the test suite, but the short answer is to wrap your render in another function call and let skin deep set up the context.

This problem should go away in 0.14

@johnnyreilly
Copy link

I am using React 0.13, yes. If I port to 0.14 this issue should resolve? I might give that a crack if that's the case (it is in RC after all)

@johnnyreilly
Copy link

Problems with the upgrade. I have raised an issue. Will have to return to this later I think.

@vvo
Copy link

vvo commented Oct 22, 2015

I have written https://github.com/algolia/react-element-to-jsx-string and https://github.com/algolia/expect-jsx as companion tools when testing with shallow rendering.

It gives you the ability to diff on JSX strings rendered instead of React element objects.

@vvo
Copy link

vvo commented Oct 22, 2015

Hi, I was wondering what was the state of:

  • simulating events when using shallow rendering
  • using refs when using shallow rendering

How can we help make this happen if that's still planned?

@gaearon
Copy link
Collaborator

gaearon commented Oct 2, 2017

This issue is pretty stale so I’ll close it. If there are specific features you miss in the shallow renderer please create new issues with proposals on how they should work. Note that we added a new test renderer that is much more feature-complete.

@gaearon gaearon closed this as completed Oct 2, 2017
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 a pull request may close this issue.