Added ability to run tests in karma #69
Conversation
There was a problem hiding this comment.
My JavaScript skills are probably as bad as my German skills, so I'd let @thiemowmde and @JonasKress say something more relevant here.
From what I can see test are run on Travis, so I am happy. Travis build script etc could probably be improved but this is something for another pull request. Thanks for your effort!
package.json
Outdated
| "author": { "name" : "H. Snater", "url" : "http://www.snater.com"}, | ||
| "contributors":[ | ||
| { "name" : "H. Snater", "url" : "http://www.snater.com"}, | ||
| { "name" : "Adrian Lang", "email" : "adrian.lang@wikimedia.de"} |
There was a problem hiding this comment.
I am not sure if it makes sense to include this email address when Adrian is no longer working for WMDE. Is it OK to only have a "name" here?
For probably more important part: I guess would be better to "title" Adrian as "Adrian Heine".
There was a problem hiding this comment.
Just copied it from composer.json. Personally, don't care. 😄
| @@ -1,4 +1,5 @@ | |||
| /** | |||
| * @ignore | |||
| */ | |||
| wikibase.datamodel = wikibase.datamodel || {}; | |||
| window.wikibase = window.wikibase || {}; | |||
There was a problem hiding this comment.
Could some JS wizard explain what is the different before the code before and after? Would both work in all cases? What "normal" browsers do different so that it apparently was not an issue before.
This comment is not mean as anything against this particular PR. I just don't understand intricacies of Java Script.
There was a problem hiding this comment.
It fails on NodeJS because wikibase variable is not defined and it tries to access it.
But having more full explanation would be nice.
There was a problem hiding this comment.
As far as I understood it might be PhantomJS "feature", but any way new code should work just fine.
| 'vendor/data-values/javascript/src/dataValues.js', | ||
| 'vendor/data-values/javascript/src/*.js', | ||
| 'vendor/data-values/javascript/src/values/*.js', | ||
| 'src/*.js', |
There was a problem hiding this comment.
This file is automatically generated right? Seems a bit odd to have some/all files listed individually and then use the wildcard for, at least partly, the same stuff.
There was a problem hiding this comment.
😄 Seems reasonable to expect that it is generated automatically, but no.
I did it by hand.
Basically, I was adding needed files before 'src/*.js' until there were no errors. May be we can generate it, who knows...
As soon as code in this repository doesn't depend on the Core, we are able to run tests independently.
With this PR you can just do
npm install && npm testand that's it!