Skip to content

feat(reaclette): Computed tests continuation#33

Merged
julien-f merged 6 commits intoJsCommunity:nextfrom
GHEMID-Mohamed:next-computed-tests-next-2
Mar 14, 2019
Merged

feat(reaclette): Computed tests continuation#33
julien-f merged 6 commits intoJsCommunity:nextfrom
GHEMID-Mohamed:next-computed-tests-next-2

Conversation

@GHEMID-Mohamed
Copy link
Collaborator

No description provided.

@GHEMID-Mohamed GHEMID-Mohamed requested a review from julien-f March 13, 2019 13:40
@GHEMID-Mohamed GHEMID-Mohamed requested a review from julien-f March 14, 2019 08:13
src/new.spec.js Outdated
effects: {
changeState() {
this.state.corge = 8;
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not use the _setState effects provided for the tests?

src/new.spec.js Outdated
});

it("are not called when its state/props dependencies do not change", async () => {
const baz = jest.fn(({ qux }, { bar }) => bar * qux);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should call it sum and it's dependencies a and b (or i/j or n/m), it will make the test either to read/understand.

await effects.changeState();
expect(getState().baz).toBe(11);
expect(baz.mock.calls.length).toBe(2);
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

This test should be almost exactly the same than the previous one except that we are changing:

  1. its state dependency
  2. its props dependency

@GHEMID-Mohamed GHEMID-Mohamed requested a review from julien-f March 14, 2019 12:24
@julien-f julien-f merged commit 5399269 into JsCommunity:next Mar 14, 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