Skip to content

Conversation

GHEMID-Mohamed
Copy link
Collaborator

No description provided.

@GHEMID-Mohamed GHEMID-Mohamed requested a review from julien-f March 15, 2019 10:33
src/new.spec.js Outdated
effects: {
async foo() {
this.state.qux = "baz";
this.resetState();
Copy link
Collaborator

Choose a reason for hiding this comment

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

await!

src/new.spec.js Outdated
},
});
return effects.foo();
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Merge with context test.

@GHEMID-Mohamed GHEMID-Mohamed requested a review from julien-f March 15, 2019 10:53
src/new.spec.js Outdated
expect(ownProps(this.effects)).toEqual(["myEffect", "_setState"]);

expect(typeof this.resetState).toBe("function");
this.state.myEntry = "thud";
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is necessary to test the state before resetState!

You can move this after the state tests below.

@GHEMID-Mohamed GHEMID-Mohamed requested a review from julien-f March 15, 2019 12:58
src/new.spec.js Outdated
this.state.myEntry = "baz";
expect(this.state.myEntry).toBe("baz");

expect(typeof this.resetState).toBe("function");
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is unnecessary because you are already calling it.

src/new.spec.js Outdated
expect(this.state.myEntry).toBe("baz");

expect(typeof this.resetState).toBe("function");
this.state.myEntry = "thud";
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the point of this if it has already been changed to baz?

@julien-f julien-f merged commit 2122851 into JsCommunity:next Mar 15, 2019
julien-f pushed a commit that referenced this pull request Mar 21, 2019
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.

2 participants