Closed brunoscopelliti closed 7 years ago
Hrm... I'm not super keen on adding setState directly, but how does your component set it's loading state in the first place? It has to have it's own way to become loading anyway, couldn't you just simulate that?
Hey @gnarf Thanks for your reply!
I'm not super keen on adding setState directly
Could you please elaborate a bit more on your reason?
how does your component set it's loading state in the first place?
The snippet I posted is simple on purpose; loading: true
is set in the onClick
handler...
I excluded the code that sets it back to false
because it doesn't help to make my point.
couldn't you just simulate that?
I don't know how to do it without having a setState
method exposed on the sut component... but any suggestion is welcome.
Could you please elaborate a bit more on your reason?
It seems not very "unity" to have the test setting state directly, you should test things the way the component is designed to work. I.E. in your example (sorry I missed the onClick arrow fn)
class Button extends Component {
constructor(props) {
super(props);
this.state = {};
}
render() {
if (this.state.loading){
return <span>Loading ...</span>;
}
return <button onClick={() => this.setState({ loading: true })}></button>;
}
}
it('shows loading after clicking button', () => {
const context = shallow(<Button />);
context.find('button').simulate('click');
expect(context.output()).toEqual(<span>Loading ...</span>);
});
Hey @gnarf, Thanks for the feedback... I see things in a slightly different way; let me change just a bit the code of my example to try to make my point:
class Button extends Component {
constructor(props) {
super(props);
this.state = {};
}
doStuff() {
// do some stuff, and eventually...
this.setState({ loading: true });
}
render() {
if (this.state.loading){
return <span>Loading ...</span>;
}
return <button onClick={() => this.doStuff()}></button>;
}
}
... you should test things the way the component is designed to work.
So how would describe "how a such component does work"? This is my take on it:
doStuff
is executed. I don't care about doStuff
at the moment.Let's write the tests. First two props are straightforward to check:
it('1', () => {
const context = shallow(<Button />);
expect(context.output()).toEqual(<button></button>);
// Not sure this is the right way to check the ouput... but you should have get the idea.
});
it('2', () => {
const doStuffStub = sinon.stub(Button.prototype, 'doStuff');
const context = shallow(<Button />);
context.find('button').simulate('click');
doStuffStub.shouild.have.been.colledOnce;
});
Now for the third I've to write:
it('3A', () => {
const context = shallow(<Button />);
context.find('button').simulate('click');
expect(context.output()).toEqual(<span>Loading ...</span>);
});
I don't see this approach more unity than the following:
it('3B', () => {
const context = shallow(<Button />);
context.setState({ loading: true });
expect(context.output()).toEqual(<span>Loading ...</span>);
});
IMHO the latter has also the following benefits:
doStuff
that might be a very slow function.Any thoughts?
Hrm... I can see that argument, I rarely use much component state, so haven't run into this before...
@mzgoddard do you think we can give access to the component instances to get & set the state?
In the meantime while I play around with a way to expose an API for this, perhaps this would help you feel less dirty about it.
import {h, rerender} from 'preact';
it('3', () => {
let ref;
const context = shallow(<Button ref={inst => ref = inst} />);
ref.setState({loading: true});
// need to import this from preact if you want to test "sync" state changes like this
rerender();
expect(context.output()).toEqual(<span>Loading ...</span>);
});
I have a PR up that adds setState()
, state()
and component()
methods to the FindWrapper
/ RenderContext
in #43 if you'd like to take a look and tell me if this feels good enough, here are the tests I added for the features:
(these run with name/func of both shallow and deep)
for (const [name, func] of Object.entries({ shallow, deep })) {
it(`${name}: can retrieve component instance`, () => {
const context = func(<ClickCount />);
expect(context.component()).toBeInstanceOf(ClickCount);
});
it(`${name}: can retrieve and set component state`, () => {
const context = func(<ClickCount />);
expect(context.state()).toEqual({ count: 0 });
expect(context.state('count')).toEqual(0);
context.setState({ count: 2 });
expect(context.text()).toEqual('2');
});
}
Also, I'm really sorry that this issue sat for a month before I noticed it, I must've missed the github notification while I was away for a bit!
No problem for the delay, I was away for a bit too... Just for the record, as workaround I was using the same as you suggested in https://github.com/mzgoddard/preact-render-spy/issues/39#issuecomment-328903423.
v1.1.0 has this now!
Let's say I've a component like this:
How can I test that when
loading
state prop istrue
the loading message is displayed? It would be super useful to have something like enzyme's setState on the component. What do you think about this?