-
Notifications
You must be signed in to change notification settings - Fork 0
Unit test #3
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
Unit test #3
Conversation
Add initial karma.confg file with settings and proper loaders. Add karma and other dependencies to package.json Update some outdated libraries in package.json
Add jasmine and karma-jasmine to the project. Add coverage reporter and configuration to karma.config Add tests for default instantiation of GitHub component Add initial test for ajax requests to test spies Add helper object to mock out json response from API Add needed devDependencies to project to complete unit testing
Add unit testing support links in README file. Add rendering tests for github information in component. Add test task in package.json file. Finish ajax testing for fetching user info. Fix testing suite __DEV__ reference by adding webpack plugin to karma config.
| */ | ||
| const config = { | ||
| devtool: 'cheap-eval-source-map', | ||
| devtool: env === 'production' ? '#eval' : '#eval-source-map', |
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.
hey @JFusco,
is this better suited in the production / develop config below?
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.
devtool: "eval-source-map" is really as good as devtool: "source-map", but can cache SourceMaps for modules. It’s much faster for rebuilds.
devtool: "eval" has the best performance, but it only maps to compiled source code per module. In many cases this is good enough.
"#" is defaulted in WP2, but I like to add it for good measure
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.
hey @JFusco,
was more asking if we should add here instead of the base config?
build-tools/webpack.config.babel.js
Line 83 in a4d2e93
| config.devtool = 'cheap-source-map' |
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.
oh HA - that needs to be removed, good catch - i'll remove that
base config is fine so we don't have to define devtools in 2 places
| }); | ||
|
|
||
| it('should return the correct data', () => { | ||
| const data = JSON.parse(request.responseText) |
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.
hey @JFusco,
without much knowledge around testing; does the response need to be parsed?
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.
Yes,
responseText is the only way that jasmine ajax works - it doesn't work with responseJSON, otherwise we'd use that
| module.exports = function (config) { | ||
| config.set({ | ||
| frameworks: ['jasmine'], | ||
| files: [ |
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.
hey @JFusco,
can we keep the spacing consistent? i don't mind if we move to tabs but proper indentation makes for clean code. go ahead and edit the editorconfig if you feel we need to change.
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'll change it, not sure why it's exploded like that lol
Add unit testing framework and functionality to build tools