-
Notifications
You must be signed in to change notification settings - Fork 42
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
Setup JavaScript tests for Nuage provider #122
Conversation
With this commit we provide nothing but configuration files needed for Nuage React Components. Signed-off-by: Miha Pleško <[email protected]>
Note: only 2nd commit is important here |
package.json
Outdated
@@ -15,6 +15,9 @@ | |||
"homepage": "https://github.com/ManageIQ/manageiq#readme", | |||
"dependencies": { | |||
"@manageiq/react-ui-components": "~0.9.5", | |||
"chai": "^4.1.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason why you want to use this library? I don't think you will need this at all. Jest already has pretty good assertions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, was looking at enzyme example and they are using it. Let me remove.
describe('<Spinner/>', () => { | ||
it('renders three <Foo /> components', () => { | ||
const wrapper = shallow(<Spinner loading />); | ||
expect(wrapper.find('.spinner')).to.have.length(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this is basically one the most annoying things about testing react components. Hopefully you can use snapshot testing with Jest https://jestjs.io/docs/en/snapshot-testing (also i would recommend using enzime-to-json instead of the react-renderer in the example).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing out, will come up with something tomorrow. Also, I need to configure Travis to actually run them, almost forgot 🐷
ee8e524
to
f9c03c8
Compare
package.json
Outdated
"homepage": "https://github.com/ManageIQ/manageiq#readme", | ||
"dependencies": { | ||
"@manageiq/react-ui-components": "~0.9.5", | ||
"enzyme": "^3.3.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enzyme, enzyme-adapter-react-16, enzyme-to-json
should be dev-dependencies
80240aa
to
902ac26
Compare
describe('<Spinner/>', () => { | ||
it('renders three <Foo /> components', () => { | ||
const wrapper = shallow(<Spinner loading />); | ||
expect(wrapper.find('.spinner')).toHaveLength(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would maybe add a snapshot test here to show off how it works:
import toJson from 'enzyme-to-json';
...
expect(toJson(wrapper)).toMatchSnapshot();
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, I owe you this from yesterday, as soon as Travis configuration works I'll do it.
fb15be4
to
eacd971
Compare
With this commit we setup two libraries for javascript tests: - jest (pojo javascript tests) - enzyme (React component tests) For each of them we provide a single dummy test just to be sure configuration is valid. Also, we provide Travis configuration to actually run the tests. Signed-off-by: Miha Pleško <[email protected]>
eacd971
to
36131df
Compare
Checked commits miha-plesko/manageiq-providers-nuage@46b2bf4~...36131df with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
@Hyperkid123 thanks for help, I think we're almost done - Travis is green and we have snapshot test showcase. Please let me know if there is something else we can improve. |
I think this should be enough. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTMT
Setup JavaScript tests for Nuage provider
With this commit we setup two libraries for javascript tests:
For each of them we provide a single dummy test just to be sure configuration is valid.
Depends on: #121
/cc @Hyperkid123