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

Improve testing #144

Merged
merged 2 commits into from
Dec 6, 2016
Merged

Improve testing #144

merged 2 commits into from
Dec 6, 2016

Conversation

yesmeck
Copy link
Member

@yesmeck yesmeck commented Dec 2, 2016

覆盖率提高到了 96%。

另外这个 PR 依赖 react-component/trigger#33

-------------------|----------|----------|----------|----------|----------------|
File               |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
-------------------|----------|----------|----------|----------|----------------|
All files          |    96.06 |    87.68 |    97.44 |    96.04 |                |
 DropdownMenu.jsx  |      100 |      100 |      100 |      100 |                |
 FilterMixin.js    |      100 |    91.11 |      100 |      100 |                |
 OptGroup.jsx      |      100 |      100 |      100 |      100 |                |
 Option.jsx        |      100 |      100 |      100 |      100 |                |
 Select.jsx        |    96.47 |    85.92 |      100 |    96.44 |... 451,529,533 |
 SelectTrigger.jsx |      100 |    95.45 |      100 |      100 |                |
 index.js          |      100 |      100 |      100 |      100 |                |
 util.js           |    86.15 |     82.5 |    93.33 |    86.15 |... 108,109,115 |
-------------------|----------|----------|----------|----------|----------------|

@@ -97,7 +97,7 @@ const DropdownMenu = React.createClass({

clonedMenuItems = menuItems.map(item => {
if (item.type === MenuItemGroup) {
const children = item.props.children.map(clone);
const children = React.Children.map(item.props.children, clone);
Copy link
Member Author

Choose a reason for hiding this comment

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

这里做了个小调整。

Copy link
Member

Choose a reason for hiding this comment

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

这个不能改的,不能用 map 的,map 会修改 key

Copy link
Member Author

Choose a reason for hiding this comment

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

不过很奇怪,我在测试里只给 MenuItemGroup 传一个 child 的话,这里的 item.props.children 就不是一个数组。但是实际使用的时候,只给一个 child 这里就是数组。

Copy link
Member Author

@yesmeck yesmeck Dec 6, 2016

Choose a reason for hiding this comment

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

奥,实际使用的时候,children 是从 FilterMixin 过来的,所以是数组。

@@ -364,6 +364,7 @@ const Select = React.createClass({
}
}
} else if (isMultipleOrTags(props) && inputValue) {
// why not use setState?
this.state.inputValue = this.getInputDOMNode().value = '';
Copy link
Member Author

Choose a reason for hiding this comment

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

这里没看懂为什么没用 setState

@yesmeck yesmeck changed the title Add tons of tests. Add tons of tests Dec 2, 2016
@coveralls
Copy link

Coverage Status

Coverage increased (+20.6%) to 95.591% when pulling e6b3bd9 on yesmeck:improve-testing into 579f4d4 on react-component:master.

@yesmeck yesmeck changed the title Add tons of tests Improve testing Dec 5, 2016
@yesmeck
Copy link
Member Author

yesmeck commented Dec 5, 2016

@yiminghe 这样测试可以的话,后面再推广到其他组件吧。

@coveralls
Copy link

Coverage Status

Coverage increased (+20.6%) to 95.591% when pulling 1377d78 on yesmeck:improve-testing into 579f4d4 on react-component:master.

@yiminghe yiminghe merged commit a00b2c4 into react-component:master Dec 6, 2016
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.

3 participants