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

Add tests for Modal component #171

Merged
merged 5 commits into from
Nov 13, 2017
Merged

Add tests for Modal component #171

merged 5 commits into from
Nov 13, 2017

Conversation

s4nji
Copy link
Contributor

@s4nji s4nji commented Nov 10, 2017

Adds tests for Modal component

@s4nji s4nji requested a review from ivanzusko November 10, 2017 15:25
@ivanzusko ivanzusko mentioned this pull request Nov 10, 2017
3 tasks
@ivanzusko
Copy link
Contributor

Thanx man! I’ll check a bit later today 😉

<div className={b('inner')}>
<div className={b('title')}>
<div className={b('inner')()}>
<div className={b('title')()}>
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@ivanzusko
Copy link
Contributor

ivanzusko commented Nov 10, 2017

@s4nji looks solid 👍
Senior @carlosyslas , un minuto de tu tiempo, por favor 😉 . What do you think?


import Modal from '../../../components/Modal/Modal';

configure({ adapter: new Adapter() });
Copy link
Contributor

Choose a reason for hiding this comment

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

This configure statement should be inside of a setup file like this one:
https://github.com/hellofresh/my-deliveries-fragment/blob/master/__tests__/setup.js

Also, take a look at this file:

https://github.com/hellofresh/my-deliveries-fragment/blob/master/.jestrc#L2-L5

It contains the instructions to provide a setup file.

};
const runCommonTest = (wrapper, props) => {
const tree = toJSON(wrapper);
const modaliz = tree.children[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

const [ modaliz ] = tree.children;

configure({ adapter: new Adapter() });

describe('Modal component', () => {
const render = props => mount(<Modal {...props} />);
Copy link
Contributor

Choose a reason for hiding this comment

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

You can probably pass the requiredProps directly in this render function, so you don't have to the following over and over again:

        const props = {
             ...requiredProps,
             title: 'Modal test title',
         };

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We will also need to pass it again in runCommonTest if we pass it on the render function

I think redefining the props on every test makes it easier to debug since you don't need to search around to find the contents of props 🙂

@carlosyslas
Copy link
Contributor

+1 Really well done!

@s4nji s4nji merged commit c1b6205 into master Nov 13, 2017
@carlosyslas carlosyslas deleted the feature/add-modal-tests branch November 13, 2017 15:45
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.

None yet

3 participants