Test 'app' module to load all modules for correct code coverage #126
Conversation
ChristophWurst
changed the title
| out: '../js/mail.min.js', | ||
| insertRequire: [ | ||
| 'app', | ||
| 'notification' |
There was a problem hiding this comment.
cc @skjnldsv: I fixed another occurrence of that bloody module. I knew it was loaded twice, but I couldn't find it when I fix the other one…
There was a problem hiding this comment.
God dammit! You again Mr notification! I was expecting you!
js/init.js
Outdated
|
|
||
| $(function() { | ||
| // Start app when the page is ready | ||
| console.log('starting Mail…'); |
There was a problem hiding this comment.
before someone nitpicks this as debug code – I've added in on purpose because it could be handy to debug js errors in the future if someone reports a weird client error. We'd then know whether the app tried to start or not.
There was a problem hiding this comment.
Nitpickingly corrected the writing style to »Starting Mail …« ;D
|
Hm. Now coverage reports 28 instead of 15 files… Not sure if we actually caught all modules 🤔 Still, it's an improvement… |
|
@jancborchardt @skjnldsv any issues detected? Can we merge this PR? |
|
Let me check a moment. |
|
Looks all good 👍 |
|
I guess coveralls decreasing is separte |
|
Haaaan, @ChristophWurst is doing all the hard work and you get the pleasure to merge! :O |
|
@skjnldsv :DD you can also press the button if you like. ;) It only seems that the plugin doesn’t register 👍 when it was written inside of a review comment (which is good I guess). |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and questions. |
Although only a few lines have changes, I expect this PR to have a huge effect on code coverage. Since the module
mailcan be seen as the root of all js modules, we basically load all modules the application uses. Therefore, the js code coverage should finally show the real percentage of tested code.We can later extend this test case to have a full integration test of the app start. Currently I mocked the initialization code and discarded the emitted events to keep things simple.
cc @Gomez @nextcloud/mail